THRIFT-6055: Limit recursion depth in JavaME struct read/write#3560
Open
Jens-G wants to merge 1 commit into
Open
THRIFT-6055: Limit recursion depth in JavaME struct read/write#3560Jens-G wants to merge 1 commit into
Jens-G wants to merge 1 commit into
Conversation
ba1349e to
307d156
Compare
Client: javame Add a recursionDepth_ counter with incrementRecursionDepth() and decrementRecursionDepth() to TProtocol, bounded at 64, plus a DEPTH_LIMIT type on TProtocolException, and update the JavaME generator to bracket each generated struct read/write body with try/finally so the counter is always restored. This bounds the work performed for deeply nested or cyclic structs, which previously had no limit in the generated read()/write() path. Add a generated-code round-trip regression test (lib/javame/test/TestRecursionDepth.java) that drives the recursive IDL types from test/Recursive.thrift through the generated read()/write() over the binary and JSON protocols: chains at the limit round-trip, chains one past it (write and read) are rejected with DEPTH_LIMIT, a wide structure confirms the counter is decremented for each sibling, and a cyclic graph is rejected instead of recursing without bound. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
307d156 to
982b0ee
Compare
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-6055— bound the recursion depth of JavaME struct read/write.On
masterthe generated JavaMEread()/write()callreadStructBegin/writeStructBegindirectly with no depth bound,TProtocolhas no recursioncounter, and
TProtocolExceptionhas noDEPTH_LIMITtype, so a deeply nested orcyclic struct is processed without a limit in the generated path.
This PR:
DEPTH_LIMIT = 6toTProtocolException;recursionDepth_counter withincrementRecursionDepth()/decrementRecursionDepth()toTProtocol, bounded at 64;try { … } finally { decrementRecursionDepth(); }. The bound lives only in thegenerated path (one count per struct);
readStructBegin/writeStructBeginare left untouched, so there is no double-counting.
Unknown-field skipping is handled separately by
TProtocolUtil.skip(whichcarries its own
maxSkipDepth) and is left unchanged here.Test
The previous test exercised
increment/decrementRecursionDepth()in isolation,which does not cover the real generated read/write path. It is replaced by
lib/javame/test/TestRecursionDepth.java— a generated-code round-trip over therecursive IDL types in
test/Recursive.thrift(CoRec⇄CoRec2,RecTree),run over the binary and JSON protocols:
DEPTH_LIMITDEPTH_LIMITDEPTH_LIMITinstead of unbounded recursionValidation with OpenJDK 17 (against
lib/javame/src, excluding thejavax.microedition-onlyTHttpClient):cyclic checks fail — over-limit writes and reads are not rejected and the
cyclic case recurses to
StackOverflowError— confirming the gap this changecloses.
JavaME has no build harness or CI; the test is a standalone
main, and thegenerate / compile / run commands are documented in its header.
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com