Skip to content

decode: limit unknown fields to bound memory amplification (owned, dynamic, and view paths) - #184

Merged
iainmcgin merged 5 commits into
mainfrom
fix/decode-unknown-field-limit
Jun 11, 2026
Merged

decode: limit unknown fields to bound memory amplification (owned, dynamic, and view paths)#184
iainmcgin merged 5 commits into
mainfrom
fix/decode-unknown-field-limit

Conversation

@iainmcgin

@iainmcgin iainmcgin commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

#186 (the view-path half, originally stacked on this branch) has been merged into this PR, so it now covers owned decoding, DynamicMessage, and zero-copy views.

What this does

Bounds the number of unknown fields the decoder will materialize in a single decode call, across every decode path. Unknown wire data can occupy ~20× more memory decoded than encoded — every 2-byte unknown varint field becomes a ~40-byte UnknownField — so a small, fully-valid payload of minimal unknown fields could force heap allocation far in excess of its own size. with_max_message_size only caps input length, so it does not bound this. On the view path, this PR additionally makes unknown-field storage dramatically cheaper via span coalescing, and fixes two defects at the view→owned conversion boundary: silent unknown-field loss and the decode-time limit being discarded.

What changed — owned path

  • New DecodeContext<'_> (remaining recursion depth + a shared unknown-field allowance in a borrowed Cell<usize>) replaces the bare depth: u32 threaded through the decode path: Message::{merge, merge_field, merge_to_limit, merge_group, merge_length_delimited}, encoding::decode_unknown_field, and message_set::merge_item. Breaking — code generated by earlier releases must be regenerated. Callers of the convenience methods (decode, decode_from_slice, merge_from_slice, DecodeOptions) are unaffected.
  • Every materialized unknown field (including nested group members and MessageSet items) consumes one slot; exhaustion fails decoding with the new DecodeError::UnknownFieldLimitExceeded.
  • Default: 1,000,000 fields per decode (DEFAULT_UNKNOWN_FIELD_LIMIT, ~40 MB max slot overhead), applied at every decode entry point including DynamicMessage; tune with DecodeOptions::with_unknown_field_limit.
  • Unknown length-delimited payload bytes are deliberately not counted: the decoder only allocates them after the sender has delivered the bytes, so they are bounded by input size (governed by with_max_message_size). The limit is a field count rather than a byte budget so behavior is identical across 32-bit and 64-bit targets.

What changed — view path

  • Span coalescing. Generated view decode records unknown fields via UnknownFieldsView::push_record(tail, span_len, ctx). When a record begins exactly where the previous span ended, the span is extended in place by re-slicing the stored input-buffer tail — deliberately not by widening the narrowed span reference, which would be provenance-unsound; the implementation needs no unsafe. A contiguous run of unknown fields of any length costs one Vec slot and re-encodes byte-identically.
  • Limit enforcement in view decode. Each new span — one per contiguous unknown run — consumes one slot of the same unknown-field allowance the owned path uses. The view decode path threads DecodeContext: MessageView::decode_view_with_limit(buf, depth)decode_view_with_ctx(buf, ctx), generated _decode_depth_decode_ctx, and DecodeOptions::decode_view now honors with_unknown_field_limit (previously ignored by views).
  • Fallible conversion, no silent data loss. MessageView::to_owned_message / to_owned_from_source (and the OwnedView wrapper) now return Result<Owned, DecodeError>. Generated conversions previously did to_owned().unwrap_or_default(), silently dropping every unknown field on error; codegen now propagates the Result through nested message, repeated, map, and oneof conversions.
  • Decode-time limit carries through conversion. UnknownFieldsView captures the allowance remaining at its first record; to_owned re-materializes under that allowance, one owned UnknownField per record. A flood decoded under a tight limit now fails at conversion with UnknownFieldLimitExceeded instead of silently losing the fields. Manual views (push_raw) fall back to the default limit.

Housekeeping

  • Checked-in generated code (WKTs, bootstrap descriptor types, examples) regenerated; conformance Dockerfile builder bumped rust:1.85-slimrust:1.87-slim to match the workspace MSRV.

How we know it works

  • Owned-path regression tests: buffa-types/tests/decode_unknown_field_limit.rs (flat flood, group nesting, limit sharing across siblings/groups, exact-count accounting, payload-not-counted, knob raise/lower) plus unit tests in encoding.rs.
  • View-path unit tests (push_record adjacency/limit/push_raw interaction, allowance capture, default fallback, multi-record to_owned, conversion failure under tight allowance) and integration tests (contiguous flood coalesces to one span under limit=1, fails at to_owned_message under that allowance, round-trips byte-identically under the default; interleaved runs counted exactly; group payload is one span).
  • 2000 workspace tests pass on the combined branch; clippy/fmt/markdownlint clean; all 12 conformance suites pass (via-view, view-json, via-vtable included); decode benchmarks show no regression beyond run-to-run noise.

After merge

Lands in the 0.8.0 release line together with a new protoc/BSR plugin release, since regenerated code is required.

Unknown wire data can occupy ~20x more memory decoded than encoded:
every 2-byte unknown varint field materializes a ~40-byte UnknownField,
so a 64 MiB payload of minimal unknown fields (flat or group-nested)
could force over 1 GiB of heap. This is not bounded by
with_max_message_size, which only caps input length.

Replace the bare `depth: u32` threaded through the decode path with
DecodeContext<'_>, a Copy struct carrying the remaining recursion depth
plus a shared unknown-field allowance (a borrowed Cell<usize>). Every
materialized unknown field -- including nested group members and
MessageSet items -- consumes one slot of the allowance before being
decoded; exhaustion fails decoding with the new
DecodeError::UnknownFieldLimitExceeded.

The limit counts fields rather than bytes: the amplification vector is
purely per-field slot overhead, while unknown length-delimited payload
bytes are already bounded by the input size (the decoder refuses to
allocate until the sender has delivered the bytes), which
with_max_message_size governs. A count is also platform-deterministic,
where a byte budget keyed on size_of::<UnknownField>() would admit
different field counts on 32-bit and 64-bit targets.

The default limit is 1,000,000 fields per decode
(DEFAULT_UNKNOWN_FIELD_LIMIT), capping slot overhead at ~40 MB, and
applies to all decode entry points including the trait-level
convenience methods; DecodeOptions::with_unknown_field_limit tunes it.
The limit also covers DynamicMessage's reflective decoder. Zero-copy
views store unknown fields as borrowed spans and are unaffected.

Breaking change: Message::{merge, merge_field, merge_to_limit,
merge_group, merge_length_delimited}, encoding::decode_unknown_field,
and message_set::merge_item now take DecodeContext<'_> instead of
depth: u32. Code generated by earlier releases must be regenerated;
callers of the convenience methods and DecodeOptions are unaffected.
Checked-in generated code (WKTs, bootstrap descriptor types, examples)
is regenerated in this commit.

Also bumps the conformance Dockerfile builder image from rust:1.85-slim
to rust:1.87-slim to match the workspace MSRV; the local task conformance
flow could not build the workspace at 1.85. All 12 conformance suites
pass with no unexpected failures.
@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

asacamano
asacamano previously approved these changes Jun 11, 2026

@asacamano asacamano 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.

Nice

@iainmcgin
iainmcgin force-pushed the fix/decode-unknown-field-limit branch from 46fe128 to 7302d2c Compare June 11, 2026 16:01
asacamano
asacamano previously approved these changes Jun 11, 2026
…pans (#186)

> **Stacked on #184** — review/merge that first; this PR's base branch
is `fix/decode-unknown-field-limit`.

## What this does

Extends the unknown-field decode limit from #184 to the zero-copy view
path, makes view storage of unknown fields dramatically cheaper via span
coalescing, and fixes two defects at the view→owned conversion boundary:
silent unknown-field loss and the decode-time limit being discarded.

## What changed

**Span coalescing.** Generated view decode records unknown fields via
`UnknownFieldsView::push_record(tail, span_len, ctx)`. When a record
begins exactly where the previous span ended, the span is extended in
place by re-slicing the stored input-buffer tail — deliberately *not* by
widening the narrowed span reference, which would be provenance-unsound;
the implementation needs no `unsafe`. A contiguous run of unknown fields
of any length costs one `Vec` slot and re-encodes byte-identically.

**Limit enforcement in view decode.** Each new span — one per contiguous
unknown run — consumes one slot of the same unknown-field allowance the
owned path uses. The view decode path threads `DecodeContext`:
`MessageView::decode_view_with_limit(buf, depth)` →
`decode_view_with_ctx(buf, ctx)`, generated `_decode_depth` →
`_decode_ctx`, and `DecodeOptions::decode_view` now honors
`with_unknown_field_limit` (previously ignored by views).

**Fallible conversion, no silent data loss.**
`MessageView::to_owned_message` / `to_owned_from_source` (and the
`OwnedView` wrapper) now return `Result<Owned, DecodeError>`. Generated
conversions previously did `to_owned().unwrap_or_default()`, silently
dropping every unknown field on error; codegen now propagates the
`Result` through nested message, repeated, map, and oneof conversions.

**Decode-time limit carries through conversion.** `UnknownFieldsView`
captures the allowance remaining at its first record; `to_owned`
re-materializes under that allowance, one owned `UnknownField` per
record. A flood decoded under a tight limit (one coalesced span) now
fails at conversion with `UnknownFieldLimitExceeded` instead of silently
losing the fields. Manual views (`push_raw`) fall back to the default
limit.

All three changes are **breaking** for code generated by earlier
releases (regeneration required), consistent with #184; checked-in
generated code is regenerated here.

## How we know it works

- Unit tests: `push_record` adjacency/limit/`push_raw` interaction,
allowance capture, default fallback, multi-record `to_owned`, conversion
failure under tight allowance.
- Integration tests: contiguous flood coalesces to one span under
`limit=1`, then fails at `to_owned_message` under that allowance and
round-trips byte-identically under the default; interleaved runs counted
exactly; group payload is one span.
- 1955 workspace tests; clippy/fmt/markdownlint clean; all 12
conformance suites pass (via-view, view-json, via-vtable included).
@iainmcgin iainmcgin changed the title decode: limit unknown fields per decode to bound memory amplification decode: limit unknown fields to bound memory amplification (owned, dynamic, and view paths) Jun 11, 2026
@iainmcgin
iainmcgin added this pull request to the merge queue Jun 11, 2026
Merged via the queue into main with commit 278fa43 Jun 11, 2026
7 checks passed
@iainmcgin
iainmcgin deleted the fix/decode-unknown-field-limit branch June 11, 2026 17:10
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants