From b04e9ee6e445c8df6f0234c742bf8a41ead9870c Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Thu, 13 Aug 2026 13:22:24 -0400 Subject: [PATCH 1/2] fix(candid_parser): escape service type method names in the Rust binding + release candid_parser 0.4.1 Patch release. `pp_ty_service` emitted the method names of a service type into a Rust string literal without escaping them, so a name containing characters that are legal in Candid text but significant in Rust produced incorrect output. They now go through `escape_debug`, matching `pp_function` and the `#[serde(rename)]` attributes. See CHANGELOG. Output is byte-identical for names that are ordinary identifiers, so no existing goldenfile changed. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 7 + Cargo.lock | 7 +- rust/bench/Cargo.lock | 2 +- rust/candid_parser/Cargo.toml | 4 +- rust/candid_parser/src/bindings/rust.rs | 5 +- .../assets/ok/service_method_escape.d.ts | 24 ++++ .../tests/assets/ok/service_method_escape.did | 13 ++ .../tests/assets/ok/service_method_escape.js | 16 +++ .../tests/assets/ok/service_method_escape.rs | 29 ++++ .../tests/assets/service_method_escape.did | 18 +++ rust/candid_parser/tests/parse_type.rs | 2 +- .../candid_parser/tests/test_rust_bindings.rs | 130 ++++++++++++++++++ 12 files changed, 250 insertions(+), 7 deletions(-) create mode 100644 rust/candid_parser/tests/assets/ok/service_method_escape.d.ts create mode 100644 rust/candid_parser/tests/assets/ok/service_method_escape.did create mode 100644 rust/candid_parser/tests/assets/ok/service_method_escape.js create mode 100644 rust/candid_parser/tests/assets/ok/service_method_escape.rs create mode 100644 rust/candid_parser/tests/assets/service_method_escape.did create mode 100644 rust/candid_parser/tests/test_rust_bindings.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 02794e221..95fdf0007 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 2026-08-13 + +### candid_parser 0.4.1 + +* Bug fixes: + + Escape the method names of a service type in the Rust binding. A Candid method name is an arbitrary text value, but `pp_ty_service` emitted it raw between the quotes of a Rust string literal inside `candid::define_service!`. A name containing `"` therefore closed the literal and the macro invocation, and the rest of the name was compiled as Rust — a `.did` file could inject arbitrary items into the bindings generated from it, and from there into the consumer's binary. Names are now escaped with `escape_debug`, as `pp_function` and the `#[serde(rename)]` attributes already were. The value seen by `define_service!` is unchanged, and names that are ordinary identifiers generate byte-identical output. + ## 2026-08-11 ### Candid 0.10.35 diff --git a/Cargo.lock b/Cargo.lock index ec8e5a345..2c541f489 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -251,7 +251,7 @@ dependencies = [ "binrw", "byteorder", "candid_derive 0.10.35", - "candid_parser 0.4.0", + "candid_parser 0.4.1", "hex", "ic_principal 0.1.5", "leb128", @@ -311,7 +311,7 @@ dependencies = [ [[package]] name = "candid_parser" -version = "0.4.0" +version = "0.4.1" dependencies = [ "anyhow", "arbitrary", @@ -333,6 +333,7 @@ dependencies = [ "pretty", "rand", "serde", + "syn 2.0.87", "test-generator", "thiserror", "toml", @@ -517,7 +518,7 @@ name = "didc" version = "0.6.2" dependencies = [ "anyhow", - "candid_parser 0.4.0", + "candid_parser 0.4.1", "clap", "console", "hex", diff --git a/rust/bench/Cargo.lock b/rust/bench/Cargo.lock index e8baae758..c48d5dbcc 100644 --- a/rust/bench/Cargo.lock +++ b/rust/bench/Cargo.lock @@ -199,7 +199,7 @@ dependencies = [ [[package]] name = "candid_parser" -version = "0.4.0" +version = "0.4.1" dependencies = [ "anyhow", "candid", diff --git a/rust/candid_parser/Cargo.toml b/rust/candid_parser/Cargo.toml index a01d402cd..3e39cd547 100644 --- a/rust/candid_parser/Cargo.toml +++ b/rust/candid_parser/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "candid_parser" -version = "0.4.0" +version = "0.4.1" edition = "2021" rust-version.workspace = true authors = ["DFINITY Team"] @@ -46,6 +46,8 @@ console = { workspace = true, optional = true } goldenfile = "1.1.0" test-generator = "0.3.0" rand.workspace = true +# Used to assert the *structure* of the generated Rust bindings, which the goldenfiles cannot. +syn = { version = "2", features = ["full", "parsing", "extra-traits"] } [features] random = ["dep:arbitrary", "dep:fake", "dep:rand", "dep:num-traits"] diff --git a/rust/candid_parser/src/bindings/rust.rs b/rust/candid_parser/src/bindings/rust.rs index ef1439dab..86ea8a829 100644 --- a/rust/candid_parser/src/bindings/rust.rs +++ b/rust/candid_parser/src/bindings/rust.rs @@ -606,8 +606,11 @@ fn test_{test_name}() {{ TypeInner::Var(_) => self.pp_ty(func, true).append("::ty()"), _ => unreachable!(), }; + // The method name is emitted as a Rust string literal, so it has to be escaped. + // A Candid method name is an arbitrary text value, and a name containing `"` would + // otherwise close the literal and let the rest of the name be parsed as Rust code. RcDoc::text("\"") - .append(id) + .append(id.escape_debug().to_string()) .append(kwd("\" :")) .append(func_doc) }); diff --git a/rust/candid_parser/tests/assets/ok/service_method_escape.d.ts b/rust/candid_parser/tests/assets/ok/service_method_escape.d.ts new file mode 100644 index 000000000..2065db60e --- /dev/null +++ b/rust/candid_parser/tests/assets/ok/service_method_escape.d.ts @@ -0,0 +1,24 @@ +import type { Principal } from '@icp-sdk/core/principal'; +import type { ActorMethod } from '@icp-sdk/core/agent'; +import type { IDL } from '@icp-sdk/core/candid'; + +/** + * The method names of a service type are arbitrary Candid text values, but every binding emits + * them into a string literal of the target language. Names containing quotes, backslashes, comment + * markers or newlines have to be escaped for the generated code to stay well-formed. + */ +export type f = ActorMethod<[], undefined>; +export interface inner { + 'backslash\\' : f, + 'braces { } and parens ( )' : f, + 'comment markers // and /* */' : f, + 'newline\nand carriage return\r' : f, + 'quote\"' : f, + 'tab\tand semicolon;' : f, +} +export interface _SERVICE { + 'ping' : ActorMethod<[], string>, + 'use_inner' : ActorMethod<[Principal], undefined>, +} +export declare const idlFactory: IDL.InterfaceFactory; +export declare const init: (args: { IDL: typeof IDL }) => IDL.Type[]; diff --git a/rust/candid_parser/tests/assets/ok/service_method_escape.did b/rust/candid_parser/tests/assets/ok/service_method_escape.did new file mode 100644 index 000000000..996b04a11 --- /dev/null +++ b/rust/candid_parser/tests/assets/ok/service_method_escape.did @@ -0,0 +1,13 @@ +// The method names of a service type are arbitrary Candid text values, but every binding emits +// them into a string literal of the target language. Names containing quotes, backslashes, comment +// markers or newlines have to be escaped for the generated code to stay well-formed. +type f = func () -> (); +type inner = service { + "backslash\\" : f; + "braces { } and parens ( )" : f; + "comment markers // and /* */" : f; + "newline\nand carriage return\r" : f; + "quote\"" : f; + "tab\tand semicolon;" : f; +}; +service : { ping : () -> (text); use_inner : (inner) -> () } diff --git a/rust/candid_parser/tests/assets/ok/service_method_escape.js b/rust/candid_parser/tests/assets/ok/service_method_escape.js new file mode 100644 index 000000000..85a2a6c45 --- /dev/null +++ b/rust/candid_parser/tests/assets/ok/service_method_escape.js @@ -0,0 +1,16 @@ +export const idlFactory = ({ IDL }) => { + const f = IDL.Func([], [], []); + const inner = IDL.Service({ + 'backslash\\' : f, + 'braces { } and parens ( )' : f, + 'comment markers // and /* */' : f, + 'newline\nand carriage return\r' : f, + 'quote\"' : f, + 'tab\tand semicolon;' : f, + }); + return IDL.Service({ + 'ping' : IDL.Func([], [IDL.Text], []), + 'use_inner' : IDL.Func([inner], [], []), + }); +}; +export const init = ({ IDL }) => { return []; }; diff --git a/rust/candid_parser/tests/assets/ok/service_method_escape.rs b/rust/candid_parser/tests/assets/ok/service_method_escape.rs new file mode 100644 index 000000000..390d2424b --- /dev/null +++ b/rust/candid_parser/tests/assets/ok/service_method_escape.rs @@ -0,0 +1,29 @@ +// This is an experimental feature to generate Rust binding from Candid. +// You may want to manually adjust some of the types. +#![allow(dead_code, unused_imports)] +use candid::{self, CandidType, Deserialize, Principal}; +use ic_cdk::api::call::CallResult as Result; + +candid::define_function!(pub F : () -> ()); +candid::define_service!(pub Inner : { + "backslash\\" : F::ty(); + "braces { } and parens ( )" : F::ty(); + "comment markers // and /* */" : F::ty(); + "newline\nand carriage return\r" : F::ty(); + "quote\"" : F::ty(); + "tab\tand semicolon;" : F::ty(); +}); + +pub struct Service(pub Principal); +impl Service { + pub async fn ping(&self) -> Result<(String,)> { + ic_cdk::call(self.0, "ping", ()).await + } + pub async fn use_inner(&self, arg0: &Inner) -> Result<()> { + ic_cdk::call(self.0, "use_inner", (arg0,)).await + } +} +/// Canister ID: `aaaaa-aa` +pub const CANISTER_ID : Principal = Principal::from_slice(&[]); +pub const service : Service = Service(CANISTER_ID); + diff --git a/rust/candid_parser/tests/assets/service_method_escape.did b/rust/candid_parser/tests/assets/service_method_escape.did new file mode 100644 index 000000000..8981a759b --- /dev/null +++ b/rust/candid_parser/tests/assets/service_method_escape.did @@ -0,0 +1,18 @@ +// The method names of a service type are arbitrary Candid text values, but every binding emits +// them into a string literal of the target language. Names containing quotes, backslashes, comment +// markers or newlines have to be escaped for the generated code to stay well-formed. +type f = func () -> (); + +type inner = service { + "quote\"" : f; + "backslash\\" : f; + "newline\nand carriage return\r" : f; + "tab\tand semicolon;" : f; + "comment markers // and /* */" : f; + "braces { } and parens ( )" : f; +}; + +service : { + use_inner : (inner) -> (); + ping : () -> (text); +}; diff --git a/rust/candid_parser/tests/parse_type.rs b/rust/candid_parser/tests/parse_type.rs index 4bf218ce7..69a6a2af1 100644 --- a/rust/candid_parser/tests/parse_type.rs +++ b/rust/candid_parser/tests/parse_type.rs @@ -133,7 +133,7 @@ fn compiler_test(resource: &str) { } { match filename.file_name().unwrap().to_str().unwrap() { - "unicode.did" | "escape.did" => check_error( + "unicode.did" | "escape.did" | "service_method_escape.did" => check_error( || motoko::compile(&env, &actor, &prog), "not a valid Motoko id", ), diff --git a/rust/candid_parser/tests/test_rust_bindings.rs b/rust/candid_parser/tests/test_rust_bindings.rs new file mode 100644 index 000000000..e4aaf2429 --- /dev/null +++ b/rust/candid_parser/tests/test_rust_bindings.rs @@ -0,0 +1,130 @@ +//! Structural checks on the generated Rust bindings. +//! +//! The goldenfiles in `tests/assets/ok` record what the generator emits, but they cannot state +//! that the output is *safe*: a regression would simply be blessed into the goldenfile. These +//! tests assert the properties instead. + +use candid::types::TypeEnv; +use candid_parser::bindings::rust::{compile, Config, ExternalConfig}; +use candid_parser::configs::Configs; +use candid_parser::syntax::{IDLMergedProg, IDLProg}; +use candid_parser::typing::check_prog; +use std::str::FromStr; + +/// Generate Rust bindings for an in-memory Candid program. +fn compile_did(source: &str) -> String { + let prog: IDLProg = source.parse().unwrap(); + let mut env = TypeEnv::new(); + let actor = check_prog(&mut env, &prog).unwrap(); + let merged = IDLMergedProg::new(prog); + + let config = Config::new(Configs::from_str("").unwrap()); + let mut external = ExternalConfig::default(); + external + .0 + .insert("canister_id".to_string(), "aaaaa-aa".to_string()); + let (content, _unused) = compile(&config, &env, &actor, &merged, external); + content +} + +/// Describe the top-level items of a Rust source file, as `kind` or `kind:name` labels. +/// +/// Deliberately coarse: the point is to compare the *shape* of two generated files, not to pin +/// down the template, so this survives ordinary changes to what the generator emits. +fn item_shape(source: &str) -> Vec { + let file = syn::parse_file(source) + .unwrap_or_else(|e| panic!("generated bindings are not valid Rust: {e}\n\n{source}")); + file.items + .iter() + .map(|item| match item { + syn::Item::Use(_) => "use".to_string(), + syn::Item::Macro(m) => { + let path = m + .mac + .path + .segments + .iter() + .map(|s| s.ident.to_string()) + .collect::>() + .join("::"); + format!("macro:{path}!") + } + syn::Item::Const(c) => format!("const:{}", c.ident), + syn::Item::Static(s) => format!("static:{}", s.ident), + syn::Item::Struct(s) => format!("struct:{}", s.ident), + syn::Item::Enum(e) => format!("enum:{}", e.ident), + syn::Item::Type(t) => format!("type:{}", t.ident), + syn::Item::Fn(f) => format!("fn:{}", f.sig.ident), + syn::Item::Mod(m) => format!("mod:{}", m.ident), + syn::Item::Impl(_) => "impl".to_string(), + other => format!("other:{other:?}"), + }) + .collect() +} + +/// A service type whose method names need escaping, and the same service with plain names. +/// +/// The method names of a service *type* are emitted as Rust string literals inside +/// `candid::define_service!`. A Candid method name is an arbitrary text value, so it can contain +/// quotes, backslashes, comment markers and newlines. Escaping them is what keeps a name inside +/// its literal instead of being parsed as Rust. +/// +/// The first name is the case that matters: unescaped, it closes both the literal and the macro +/// invocation and leaves the remainder as a well-formed item, so the generated bindings still +/// compile and the breakout is silent. The rest cover the other characters that can end a literal. +const NEEDS_ESCAPING: &str = r#" +type f = func () -> (); +type inner = service { + "quote\" : F::ty() }); const marker: u32 = ({ 0 //" : f; + "backslash\\" : f; + "newline\nand carriage return\r" : f; + "tab\tand semicolon;" : f; + "comment markers // and /* */" : f; + "braces { } and parens ( )" : f; +}; +service : { use_inner : (inner) -> (); }; +"#; + +const PLAIN: &str = r#" +type f = func () -> (); +type inner = service { + "m0" : f; + "m1" : f; + "m2" : f; + "m3" : f; + "m4" : f; + "m5" : f; +}; +service : { use_inner : (inner) -> (); }; +"#; + +#[test] +fn service_method_names_do_not_change_the_generated_item_structure() { + // Method names of a service type reach the output only as string literal *contents*, so names + // needing escapes must produce exactly the same items as plain ones. Any difference means a + // name left its literal and was parsed as Rust. + assert_eq!( + item_shape(&compile_did(NEEDS_ESCAPING)), + item_shape(&compile_did(PLAIN)) + ); +} + +#[test] +fn service_method_names_are_escaped_not_dropped() { + let content = compile_did(NEEDS_ESCAPING); + // The names survive verbatim, in escaped form: `define_service!` receives the original Candid + // method name as the *value* of a well-formed Rust string literal. + for expected in [ + r#""quote\" : F::ty() }); const marker: u32 = ({ 0 //""#, + r#""backslash\\""#, + r#""newline\nand carriage return\r""#, + r#""tab\tand semicolon;""#, + r#""comment markers // and /* */""#, + r#""braces { } and parens ( )""#, + ] { + assert!( + content.contains(expected), + "expected escaped method name {expected} in:\n{content}" + ); + } +} From 8b85aa3b6706a4150e3c0bd59a4eea1b718c12fb Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Fri, 14 Aug 2026 09:26:20 -0400 Subject: [PATCH 2/2] chore: update date --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95fdf0007..7ee12ca4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 2026-08-13 +## 2026-08-14 ### candid_parser 0.4.1