THRIFT-6053: Limit struct read/write recursion depth in D library#3558
Merged
Conversation
Client: d D struct serialization runs through the readStruct/writeStruct templates in thrift.codegen.base (the generated read()/write() methods forward to them). These did not bound recursion depth, so a deeply nested message was read or written without a limit. Wrap the readStruct/writeStruct bodies with a thread-local depth counter that throws TProtocolException.DEPTH_LIMIT once nesting exceeds 64 levels and is decremented on the way out via scope(exit). The counter is a module-level variable and therefore thread-local in D, so concurrent (de)serialization on separate threads is unaffected. Unknown-field skipping (skip()) has its own, separate traversal and is left unchanged. Replace the isolated counter test with a round-trip unittest in thrift.codegen.base that drives the generated struct read/write path with a self-recursive RecTree, mirroring test/Recursive.thrift. (The CoRec/CoRec2/ RecList types there recurse by value and cannot be expressed as D structs, so RecTree -- which nests through a list -- is the representable case.) A chain at the limit round-trips; a chain one level over is rejected with DEPTH_LIMIT on both write and read (the over-limit read payload is crafted with raw protocol primitives, since a normal write would itself be rejected); and a wide, shallow tree round-trips, confirming the counter is decremented per sibling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
THRIFT-6053— bound the recursion depth of D struct read/write.D struct serialization runs through the
readStruct/writeStructtemplatesin
thrift.codegen.base— the generatedread()/write()methods forward tothem. On
masterthese templates do not bound recursion depth, so a deeplynested message is read or written without a limit.
This PR wraps the
readStruct/writeStructbodies with a thread-local depthcounter that throws
TProtocolException.DEPTH_LIMITonce nesting exceeds 64levels, decrementing on the way out via
scope(exit). The counter is amodule-level variable and therefore thread-local in D, so concurrent
(de)serialization on separate threads is unaffected. Unknown-field skipping
(
skip()) has its own, separate traversal and is left unchanged.Test
The previous test set the depth counter directly and called
readStruct/writeStructonce with anullprotocol — it never serialized a nested struct.It is replaced by a round-trip
unittestinthrift.codegen.basethat drivesthe generated struct read/write path with a self-recursive
RecTree, mirroringtest/Recursive.thrift.DEPTH_LIMITDEPTH_LIMITValidation (
dmd -unittest, DMD 2.087):thrift.codegen.baseunittests pass.at "writing past the limit must throw" — an over-limit chain serializes with
no
DEPTH_LIMIT— confirming the gap this change closes.D is excluded from the Linux CI matrix (
--without-d), so this was validatedlocally with DMD.
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com