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

   [THRIFT-6165](https://issues.apache.org/jira/browse/THRIFT-6165)
   
   `TFramedTransport` and `TFastFramedTransport` read a frame header that tells 
them the exact size of the message they are about to hand to the protocol, and 
never tell the budget. `checkReadBytesAvailable` is therefore measured against 
`TConfiguration.maxMessageSize` for the life of the connection, so a 68-byte 
frame may declare a 64 MB field and the array is allocated before the shortfall 
is found.
   
   ### The API this needs
   
   Binding the budget to the frame needs an entry point that does not exist 
today.
   
   - `resetConsumedMessageSize(-1)` is the only genuine full reset, and it is 
`protected`, so a layered transport cannot call it on its inner transport.
   - `updateKnownMessageSize()` refuses to grow a budget an earlier, smaller 
frame narrowed, and carries that frame's consumption forward besides.
   
   So this adds `resetMessageSizeAndConsumedBytes(long)` to `TTransport`, 
mirroring netstd's `ResetMessageSizeAndConsumedBytes`, implemented on 
`TEndpointTransport`, delegated by `TLayeredTransport`, and a no-op on 
`TFileTransport` alongside its existing no-op budget methods.
   
   ### Change
   
   `readFrame()` resets twice, for two different reasons.
   
   - **Before** the framing reads, because what is left of the previous frame's 
bound describes a frame we are done with, and an inner transport that 
decrements on read — `TMemoryBuffer`, or the `TMemoryInputTransport` the 
nonblocking server uses — would otherwise refuse a frame larger than the last 
one.
   - **After** them, binding to the frame, discarding what the framing itself 
spent rather than charging it twice.
   
   `TMemoryInputTransport.reset()` binds to the buffer it is given, as its 
constructor already did. Reads there are clamped to the buffer and short-read 
rather than checked against the budget, so this only tightens what the protocol 
may declare.
   
   ### Source compatibility
   
   The new abstract method is a **source-compatibility break** for anything 
extending `TTransport` directly rather than through `TEndpointTransport` or 
`TLayeredTransport`. Four test doubles in this repository needed it. Worth a 
release note.
   
   ### Tests
   
   Nine in `TestFrameBoundReadBudget`, eight of them written before the change. 
Three fail against the unmodified library:
   
   ```
   testDeclaredFieldLargerThanFrameIsRejected
   testDeclaredFieldLargerThanFrameIsRejectedFastFramed
   testDeclaredFieldLargerThanResetBufferIsRejected
   ```
   
   The rest describe traffic that is legitimate today, and they exist precisely 
because the natural implementation of frame binding breaks them. The ninth 
records why the bound cannot go on the inner transport: with the nonblocking 
server's shape, getting the frame off the wire spends the inner budget 
entirely, and without the first reset the frame's own bytes are then unreadable.
   
   The tests use `TIOStreamTransport` rather than `TMemoryBuffer` as the 
endpoint — `TMemoryBuffer`'s constructor calls `updateKnownMessageSize`, which 
is the behaviour under test, and using it made two of the three failing tests 
pass spuriously.
   
   346 tests pass; `spotlessCheck` clean.
   
   ### Relationship to THRIFT-5371
   
   [THRIFT-5371](https://issues.apache.org/jira/browse/THRIFT-5371) reports the 
C++ half of this and asks there whether Java is affected too. Partly. Java does 
**not** accumulate across frames the way C++ does, because `TIOStreamTransport` 
never decrements the budget on read — its only reset is the `flush()` one on 
the write side. What Java does share is the other half, the budget never being 
narrowed to the frame, which is what the three failing tests describe.
   
   c_glib is [THRIFT-6166](https://issues.apache.org/jira/browse/THRIFT-6166). 
Separate PRs, one per binding.
   
   🤖 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