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

   [THRIFT-6160](https://issues.apache.org/jira/browse/THRIFT-6160)
   
   A Haxe `TFramedTransport` cannot read a second frame on the same connection 
unless something flushed in between. The second frame fails on its four-byte 
header with `MESSAGE_SIZE_LIMIT`, a few kilobytes into a 100 MB allowance.
   
   ### Mechanism
   
   - `readFrame()` binds the read budget to each frame with 
`UpdateKnownMessageSize()` and never returns it to the configured maximum first.
   - `TSocket` charges every read against that budget.
   - `ResetConsumedMessageSize()` refuses to grow a budget an earlier, smaller 
frame narrowed.
   
   Reading a frame therefore leaves the budget at zero, and the next 
`readFrameSize()` throws at once.
   
   A request/response exchange hides this, because writing the reply flushes 
and the flush is one of only two resets on the socket path (the other is 
connect). A run of **one-way** calls does not: the generated processor returns 
before the flush for a one-way function, so a server handling two one-way calls 
in a row fails the second.
   
   ### Fix
   
   Reset the budget at the top of `readFrame()` — what the Delphi binding does 
at the same point in the message, and for the same reason, its endpoint 
charging reads too.
   
   Reaching the budget from a layered transport needs a public entry point, so 
`TTransport` gains `ResetMessageSizeAndConsumedBytes()`, named after the Delphi 
and netstd equivalents. It is a plain method beside the existing 
`UpdateKnownMessageSize()` and `CheckReadBytesAvailable()` rather than an 
abstract one, so nothing outside the tree has to implement it.
   
   ### Tests
   
   Three, in `lib/haxe/test/src/tests/FramedTransportTest.hx`, wired into the 
suite. Written before the change; verified by stashing the fix and rebuilding:
   
   ```
   baseline:  "first frame reads" - OK
              TTransportException: CountConsumedMessageBytes(4): message size 
exceeds limit 104857600
   fixed:     ... all assertions OK ... All tests completed.
   ```
   
   The third test declares a 64 MB field inside a 64-byte frame and passes both 
before and after — it is there to show the budget has not been loosened while 
the reset was widened.
   
   The tests need a purpose-built endpoint: `TStreamTransport` does not charge 
reads against the budget, so nothing in the library could stand in for socket 
accounting without a real socket. The double mirrors `TSocket.read` and says so.
   
   Verified on neko (full suite, including the existing stream and 
recursion-limit tests); the python and php targets cross-compile clean.
   
   ### Affected versions
   
   Present since 2dcefadba (THRIFT-5370), i.e. 0.15.0 onward.
   
   🤖 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]

Reply via email to