You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@thrift.apache.org by "Jens-G (via GitHub)" <gi...@apache.org> on 2026/06/01 12:43:15 UTC
[PR] Add recursion-depth round-trip test for the Haxe library [thrift]
Jens-G opened a new pull request, #3571:
URL: https://github.com/apache/thrift/pull/3571
## Summary
The Haxe library already enforces a struct read/write recursion-depth limit (`TConfiguration.RecursionLimit`, default 64, added in THRIFT-5370): the generator wraps every struct reader/writer with `IncrementRecursionDepth`/`DecrementRecursionDepth`, `TProtocolImplBase` holds the counter, and `TProtocolException.DEPTH_LIMIT` is raised on excess. There was, however, **no test** covering it. This adds one.
Verified — by inspection and by the new test — that the guard is:
- **single-count, not double-count**: `readStructBegin`/`writeStructBegin` do not increment (only the generated `read()`/`write()` do), so a chain exactly at the limit (64) still round-trips and is *not* rejected at 32.
- applied to **struct, union, and exception** alike (all route through the same generated reader/writer), on **both read and write**.
## Test
`lib/haxe/test/src/tests/RecursionLimitTest.hx` performs full round-trips over the recursive types in `test/Recursive.thrift` — `CoRec` (struct), `CoUnion` (union), `CoError` (exception):
- a 64-deep chain round-trips (struct depth preserved);
- writing a 65-deep chain throws `DEPTH_LIMIT`;
- reading a crafted 65-deep payload throws `DEPTH_LIMIT` (asserting the specific `DEPTH_LIMIT` code).
It is wired into `Main` so it runs as part of the default test mode.
The test `Makefile.am` previously did not generate `ConstantsDemo` (needed by the existing `ConstantsTest`, and therefore by `Main`), so the unit-test harness did not build; this PR also adds that missing generation rule alongside the new `Recursive.thrift` rule.
## Validation
Built and run on the **neko** target in the project's `thrift:jammy` image: **13 / 13** assertions pass (the existing `StreamTest` plus the 9 new recursion-depth checks). `lib/haxe/test` is not exercised by CI (`--without-haxe`; the `lib-haxe-codegen` job is codegen-only), so this is validated locally.
No JIRA ticket — the depth limit already shipped (THRIFT-5370); this is a test (plus a one-line harness fix) only.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recursion-depth round-trip test for the Haxe library [thrift]
Posted by "Jens-G (via GitHub)" <gi...@apache.org>.
Jens-G merged PR #3571:
URL: https://github.com/apache/thrift/pull/3571
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org