JSON decoding ignores DecodeOptions, so element-memory and size limits bound only the protobuf codec

[iainmcgin]

DecodeOptions is not consulted anywhere on the JSON path, so a message decoded from JSON is bounded by none of the limits that bound the same message decoded from protobuf.

The two paths

The protobuf path threads a DecodeContext carrying the remaining budgets through every merge call. Both DecodeOptions::decode_from_slice and DecodeOptions::decode_view build one:

let limit = core::cell::Cell::new(self.unknown_field_limit);
let elem_budget = core::cell::Cell::new(self.element_memory_limit);
V::decode_view_with_ctx(
    buf,
    DecodeContext::new(self.recursion_limit, &limit).with_element_memory(&elem_budget),
)

The JSON path has no equivalent. Generated messages get #[derive(::serde::Serialize, ::serde::Deserialize)] (buffa-codegen/src/message.rs:517-522), with a hand-written Deserialize where derive cannot express the shape (oneofs, extension keys). The caller invokes them through their own serde_json::from_str, which is the idiom the guide documents (docs/guide.md:1434). Grepping recursion_limit|max_message_size|unknown_field_limit|element_memory_limit|DecodeContext|DecodeOptions across buffa/src/json.rs, buffa/src/json_helpers.rs, buffa/src/json_helpers/ and buffa-descriptor/src/reflect/json.rs returns nothing. Every terminal method on DecodeOptions is binary-wire only.

Why this is more than a doc gap

element_memory_limit was added in 0.9.0 precisely because element footprint is not proportional to encoded size. The 0.9.0 changelog entry makes the case in wire terms:

an empty repeated message element is 2 bytes on the wire and size_of::<T>() in the Vec it lands in — measured at 256 bytes for a message of a few Vec/String fields, a 128x ratio, so 4 MiB of them forced ~512 MiB

None of that reasoning is protobuf-specific. Against the same repeated Payload items field, {}, is three JSON bytes for the same ~256 bytes of Vec footprint — about 85x, versus 128x on the wire. The JSON encoding is within a small constant of the exact case the limit was introduced to stop, and has no ceiling at all. "" into repeated string and "a":{} into map<string, Payload> amplify by the same route.

serde_json's own 128-deep recursion default is the one accidental substitute, and it is a poor one: it is looser than buffa's own RECURSION_LIMIT of 100, it is not reachable through DecodeOptions, and it disappears entirely for a non-serde_json Deserializer — this repo already ships one in buffa-yaml.

Documentation that currently reads the wrong way

• DecodeOptions' rustdoc says "Use this to set custom recursion depth limits or maximum message sizes when decoding from untrusted input" without naming a codec. With json enabled, that is a reasonable thing to believe and it is not true.
• The 0.9.0 changelog says "The owned, view and reflective (DynamicMessage) decoders are all bounded." True of the binary decoders; DynamicMessage::from_json is not bounded.
• The guide's decode-options table (docs/guide.md:1050-1056) does not list with_element_memory_limit at all, and the JSON section says nothing about limits.

What a fix has to get past, and what already exists

buffa has already hit this constraint and documented it, in buffa/src/json.rs:

Serde's Deserialize trait has no context parameter, so runtime options must be passed through ambient state.

That module exists because of it: JsonParseOptions is carried by a thread-local on std (with_json_parse_options) and a leaked AtomicPtr global on no_std (set_global_json_parse_options). So there is a working precedent for getting per-call state into the derive-generated deserializers, which makes this cheaper than it first looks.

The two surfaces differ a lot:

Reflective JSON is a contained, additive fix. DynamicMessage::from_json owns its serde_json::Deserializer and already goes through a real seed, DynamicMessageSeed, whose doc says new parse options belong on it as builder-style setters. A budget setter plus charging in the seq/map visitors covers it.

Generated-message JSON is the hard half, because buffa never sees the call. Options, in increasing cost:

1. Extend JsonParseOptions with the limit fields and charge from the container visitors in buffa/src/json_helpers.rs (the Vec::with_capacity(clamp_size_hint(..)) sites, the map visitors, DefaultDeserializeSeed/NullableDeserializeSeed). Reuses machinery that already exists. Inherits its warts: ambient state, and the no_std variant is process-wide set-once.
2. Add a buffa-owned entry point — DecodeOptions::decode_json_from_slice::<M> — that installs the budget for the duration of the call and caps input size first. Additive, but opt-in: the documented idiom stays unbounded, so most callers stay unprotected.
3. Move generated JSON off serde-derive to a seed-based codec, as the reflective path already uses. This is the rewrite. It changes the public shape of generated code, and it would also remove the serde_json::Value buffering the extension path does today.

I do not have a strong view between these, and the choice looks like a real design decision rather than an obvious one. What I would argue is that doing nothing is not tenable while element_memory_limit is presented as codec-independent — at minimum DecodeOptions, with_element_memory_limit and the guide should say which codec they bound.

One thing that does not carry over, for scope: unknown-field amplification. __buffa_unknown_fields is #[serde(skip)] for messages without extension ranges, and unknown JSON keys are dropped rather than materialized.

Where this came from

Found while wiring the 0.9 element-memory budget through to user-facing configuration in connect-rust (connectrpc/connect-rust#235 for the server, #236 for the client). Both sides normalize JSON to protobuf bytes and decode that under the caller's limits, which is enough to make the knob behave the same on both codecs, but the serde_json parse ahead of it is unbounded and only the second materialization is charged.

[harshitwandhare]

Three things I checked against main before commenting, one of which changes what is left here.

The documentation half is already done. The issue's fallback ask is that DecodeOptions, with_element_memory_limit and the guide say which codec they bound. On main all three do:

• DecodeOptions now carries a # Scope: the protobuf binary codec section (buffa/src/message.rs:1121-1129). It says the limits bound the binary decoders and only those, that JSON goes straight into the generated Deserialize impls which never receive a DecodeOptions, and that JSON input needs a bound imposed by the caller. Added in fe5635f (v0.9.1, 2026-07-23), two days after this issue was filed.
• The guide's decode-options table does list .with_element_memory_limit(n) (docs/guide.md:1076).
• docs/guide.md:1100 states "None of them applies to JSON" and gives the amplification argument, and 1104 points back here as the tracking issue.

So if the doc floor was the fallback position, it is met, and what remains is only the code question.

There is an in-tree precedent for options 1 and 2 that the issue does not cite: textproto. TextDecoder carries its own element-memory bound (buffa/src/text/decoder.rs:68), exposed as buffa::text::decode_from_str_with_element_memory_limit (buffa/src/text/mod.rs:161). The guide gives the reasoning at 1102: the parser needs the same bound and carries its own "because DecodeContext never reaches it". That is the same constraint the JSON path has, and the same answer options 1 and 2 propose, already shipped for another non-binary codec. One non-binary codec already honours the element-memory limit, so the choice here is which mechanism to reuse rather than whether to invent one.

The buffa-yaml aside holds, with one caveat. buffa-yaml has no depth, recursion or budget guard of any kind. Grepping depth|recursion|limit|budget across its 674 lines returns nothing, and all three entry points hand straight to serde_norway (buffa-yaml/src/decode.rs:21,34,48). The caveat is that the crate is publish = false and unreleased (phase 1 of #101), so this is not a live exposure and I would not present it as one. It does make the point about serde_json's 128 being an accidental substitute concrete in this repo rather than hypothetical, and whichever shape wins here is cheaper to land before buffa-yaml ships than after.

I do not have a view on 1 vs 2 vs 3 either. The textproto precedent is the one piece of evidence I would weigh, since picking 1 or 2 would be consistent with how the repo already solved this same problem once.

All of the above is from reading main at 2827cbf, not from running anything.