Skip to content

fix(candid_parser): escape service type method names in the Rust binding + release candid_parser 0.4.1 - #759

Merged
lwshang merged 2 commits into
masterfrom
fix/rust-binding-escape-service-method-names
Aug 14, 2026
Merged

fix(candid_parser): escape service type method names in the Rust binding + release candid_parser 0.4.1#759
lwshang merged 2 commits into
masterfrom
fix/rust-binding-escape-service-method-names

Conversation

@lwshang

@lwshang lwshang commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Patch release. The Rust binding emitted the method names of a service type into a 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, which already escaped. See CHANGELOG.

Output is byte-identical for names that are ordinary identifiers, so no existing goldenfile changed.

Tests: a new service_method_escape.did asset covers the characters that need escaping across all backends, and tests/test_rust_bindings.rs asserts the generated Rust has the same item structure as an equivalent service with plain names. Both fail on master.

The JS/TS bindings needed no change — they route names through candid::pretty::utils::quote_ident, which already escapes.

🤖 Generated with Claude Code

…ing + 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) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a Rust-binding code generation correctness/security issue by properly escaping service type method names before emitting them into Rust string literals, and ships it as a candid_parser patch release.

Changes:

  • Escape service type method names in the Rust binding via escape_debug() so generated Rust stays syntactically valid and non-injectable.
  • Add a dedicated .did asset plus structural Rust tests (via syn) to ensure escaping happens and output shape is unaffected.
  • Release bump to candid_parser 0.4.1 with corresponding lockfile and changelog updates.

Reviewed changes

Copilot reviewed 9 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
rust/candid_parser/src/bindings/rust.rs Escapes service type method names before embedding them into Rust string literals.
rust/candid_parser/tests/test_rust_bindings.rs New structural tests validating generated Rust remains well-formed and escaped names are preserved.
rust/candid_parser/tests/parse_type.rs Adds the new escape-focused asset to the Motoko-backend “expected error” list.
rust/candid_parser/tests/assets/service_method_escape.did New test asset covering method names requiring escaping (quotes, backslashes, control chars, comment markers).
rust/candid_parser/tests/assets/ok/service_method_escape.rs New golden output for Rust backend demonstrating correct escaping in define_service!.
rust/candid_parser/tests/assets/ok/service_method_escape.js New golden output for JS backend for the same asset.
rust/candid_parser/tests/assets/ok/service_method_escape.did New golden pretty-printed .did output for the asset.
rust/candid_parser/tests/assets/ok/service_method_escape.d.ts New golden output for TS declarations backend for the asset.
rust/candid_parser/Cargo.toml Bumps version to 0.4.1 and adds syn as a dev-dependency for the new tests.
CHANGELOG.md Documents the candid_parser 0.4.1 fix and its security implications.
Cargo.lock Updates lockfile entries for candid_parser 0.4.1 and adds syn to the dependency graph.
rust/bench/Cargo.lock Updates bench lockfile to use candid_parser 0.4.1.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@lwshang
lwshang marked this pull request as ready for review August 13, 2026 17:34
@lwshang
lwshang requested a review from a team as a code owner August 13, 2026 17:34
@zeropath-ai

zeropath-ai Bot commented Aug 13, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 8b85aa3.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► rust/candid_parser/src/bindings/rust.rs
     Implement escaping of method names in Rust bindings
► rust/candid_parser/Cargo.toml
     Update version and dependencies for new escaping feature
► rust/candid_parser/tests/assets/ok/service_method_escape.did
     Add test service with escaped method names
► rust/candid_parser/tests/assets/ok/service_method_escape.js
     Add JS binding for escaped method names
► rust/candid_parser/tests/assets/ok/service_method_escape.rs
     Add Rust binding for escaped method names
► rust/candid_parser/tests/assets/ok/service_method_escape.d.ts
     Add TypeScript declarations for escaped method names
► rust/candid_parser/tests/assets/service_method_escape.did
     Add DID for escaped method names
► rust/candid_parser/tests/assets/service_method_escape.js
     Add JavaScript test asset for escaped names
► rust/candid_parser/tests/parse_type.rs
     Include service_method_escape.did in error checks
► rust/candid_parser/tests/test_rust_bindings.rs
     Add tests validating escaped method names do not change item structure

@lwshang
lwshang merged commit 7cc578c into master Aug 14, 2026
18 checks passed
@lwshang
lwshang deleted the fix/rust-binding-escape-service-method-names branch August 14, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants