fix: Bound the type-table header by its byte length by lwshang · Pull Request #770 · dfinity/candid

lwshang · 2026-09-23T07:11:54Z

What this changes

Adds a bound on the byte length of the type-table header, defaulting to 64 KiB and configurable with DecoderConfig::set_max_header_len.

A header is a type description, so its size is a property of the interface, not of the payload. But the counts it declares — the argument count, a record or variant's field count, a function type's argument and result counts, a service type's method count — were free to describe millions of elements.

Because every element a header introduces occupies at least one byte, one bound on its length covers all of them at once. There is no per-site number to choose, which is what makes fixed per-count caps hard to size safely.

The header is parsed from a bounded prefix of the input, so an oversized header costs the bound rather than its declared size. The value section reads from the full input afterwards and is unaffected — a message may still carry a payload of any size its quota allows.

Why a byte bound rather than the decoding quota

The first revision of this PR derived the bound from decoding_quota. Two problems, both of which this avoids:

It coupled two unrelated things. A larger permitted payload silently loosened the header limit: a 10 MB module upload needs a quota that also admits a ~3 M element header. "Why did uploading a bigger module raise my header limit?" is not a question the config should provoke.
It did nothing unless a decoding quota was set, and in practice callers set skipping_quota and leave decoding_quota unset.

This version needs no quota at all. It also sits naturally beside max_type_len, which bounds a different dimension — the number of type-table entries, whatever their size — and follows the same existing pattern of a structural header bound that is on by default.

Choosing 64 KiB

Measured worst-case header per interface (largest method's argument types, encoded):

interface	methods	largest header
NNS governance	33	2,032 B (`manage_neuron`)
SNS governance	24	1,461 B
registry	57	583 B
management canister (`ic.did`)	37	214 B
ckBTC minter	17	78 B
ckETH minter	15	50 B

64 KiB is ~32x the largest header in the IC's own interfaces. Across this entire workspace the default rejects exactly one message: the deliberately oversized one in skip_optional_queue.rs, which now opts into a larger bound so it still reaches the code it tests.

Note the bound can be tighter than max_type_len permits in principle — a legal 10,000-entry table of 8-field records is ~316 KB. That is intentional: the two bound different dimensions, and no real interface comes close.

Effect

Time to reject an oversized header, release build, no quota configured:

declared args	message	default (64 KiB)	bound raised
1,000,000	1.0 MB	0.85 ms	51.0 ms
4,000,000	4.0 MB	0.28 ms	121.9 ms
8,000,000	8.0 MB	0.23 ms	249.4 ms

Constant rather than linear, and it applies without the caller configuring anything.

Compatibility

This is a new default-on bound, so it can reject messages that are valid today — headers above 64 KiB. That is the same class of decision as the existing MAX_TYPE_TABLE_LEN = 10_000, and the measurements above put real interfaces ~32x below it. set_max_header_len raises or lowers it.

set_max_type_len, both quotas, and the wire format are unchanged.

Tests

rust/candid/tests/header_len.rs:

an oversized header is rejected under the default, for args, record fields and variant fields;
one bound reaches every declared count, including service methods and function args;
the bound is on the header, not the payload — a 2 MB value section decodes fine under a 64-byte header bound;
the boundary is exact at several limits;
a realistic management-canister-shaped record is unaffected;
a larger bound can be opted into.

Full workspace passes with --all-features and --no-default-features; clippy clean. The ic_principal doctest failure at rust/ic_principal/src/lib.rs:55 is pre-existing on master.

Release

No version bump; the changelog entry sits under ## Unreleased.

🤖 Generated with Claude Code

The header's wire-declared element counts each drive a `count`-sized parse: the argument count, a record or variant's field count, a function type's argument and result counts, and a service type's method count. The decoding quota was charged for the header only once that parse had finished, so it reported the header's cost rather than bounding it -- a header declaring many elements was parsed in full and only then found to be over budget. Thread the caller's budget into the parse and check each count as it is read. An element occupies at least one byte and the header is charged 4 cost units per byte, so a count above `quota / 4` describes a header the budget cannot pay for, whatever the rest of the message contains. The bound agrees with the charge it anticipates, so no header the charge would have accepted is rejected; it only moves the same decision ahead of reading the bytes. Callers that set no decoding quota keep unbounded counts, as before. `Table.len` already carries its own `max_type_len` bound and is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Replaces the quota-derived count bound from the previous commit. A header is a type description: its size is a property of the interface, not of the payload. Deriving its limit from the decoding quota tied those two together, so a larger permitted payload silently loosened the header limit -- a 10 MB module upload would need a quota that admits a ~3M element header. It also did nothing unless a decoding quota was set. Bound the header's byte length directly instead, defaulting to 64 KiB and configurable with `set_max_header_len`. Every element a header introduces occupies at least one byte, so one bound covers the argument count, a record or variant's field count, a function type's argument and result counts, and a service type's method count at once -- no per-site number to choose, which is what made fixed per-count caps hard to size. The header is parsed from a bounded prefix of the input, so an oversized header costs the bound rather than its declared size. The value section reads from the full input afterwards and is unaffected: a message may still carry a payload of any size its quota allows. This sits alongside `max_type_len`, which bounds a different dimension -- the number of type-table entries, whatever their size -- and follows the same pattern of a structural header bound that is on by default. For scale, the largest header among the IC's own interfaces is ~2 KB (NNS governance `manage_neuron`); the management canister's largest is 214 B. The only message in this workspace the default rejects is the deliberately oversized one in `skip_optional_queue.rs`, which now opts into a larger bound so it still reaches the code it is testing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`clippy::same_item_push` fires on pushing a constant byte in a loop, and CI lints the test targets (`cargo clippy --features all --tests`). Build the service-method and function-argument vectors with extend_from_slice and repeat().take() instead. No change to what the test asserts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Copilot review overview

🟡 Changes recommended

Wire-declared vector counts can still trigger unbounded allocations before the bounded reader rejects the header.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 1

Open (2)

Declared vector counts can trigger unbounded allocations · New
Large malformed messages receive misleading size-limit errors · New

What changed in this PR

Adds a configurable 64 KiB default limit for Candid type-table headers.

Changes:

Parses headers through a bounded input prefix.
Adds DecoderConfig::set_max_header_len.
Adds boundary, compatibility, payload, and oversized-header tests.

File	Description
`rust/candid/src/de.rs`	Adds and enforces header-length configuration.
`rust/candid/src/binary_parser.rs`	Defines the default 64 KiB limit.
`rust/candid/tests/header_len.rs`	Tests header-limit behavior.
`rust/candid/tests/skip_optional_queue.rs`	Raises the limit for an existing stress test.
`CHANGELOG.md`	Documents the new bound.

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

Addresses review feedback on the error reported when a header parse fails inside the bounded prefix. Any failure was rewritten as "header exceeds the limit" whenever the message as a whole was larger than the bound, so a malformed but short header in a large message was misdiagnosed as oversized: bad magic in a 100 KB message reported a size violation instead of "Unexpected bytes at byte offset 0", and an over-long type table lost its own "type table size exceeded". Report the bound only when the failure was at or beyond the prefix boundary, and otherwise keep the original error. binrw rewinds the reader on error, so the offset comes from the error rather than the cursor, and the whole error tree is walked: a failure inside an enum variant is reported as `EnumErrors` positioned at the start of the enum, not at the offset the variant stopped on, which is how a wide record's type-table entry ends up looking like a failure at offset 5. Two regression tests: malformed headers keep their own diagnosis inside an over-bound message, and a count far larger than the input is still charged only the bound. The latter also pins that nothing is allocated for a declared count -- binrw reserves up front only for `Vec<u8>`, which `read_len_prefixed` already handles, while the header's element vectors collect an iterator whose size-hint lower bound is 0. Were that to change, the test would exhaust memory rather than fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Copilot review overview

🟡 Changes recommended

Critical test-compilation and diagnostic-allocation issues, along with boundary error-classification gaps, remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 · 1

Open (3)

Bound diagnostics for oversized header rejection · New
Fix no-default-features test dependency on serde_bytes · New
Detect bounded short reads at the header limit · New

Resolved since last review (2)

Declared vector counts can trigger unbounded allocations
Large malformed messages receive misleading size-limit errors

Second round of review feedback. The offset of a parse failure is not a reliable signal that a header ran past its bound. A length-prefixed method name or blob, and a multi-byte LEB128, are all reported at the offset they *start* at, which can be far short of the boundary they cross: a service method name declared longer than the bound surfaced at offset 10, and a future type's blob at offset 9, so both were reported as ordinary short reads rather than as the header limit. Detect the condition itself instead. Reads that stop because the reader ran out of input now carry a `Truncated` marker, distinguished from a LEB128 overflow, which is malformed rather than truncated. Inside a bounded prefix that marker is exactly the signal that the header needed more than the bound, and it holds wherever the read gave out. `Truncated` renders as the plain field name, so error text is unchanged; `get_binread_labels` learns to read it alongside the `&str` case. Also stop attaching the input dump to the header-limit error. It was still wrapped by the "Cannot parse header" context, which hex-encodes the whole message when full error messages are on, so rejecting an oversized header cost a diagnostic proportional to the payload -- the cost this bound exists to avoid. Rejecting an 8 MB message now takes 0.35 ms rather than 39 ms, matching the constant cost the bound is meant to give, and ordinary parse errors keep their dump. The payload test drops `serde_bytes::ByteBuf` for a plain `Vec<u8>`. The test did compile without default features, because the `candid_parser` dev-dependency pulls candid with `features = ["all"]` and Cargo unifies them, but relying on that is incidental and `Vec<u8>` needs no feature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Copilot review overview

🟢 Approval recommended

No unresolved review issues were identified.

Review effort: Lite
Findings: None

Resolved since last review (3)

Fix no-default-features test dependency on serde_bytes
Bound diagnostics for oversized header rejection
Detect bounded short reads at the header limit

zeropath-ai · 2026-09-24T08:32:56Z

✅ No security or compliance issues detected. Reviewed everything up to 96d361a.

Security Overview

🔎 Scanned files: 6 changed file(s)
🔗 Scan Link: https://zeropath.com/app/repositories/7ddf29dd-76dc-4f78-adfa-47ac8f4643e7?scanId=d6102d77-82a8-47f3-8415-29dd634b9b2e&codeScanTypes=PrScan&tab=issues&detectionTypes=traditional,businessLogic,customRule,IAC,SECRETS,CICD,EOL,uncategorized

Detected Code Changes

Change Type	Relevant files
Enhancement	► rust/candid/src/binary_parser.rs Add DEFAULT_MAX_HEADER_LEN, Truncated type, error handling helpers, and related utilities for header truncation and bounded parsing ► rust/candid/src/de.rs Introduce max_header_len in DecoderConfig, add set_max_header_len, and use bounded header parsing logic in from_bytes and header context ► rust/candid/src/error.rs Improve error reporting to handle Truncated errors from header parsing ► rust/candid/tests/header_len.rs Add comprehensive tests around header length bounding and error reporting ► rust/candid/tests/skip_optional_queue.rs Update test to configure max_header_len for testing bounded header parsing

Review feedback. Drop the measurement figures from the doc comments. They measured another repository at a point in time -- the largest header among the IC's own interfaces, the management canister's largest, the resulting margin -- which will go stale in candid's source with nobody to update it, and does not belong in the public docs of a crate that is not IC-specific. The figures are in the pull request instead; the comments keep the reasoning that does not date: a header's size follows the interface rather than the payload, one byte bound covers every declared count because each element occupies at least one byte, and `max_type_len` bounds a different dimension. `is_truncation` keeps a compressed note on why the failure offset cannot serve as the signal. A length-prefixed field and a multi-byte LEB128 are reported where they start, and binrw reports a failure inside an enum variant at the start of the enum, so the obvious simplification back to a position check is wrong in a way that still passes the common case. That reasoning is pinned by the binrw version rather than by anything external. The changelog entry is two sentences and stays under bug fixes. The bound is set far above any realistic interface, so it is not expected to affect valid traffic, and the entry still says that an over-bound header is now rejected. Net 46 lines of comment to 28. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

marc0olo

LGTM. Tiny wording nit in the changelog: "…is now rejected, which is far above any realistic interface" attaches "which" to the rejection. Maybe: "Headers over the bound, far above any realistic interface, are now rejected; …"

"is now rejected, which is far above any realistic interface" attached the clause to the rejection rather than to the bound. Reviewer's wording. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

## What this changes A diagnostic that names a type now renders it through `elide_large`, which checks the existing `text_size` budget first and substitutes `(type elided)` when the type is over it — so nothing is built for a type whose rendering cost would follow its own width and depth. Types reaching the decoder come from the wire, so the budget applies to every diagnostic, including the verbose form `set_full_error_message(true)` selects, which previously short-circuited it away. Two budgets: `MAX_DIAGNOSTIC_TYPE_LEN` for one type, `MAX_DIAGNOSTIC_LIST_LEN` for a list of them, since a per-element bound leaves a list growing with its length and the type-table dump can hold `max_type_len` entries. This covers every type-naming diagnostic in `de.rs` and `subtype.rs`. The latter builds its own message text and had no budget at all, which is the larger half of the change. With #770's 64 KiB header bound in place this is defence in depth, mainly for callers raising `max_header_len` and for the `candid_parser` paths that have no header bound. ## Public behaviour change `text_size` asserted a service constructor could not reach it. That held for its two original callers, but `elide_large` is reached from the public `subtype` and `equal`, including for a `Class` nested in a record, vector or option. It now returns `Err(())` for one wherever it appears, and such types are elided rather than rendered — the pretty printer has no arm for one either. ## Two pre-existing bugs, surfaced by removing the skip `text_size` subtracted its running total from the remaining budget after each field rather than that field's cost, so the budget drained quadratically and rejected types well under it: an eleven-field ICRC-2-shaped record renders to 287 characters and was judged over a budget of 500. Hashed field ids were also charged 4 however wide they print. Both only showed in terse errors before, because verbose mode skipped the check. Removing that skip would have turned them into elided output for ordinary interfaces, so both are fixed here. ## Compatibility Diagnostics only — no change to what decodes. `subtype.rs` is shared with `candid_parser`, so a `.did` typechecking error naming a very large type elides it too. `binary_parser`'s `Invalid table entry` still renders a raw parse struct with `{t:?}`: its `Debug` is flat and its size follows the header, which carries its own bound. ## Tests `rust/candid/tests/type_diagnostics.rs`, gated on the `value` feature, decoding on a thread given little room: outsized types against `int`, `nat16` and a function reference under both `full_error_message` settings; a service constructor at the top of a type, in a record, in a vector and under an option; a 20,000-element argument list; and, on the other side, an ordinary ICRC-2-shaped record still named in full. ## Release No version bump; the changelog entry joins the existing `## Unreleased` section. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Patch release of `candid` / `candid_derive`, 0.10.36 → 0.10.37. No source changes — it dates the two decoder fixes already on `master`. ## Summary - Bump `candid` and `candid_derive` 0.10.36 → 0.10.37, and the `=` pin between them - Date the `Unreleased` changelog entries as `2026-09-25` / `Candid 0.10.37`, covering: + Bounding the byte length of the type-table header, 64 KiB by default and configurable with `set_max_header_len` (#770) + Bounding the size of a type named in a decoder diagnostic, so its rendering no longer follows the type's own width and depth (#771) - Refresh `Cargo.lock`, `rust/bench/Cargo.lock`, `tools/ui/Cargo.lock` and `rust/candid/fuzz/Cargo.lock` `candid_parser` is unchanged at 0.4.1; its dependency on `candid` is a `0.10.16` floor, so it needs no bump. The lockfile diffs are version bumps only — no dependency churn. ## All four lockfiles this time The 0.10.36 release noted that `rust/candid/fuzz/Cargo.lock` was stale at 0.10.34 and that all four lockfiles should move together as a release-checklist step. This one does that, so `fuzz` goes 0.10.34 → 0.10.37. `tools/ui` patches `candid` to the workspace path, so its lockfile pins the path crate's version and has to move with the bump. `candid-ui-release.yml` builds `didjs` there with `--locked` and triggers on the date tag, so a stale lock would only fail at tag time, after merge. ## Test plan - [x] `cargo check -p candid -p candid_derive -p candid_parser` - [x] `cargo test -p candid --features all` — all suites pass - [x] `cd tools/ui && cargo build --target wasm32-unknown-unknown --profile canister --package didjs --locked` — the release workflow's exact command, succeeds - [x] `cd rust/candid/fuzz && cargo check` - [ ] Reviewer to confirm release scope and changelog date ## After merge 1. Tag `2026-09-25` on `master` — this is what `candid-ui-release.yml` triggers on to publish the `candid_ui` canister wasm 2. Run the **Crates Publish** workflow (`workflow_dispatch`) with the `candid` input checked, which publishes `candid_derive` then `candid` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

lwshang and others added 2 commits September 23, 2026 15:11

lwshang changed the title ~~fix: Bound header parsing by the caller's decoding quota~~ fix: Bound the type-table header by its byte length Sep 23, 2026

lwshang requested a balanced review from Copilot September 24, 2026 07:53

Copilot started reviewing on behalf of lwshang September 24, 2026 07:54 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Comment thread rust/candid/src/de.rs Outdated

Comment thread rust/candid/src/de.rs Outdated

lwshang requested a lite review from Copilot September 24, 2026 08:09

Copilot started reviewing on behalf of lwshang September 24, 2026 08:09 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Comment thread rust/candid/src/de.rs Outdated

Comment thread rust/candid/tests/header_len.rs

Comment thread rust/candid/src/binary_parser.rs Outdated

lwshang requested a lite review from Copilot September 24, 2026 08:28

Copilot started reviewing on behalf of lwshang September 24, 2026 08:28 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

lwshang marked this pull request as ready for review September 24, 2026 08:31

lwshang requested a review from a team as a code owner September 24, 2026 08:31

marc0olo approved these changes Sep 24, 2026

View reviewed changes

docs: Fix a dangling relative clause in the changelog entry

96d361a

"is now rejected, which is far above any realistic interface" attached the clause to the rejection rather than to the bound. Reviewer's wording. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

lwshang merged commit 27440c2 into master Sep 24, 2026
17 checks passed

lwshang deleted the bound-header-parse-by-decoding-quota branch September 24, 2026 10:06

lwshang mentioned this pull request Sep 24, 2026

fix: Bound the size of a type named in a decoder diagnostic #771

Merged

lwshang mentioned this pull request Sep 25, 2026

chore: Release candid 0.10.37 #772

Merged

4 of 5 tasks

Navigation Menu

Repository navigation

fix: Bound the type-table header by its byte length - #770

fix: Bound the type-table header by its byte length#770
lwshang merged 7 commits into
masterfrom
bound-header-parse-by-decoding-quota

lwshang commented Sep 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

zeropath-ai Bot commented Sep 24, 2026 •

edited

Loading

Uh oh!

marc0olo left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

lwshang commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this changes

Why a byte bound rather than the decoding quota

Choosing 64 KiB

Effect

Compatibility

Tests

Release

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

zeropath-ai Bot commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marc0olo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lwshang commented Sep 23, 2026 •

edited

Loading

zeropath-ai Bot commented Sep 24, 2026 •

edited

Loading