Jens-G opened a new pull request, #3766: URL: https://github.com/apache/thrift/pull/3766
`skip()` in `lib/d/src/thrift/protocol/base.d` walks type ids taken off the wire rather than the types the IDL declared, so the peer picks both the shape of the nesting and how deep it goes. Every branch that can nest — struct, list, map, set — calls `skip()` again, and nothing carries a depth. A struct level costs the sender four bytes and a list level five, so 200 levels is 797 bytes as a struct chain and 1000 as a list chain. ### Why the existing counter did not cover it D has had a recursion depth counter since THRIFT-6053 (shipped in 0.24.0), but `skip()` could not reach it: it was module-private to `thrift.codegen.base`, and that module imports `thrift.protocol.base` rather than the other way round. Only `readStruct`/`writeStruct` were counted. That matters because `skip()` sits on the pre-dispatch path. The generated processor skips the whole argument struct *before* it knows the method — once on an unexpected message type and once on an unknown method name (`codegen/processor.d:115`, `:126`). It is also reached from the generated readers for a field the struct does not declare or whose type does not match (`codegen/base.d:711`, `:766`), and from `TApplicationException.read()`. ### The change `currentRecursionDepth_` and `DEFAULT_MAX_RECURSION_DEPTH` move into `thrift.protocol.base` as `package(thrift)`, gain `incrementRecursionDepth()` / `decrementRecursionDepth()` helpers, and `skip()` opens with the pair. Past 64 levels it throws `TProtocolException` with `DEPTH_LIMIT`. `readStruct`/`writeStruct` now call the same two helpers instead of open-coding the identical five lines twice — the counter, its value and their behaviour are unchanged. **One budget, not two.** `skip()` is called from inside `readStruct`, so a payload can alternate between the two paths; charging them separately would let it reach twice the ceiling. It is also what the other bindings do — C++ opens `skip()` with `TInputRecursionTracker`, the same tracker its generated readers use, and Haxe, netstd, Delphi, Lua, Perl, PHP, Smalltalk and OCaml all wrap `skip()` in the protocol's own increment/decrement pair. ### Cross-binding check Every binding's `skip()` was read for this. Bounded already: C++ (`TProtocol.h:703`), c_glib (`recursion_depth` parameter), Dart, Delphi, Erlang (THRIFT-6164), Go, Haxe, Java, JavaME, browser JS (THRIFT-6014), Lua, netstd, Node.js, OCaml, Perl, PHP, Python incl. the C extension, Ruby (THRIFT-6013), Rust, Smalltalk. Kotlin uses the Java library; `lib/cl` carries no protocol implementation of its own. **D was the last one with no bound at all.** ### Tests Five checks in a helper instantiated for the binary, compact and JSON protocols: a struct chain and a list chain at exactly the limit are still skipped, one level deeper is refused, and a struct 191 fields wide but one level deep still skips — which only holds if the counter unwinds per field. A sixth, in the codegen tests, drives an over-deep chain in as an undeclared field so it is skipped from inside an already-counted `readStruct`: `limit - 1` fits, `limit` does not. Two comments in `codegen/base.d` that described `skip()` as "the (separate, unbounded) skip() path" are now wrong, and are corrected. Eight unittest binaries — binary, compact, json and codegen/base, each built debug and release the way `lib/d/Makefile.am` builds them — fail against the unmodified library. With the change the full set of 43 modules passes in both modes, 86 binaries, no warnings under `-w -wi`. **No CI job builds D**, so that local run (dmd 2.087 in the project's `thrift:jammy` image) is the only coverage this gets. 🤖 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]
