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/09/07 15:15:19 UTC

[PR] Test the string length bound in the Smalltalk binary protocol [thrift]

Jens-G opened a new pull request, #3815:
URL: https://github.com/apache/thrift/pull/3815

   `8e3a6de69` gave `TProtocol` a `stringSizeLimit` and made `readString` check the
   declared length against it, but went in without a test. This adds one.
   
   ### What it asserts, and why that shape
   
   The transport the tests use records **how many bytes it was asked for**, not how
   many it served. That is the only observation that separates the two versions: an
   unbounded `readString` also ends in an exception once the peer stops sending, so
   a test that merely checks for a raise passes either way.
   
   With a declared length of `2147483647` the unbounded reader asks the transport
   for `2147483651` bytes; the bounded one asks for `4` — the length prefix, and
   nothing more.
   
   ### Coverage
   
   Eleven cases: the default limit's value; an over-limit length signalling
   `sizeLimit` rather than `negativeSize`; the payload read never being issued; the
   boundary from both sides against a configured limit; a caller-set limit being
   honoured; non-positive limits being refused; and `readMessageBegin`'s name going
   through the same bound, because the name is read before any handler is chosen.
   
   Three of the eleven assert unchanged behaviour — a short string, an empty string,
   and a negative length keeping its own error code.
   
   Unlike `TProtocolRecursionDepthTest` this needs no generated code; it exercises
   the library alone.
   
   ### Verification
   
   Pharo 13.1, headless:
   
   | Against | Result |
   |---|---|
   | `master` | **11/11 pass** |
   | `8e3a6de69^` | 3 pass, 3 fail, 5 error |
   
   The three that still pass without the bound are exactly the three that assert
   unchanged behaviour.
   
   ### Notes
   
   - Test only — no library change, so nothing here alters behaviour.
   - No JIRA ticket: this adds coverage for behaviour already on `master` and
     changes nothing itself. Happy to file one and retitle if you would rather it
     had one.
   
   ---
   Generated-by: Claude Opus 5
   


-- 
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] Test the string length bound in the Smalltalk binary protocol [thrift]

Posted by "Jens-G (via GitHub)" <gi...@apache.org>.
Jens-G merged PR #3815:
URL: https://github.com/apache/thrift/pull/3815


-- 
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