Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

* Bug fixes:
+ Bound the byte length of the type-table header, 64 KiB by default and configurable with `DecoderConfig::set_max_header_len`. Headers over the bound, far above any realistic interface, are now rejected; the value section is unaffected, and `set_max_type_len` still separately bounds the number of type-table entries.
+ Bound the size of a type named in a decoder diagnostic. What it costs to render a type follows that type's own width and depth, and a type reaching the decoder is chosen by the sender, so a diagnostic now elides one whose rendering would exceed a fixed budget instead of rendering it in full. This holds for every diagnostic that names a type, including the subtyping messages and the verbose form that `set_full_error_message(true)` selects; ordinary mismatches are unchanged and still name both types.

## 2026-09-22

Expand Down
5 changes: 5 additions & 0 deletions rust/candid/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ all = ["default", "value", "ic_principal/arbitrary"]
name = "types"
path = "tests/types.rs"
required-features = ["value"]

[[test]]
name = "type_diagnostics"
path = "tests/type_diagnostics.rs"
required-features = ["value"]
[[test]]
name = "serde"
path = "tests/serde.rs"
Expand Down
58 changes: 41 additions & 17 deletions rust/candid/src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

use super::{
error::{Error, Result},
types::internal::{text_size, type_of, TypeId},
types::internal::{
elide_large, text_size, type_of, TypeId, MAX_DIAGNOSTIC_LIST_LEN, MAX_DIAGNOSTIC_TYPE_LEN,
},
types::{Field, Label, SharedLabel, Type, TypeEnv, TypeInner},
CandidType,
};
Expand All @@ -19,7 +21,22 @@ use serde::de::{self, Visitor};
use std::fmt::Write;
use std::{collections::VecDeque, io::Cursor, mem::replace, rc::Rc};

const MAX_TYPE_LEN: i32 = 500;
/// Render a type table for a diagnostic, eliding entries too large to render and
/// stopping once the table itself reaches its budget.
///
/// A table holds as many entries as `max_type_len` allows, so bounding each entry on
/// its own would still leave the whole rendering growing with their number.
fn describe_table(env: &crate::types::TypeEnv) -> String {
let mut out = String::new();
for (i, (name, ty)) in env.0.iter().enumerate() {
if out.len() >= MAX_DIAGNOSTIC_LIST_LEN {
let _ = writeln!(&mut out, "... and {} more", env.0.len() - i);
break;
}
let _ = writeln!(&mut out, "type {name} = {}", elide_large(ty));
}
out
}

/// Use this struct to deserialize a sequence of Rust values (heterogeneous) from IDL binary message.
pub struct IDLDeserialize<'de> {
Expand Down Expand Up @@ -73,9 +90,7 @@ impl<'de> IDLDeserialize<'de> {
self.de.expect_type = expected_type;
self.de.wire_type = TypeInner::Null.into();
return T::deserialize(&mut self.de);
} else if self.de.config.full_error_message
|| text_size(&expected_type, MAX_TYPE_LEN).is_ok()
{
} else if text_size(&expected_type, MAX_DIAGNOSTIC_TYPE_LEN).is_ok() {
return Err(Error::msg(format!(
"No more values on the wire, the expected type {expected_type} is not opt, null, or reserved"
)));
Expand All @@ -94,9 +109,8 @@ impl<'de> IDLDeserialize<'de> {
self.de.wire_type = ty.clone();

let mut v = T::deserialize(&mut self.de).with_context(|| {
if self.de.config.full_error_message
|| (text_size(&ty, MAX_TYPE_LEN).is_ok()
&& text_size(&expected_type, MAX_TYPE_LEN).is_ok())
if text_size(&ty, MAX_DIAGNOSTIC_TYPE_LEN).is_ok()
&& text_size(&expected_type, MAX_DIAGNOSTIC_TYPE_LEN).is_ok()
{
format!("Fail to decode argument {ind} from {ty} to {expected_type}")
} else {
Expand Down Expand Up @@ -389,12 +403,13 @@ impl<'de> Deserializer<'de> {
let (before, after) = hex.split_at(pos);
let mut res = format!("input: {before}_{after}\n");
if !self.table.0.is_empty() {
write!(&mut res, "table: {}", self.table).unwrap();
write!(&mut res, "table: {}", describe_table(&self.table)).unwrap();
}
write!(
&mut res,
"wire_type: {}, expect_type: {}",
self.wire_type, self.expect_type
elide_large(&self.wire_type),
elide_large(&self.expect_type)
)
.unwrap();
if let Some(field) = &self.field_name {
Expand Down Expand Up @@ -507,9 +522,8 @@ impl<'de> Deserializer<'de> {
&self.expect_type,
)
.with_context(|| {
if self.config.full_error_message
|| (text_size(&self.wire_type, MAX_TYPE_LEN).is_ok()
&& text_size(&self.expect_type, MAX_TYPE_LEN).is_ok())
if text_size(&self.wire_type, MAX_DIAGNOSTIC_TYPE_LEN).is_ok()
&& text_size(&self.expect_type, MAX_DIAGNOSTIC_TYPE_LEN).is_ok()
{
format!(
"{} is not a subtype of {}",
Expand Down Expand Up @@ -619,7 +633,7 @@ impl<'de> Deserializer<'de> {
} else {
return Err(Error::subtype(format!(
"{} cannot be deserialized to int",
self.wire_type
elide_large(&self.wire_type)
)));
}
}
Expand All @@ -628,7 +642,12 @@ impl<'de> Deserializer<'de> {
let int = match self.wire_type.as_ref() {
TypeInner::Int => Int::decode(&mut self.input).map_err(Error::msg)?,
TypeInner::Nat => Int(Nat::decode(&mut self.input).map_err(Error::msg)?.0.into()),
t => return Err(Error::subtype(format!("{t} cannot be deserialized to int"))),
_ => {
return Err(Error::subtype(format!(
"{} cannot be deserialized to int",
elide_large(&self.wire_type)
)))
}
};
self.add_cost((self.input.position() - bignum_pos) as usize)?;
bytes.extend_from_slice(&int.0.to_signed_bytes_le());
Expand Down Expand Up @@ -1038,7 +1057,12 @@ impl<'de> de::Deserializer<'de> for &mut Deserializer<'de> {
TypeInner::Int => decode_int(&mut self.input)?,
TypeInner::Nat => i128::try_from(decode_nat(&mut self.input)?)
.map_err(|_| Error::msg("Cannot convert nat to i128"))?,
t => return Err(Error::subtype(format!("{t} cannot be deserialized to int"))),
_ => {
return Err(Error::subtype(format!(
"{} cannot be deserialized to int",
elide_large(&self.wire_type)
)))
}
};
visitor.visit_i128(value)
}
Expand Down Expand Up @@ -1244,7 +1268,7 @@ impl<'de> de::Deserializer<'de> for &mut Deserializer<'de> {
if !self.wire_type.is_tuple() {
return Err(Error::subtype(format!(
"{} is not a tuple type",
self.wire_type
elide_large(&self.wire_type)
)));
}
let value = visitor.visit_seq(Compound::new(
Expand Down
41 changes: 40 additions & 1 deletion rust/candid/src/types/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,42 @@ impl fmt::Display for TypeInner {
write!(f, "{:?}", self)
}
}
/// Budget, in rendered characters, for a type named in a diagnostic.
pub(crate) const MAX_DIAGNOSTIC_TYPE_LEN: i32 = 500;

/// Stands in for a type a diagnostic cannot render within its budget.
pub(crate) const ELIDED_TYPE: &str = "(type elided)";
Comment thread
lwshang marked this conversation as resolved.

/// Budget, in characters, for a list of types in a diagnostic. Bounds the list itself,
/// which a per-element budget does not.
pub(crate) const MAX_DIAGNOSTIC_LIST_LEN: usize = 2048;

/// Renders a type for a diagnostic, eliding it when rendering would be unreasonable.
///
/// What it costs to render a type follows that type's own width and depth, so an
/// outsized one must not be rendered at all: [`text_size`] settles that against a
/// budget before any of the work is done. Types reaching a decoder come from the wire
/// and so are chosen by the sender, which is why the budget holds for every diagnostic
/// naming one, however verbose the caller asked its errors to be.
///
/// Total for every type: one that cannot be rendered at all, such as a service
/// constructor, is refused by [`text_size`] and elided like any oversized one.
pub(crate) fn elide_large(t: &Type) -> ElidedType<'_> {
ElidedType(t)
}

pub(crate) struct ElidedType<'a>(&'a Type);

impl fmt::Display for ElidedType<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if text_size(self.0, MAX_DIAGNOSTIC_TYPE_LEN).is_ok() {
write!(f, "{}", self.0)
Comment thread
lwshang marked this conversation as resolved.
Comment thread
lwshang marked this conversation as resolved.
} else {
f.write_str(ELIDED_TYPE)
}
}
}

#[allow(clippy::result_unit_err)]
pub fn text_size(t: &Type, limit: i32) -> Result<i32, ()> {
use TypeInner::*;
Expand Down Expand Up @@ -422,7 +458,10 @@ pub fn text_size(t: &Type, limit: i32) -> Result<i32, ()> {
}
Future => 6,
Unknown => 7,
Class(..) => unreachable!(),
// A service constructor has no rendering of its own: the pretty printer has no
// arm for one. Refusing it here, rather than asserting it cannot appear, keeps
// every caller total for a type that merely contains one.
Class(..) => return Err(()),
};
if cost > limit {
Err(())
Expand Down
92 changes: 66 additions & 26 deletions rust/candid/src/types/subtype.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use super::internal::{find_type, Field, Label, Type, TypeInner};
use super::internal::{
elide_large, find_type, Field, Label, Type, TypeInner, MAX_DIAGNOSTIC_LIST_LEN,
};
use crate::types::TypeEnv;
use crate::utils::RecursionDepth;
use crate::{Error, Result};
Expand Down Expand Up @@ -291,7 +293,7 @@ fn subtype_collect_(
Ok(Null | Reserved | Opt(_))
) => {}
(_, Opt(_)) => {
let msg = format!("WARNING: {t1} <: {t2} due to special subtyping rules involving optional types/fields (see https://github.com/dfinity/candid/blob/c7659ca/spec/Candid.md#upgrading-and-subtyping). This means the two interfaces have diverged, which could cause data loss.");
let msg = format!("WARNING: {} <: {} due to special subtyping rules involving optional types/fields (see https://github.com/dfinity/candid/blob/c7659ca/spec/Candid.md#upgrading-and-subtyping). This means the two interfaces have diverged, which could cause data loss.", elide_large(t1), elide_large(t2));
match report {
OptReport::Silence => (),
OptReport::Warning => eprintln!("{msg}"),
Expand Down Expand Up @@ -324,13 +326,15 @@ fn subtype_collect_(
path: path.clone(),
message: if is_input {
format!(
"new service requires field {id} (type {ty2}), \
which old callers don't provide and is not optional"
"new service requires field {id} (type {}), \
which old callers don't provide and is not optional",
elide_large(ty2)
)
} else {
format!(
"new type is missing required field {id} (type {ty2}), \
which is expected by the old type and is not optional"
"new type is missing required field {id} (type {}), \
which is expected by the old type and is not optional",
elide_large(ty2)
)
},
});
Expand Down Expand Up @@ -428,7 +432,11 @@ fn subtype_collect_(
(_, _) => {
errors.push(Incompatibility {
path: path.clone(),
message: format!("{t1} is not a subtype of {t2}"),
message: format!(
"{} is not a subtype of {}",
elide_large(t1),
elide_large(t2)
),
});
}
}
Expand Down Expand Up @@ -573,7 +581,7 @@ fn subtype_(
Ok(())
}
(_, Opt(_)) => {
let msg = format!("WARNING: {t1} <: {t2} due to special subtyping rules involving optional types/fields (see https://github.com/dfinity/candid/blob/c7659ca/spec/Candid.md#upgrading-and-subtyping). This means the two interfaces have diverged, which could cause data loss.");
let msg = format!("WARNING: {} <: {} due to special subtyping rules involving optional types/fields (see https://github.com/dfinity/candid/blob/c7659ca/spec/Candid.md#upgrading-and-subtyping). This means the two interfaces have diverged, which could cause data loss.", elide_large(t1), elide_large(t2));
match report {
OptReport::Silence => (),
OptReport::Warning => eprintln!("{msg}"),
Expand All @@ -587,15 +595,22 @@ fn subtype_(
match fields.get(id) {
Some(ty1) => {
subtype_(report, gamma, env, ty1, ty2, depth).with_context(|| {
format!("Record field {id}: {ty1} is not a subtype of {ty2}")
format!(
"Record field {id}: {} is not a subtype of {}",
elide_large(ty1),
elide_large(ty2)
)
})?
}
None => {
if !matches!(
env.trace_type_with_depth(ty2, depth)?.as_ref(),
Null | Reserved | Opt(_)
) {
return Err(Error::msg(format!("Record field {id}: {ty2} is only in the expected type and is not of type opt, null or reserved")));
return Err(Error::msg(format!(
"Record field {id}: {} is only in the expected type and is not of type opt, null or reserved",
elide_large(ty2)
)));
}
}
}
Expand All @@ -608,7 +623,11 @@ fn subtype_(
match fields.get(id) {
Some(ty2) => {
subtype_(report, gamma, env, ty1, ty2, depth).with_context(|| {
format!("Variant field {id}: {ty1} is not a subtype of {ty2}")
format!(
"Variant field {id}: {} is not a subtype of {}",
elide_large(ty1),
elide_large(ty2)
)
})?
}
None => {
Expand All @@ -626,7 +645,11 @@ fn subtype_(
match meths.get(name) {
Some(ty1) => {
subtype_(report, gamma, env, ty1, ty2, depth).with_context(|| {
format!("Method {name}: {ty1} is not a subtype of {ty2}")
format!(
"Method {name}: {} is not a subtype of {}",
elide_large(ty1),
elide_large(ty2)
)
})?
}
None => {
Expand Down Expand Up @@ -657,7 +680,11 @@ fn subtype_(
(_, Class(_, t)) => subtype_(report, gamma, env, t1, t, depth),
(Unknown, _) => unreachable!(),
(_, Unknown) => unreachable!(),
(_, _) => Err(Error::msg(format!("{t1} is not a subtype of {t2}"))),
(_, _) => Err(Error::msg(format!(
"{} is not a subtype of {}",
elide_large(t1),
elide_large(t2)
))),
}
}

Expand Down Expand Up @@ -722,7 +749,9 @@ fn equal_impl(
}
equal_impl(gamma, env, &f1.ty, &f2.ty, depth).context(format!(
"Field {} has different types: {} and {}",
f1.id, f1.ty, f2.ty
f1.id,
elide_large(&f1.ty),
elide_large(&f2.ty)
))?;
}
Ok(())
Expand All @@ -739,7 +768,9 @@ fn equal_impl(
}
equal_impl(gamma, env, &m1.1, &m2.1, depth).context(format!(
"Method {} has different types: {} and {}",
m1.0, m1.1, m2.1
m1.0,
elide_large(&m1.1),
elide_large(&m2.1)
))?;
}
Ok(())
Expand Down Expand Up @@ -770,7 +801,11 @@ fn equal_impl(
}
(Unknown, _) => unreachable!(),
(_, Unknown) => unreachable!(),
(_, _) => Err(Error::msg(format!("{t1} is not equal to {t2}"))),
(_, _) => Err(Error::msg(format!(
"{} is not equal to {}",
elide_large(t1),
elide_large(t2)
))),
Comment thread
lwshang marked this conversation as resolved.
}
}

Expand Down Expand Up @@ -814,19 +849,24 @@ fn to_tuple(args: &[Type]) -> Type {
)
.into()
}
#[cfg(not(feature = "printer"))]
/// Renders an argument list for a diagnostic, eliding any type too large to render and
/// stopping once the list itself reaches its budget.
///
/// A per-element budget alone would leave the list unbounded, since the cost would then
/// grow with the number of arguments. `elide_large` renders through `Display`, which
/// already picks the pretty printer or the fallback according to the `printer` feature,
/// so one implementation serves both.
fn pp_args(args: &[crate::types::Type]) -> String {
use std::fmt::Write;
let mut s = String::new();
write!(&mut s, "(").unwrap();
for arg in args.iter() {
write!(&mut s, "{:?}, ", arg).unwrap();
let _ = write!(&mut s, "(");
for (i, arg) in args.iter().enumerate() {
if s.len() >= MAX_DIAGNOSTIC_LIST_LEN {
let _ = write!(&mut s, "... and {} more", args.len() - i);
break;
}
let _ = write!(&mut s, "{}, ", elide_large(arg));
}
write!(&mut s, ")").unwrap();
let _ = write!(&mut s, ")");
s
}
#[cfg(feature = "printer")]
fn pp_args(args: &[crate::types::Type]) -> String {
use crate::pretty::candid::pp_args;
pp_args(args).pretty(80).to_string()
}
Loading
Loading