Jens-G opened a new pull request, #3760:
URL: https://github.com/apache/thrift/pull/3760

   [THRIFT-6164](https://issues.apache.org/jira/browse/THRIFT-6164)
   
   `thrift_protocol:skip/2` walks type ids taken off the wire rather than the 
types the IDL declared, so the peer chooses both the shape of the nesting and 
how deep it goes. `skip/2` and the four skip loops — `skip_struct_loop`, 
`skip_map_loop`, `skip_set_loop`, `skip_list_loop` — call one another with 
nothing carrying a depth.
   
   A struct level costs the sender three bytes. 200 levels of nesting is 801 
bytes on the wire.
   
   ### Change
   
   `skip/2` seeds `DEFAULT_RECURSION_DEPTH`, 64 — the same value `lib/py` and 
`lib/cpp` already use — and each level of nesting spends one. `skip/3` is 
exported so a caller who knows what its own schema can contain can ask for a 
lower ceiling. Exceeding the ceiling raises `error({protocol_error, 
max_skip_depth_exceeded})`, matching the binding's existing `negative_size` 
idiom.
   
   ### Two things this deliberately does not do
   
   - **No guard on the typed read path.** `read/2`'s `struct`, `list`, `map` 
and `set` clauses recurse on a type taken from the generated `struct_info`, 
never from the wire, so their depth is fixed by the IDL and a peer cannot drive 
it. That is why the allowance starts fresh at `skip/2` rather than being 
threaded down from the reader.
   - **No charge for element counts.** The three container loops recurse once 
per element in tail position, which costs no stack. Charging them there would 
refuse large flat containers while leaving the dimension that does consume 
stack wide open.
   
   ### Tests
   
   Seven over a memory buffer: one per container type, one pinning the wire 
cost of 200 struct levels at 801 bytes, one checking ordinary nesting still 
skips, and one checking an explicit ceiling passed to `skip/3` is honoured. 
Five of the seven fail against the unmodified library.
   
   `rebar3 eunit` on OTP 25: 324 → 331 tests, 0 failures. `rebar3 xref` clean, 
`rebar3 fmt --check` clean apart from the pre-existing 
`src/thrift_binary_protocol.erl` warning.
   
   ### Not the last gap
   
   `lib/d`'s `skip()` (`lib/d/src/thrift/protocol/base.d:262`) has no depth 
parameter either — worth knowing so this is not mistaken for closing the sweep. 
Separate change, not in this PR.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to