Jens-G opened a new pull request, #3862: URL: https://github.com/apache/thrift/pull/3862
JIRA: [THRIFT-6264](https://issues.apache.org/jira/browse/THRIFT-6264) Client: dart The ticket says `TProtocol.incrementRecursionDepth()` and `decrementRecursionDepth()` are never called. That is true inside `lib/dart`, but not overall: the Dart generator emits both calls around every generated struct `read()` and `write()` (`t_dart_generator.cc`, THRIFT-6056). Removing the methods would break generated code, and calling them from the protocol implementations as well would count every struct twice. The ticket's last point does hold: the binding had two depth mechanisms. `TProtocolUtil.skip()` threaded a limit of its own (`maxRecursionLimit`) and never touched the protocol's count, although the comments on both said they shared one budget. An unknown field inside a struct that generated code was reading was therefore skipped with a fresh 64 levels. The read and the skip together could nest up to twice the limit. ### Change - `TProtocolUtil._skip()` now calls `incrementRecursionDepth()` for each level it enters and `decrementRecursionDepth()` in a `finally`. A skip that starts inside a nested read therefore gets only the depth that read has left. This is what C++ (`TInputRecursionTracker` in `skip()`) and netstd (`SkipAsync`) do. - `maxRecursionLimit` is public API and stays. It still bounds a single `skip()` call on top of the shared count. - The `switch` moved unchanged into `_skipValue()`, so the diff stays small. - The comments on `TProtocol.defaultRecursionDepth` and `TProtocolUtil.defaultRecursionLimit` now describe what the code does. ### Compatibility A message whose known and unknown nesting together exceeds 64 levels is now refused with `DEPTH_LIMIT`. Before, each part was limited to 64 on its own. ### Tests **`lib/dart/test/protocol/t_protocol_util_depth_test.dart`:** - When the protocol has already used 60 levels, a 4-level skip passes and a 5-level skip is refused. - The count is restored after a skip that returns and after one that throws. The check: exactly 64 levels can be entered afterwards. **`test/dart/recursion_depth_test`**, through the generated `CoRec`/`CoRec2` `read()`, for binary, compact and JSON: - A 32-deep chain whose innermost struct carries an unknown field 32 levels deep is read. - With the unknown field 33 levels deep, the read is refused with `DEPTH_LIMIT`. Results: - Against the unmodified library, the 5-level case and the three 33-level cases fail. - Two mutations fail as intended: - Moving the decrement out of the `finally` fails the "throws" case. - Counting each level twice fails the at-limit cases, which guards against a halved limit. - `make -C lib/dart check` (90 tests) and the recursion test in `test/dart` (33 tests) pass with Dart 3.2.6 and 3.13.3. - `dart analyze` over the package reports the same issues as before. 🤖 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]
