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

   [THRIFT-6161](https://issues.apache.org/jira/browse/THRIFT-6161) — follows 
[THRIFT-6160](https://issues.apache.org/jira/browse/THRIFT-6160) (#3755), same 
subsystem, independent change.
   
   `TStreamTransport` does not charge reads against the message budget, so 
`MaxMessageSize` has no effect on a stream-backed connection however much a 
message reads, as long as no single read exceeds the limit on its own. Its 
`flush()` does not restore the allowance either.
   
   ### Why this is a defect in Haxe specifically
   
   There is no single convention across the bindings — there are three, and 
every other binding keeps to one:
   
   | Binding | Sockets / streams charge? | Boundary-aware endpoints (HTTP, 
pipe, whole-message buffer) |
   |---|---|---|
   | cpp, c_glib | no | no — only the zlib transport charges |
   | java, netstd | no | yes |
   | delphi | yes | yes |
   | **haxe** | `TSocket` **yes**, `TStreamTransport` **no** | yes |
   
   Haxe is the only binding that departs from *its own* convention. Its four 
endpoint transports are `TSocket` (charges, 3 sites), `THttpClient` (charges), 
`TFullDuplexHttpClient` (charges) and `TStreamTransport` (charges nothing).
   
   `doc/specs/thrift-tconfiguration.md` describes `MaxMessageSize` as "a 
general device to be used with any transport or protocol", expressed as the 
bytes *remaining* to be read — which only means something if reads draw it down.
   
   ### Fix
   
   Two coordinated changes, mirroring `TSocket`:
   
   - `read()` charges what it read
   - `flush()` resets the allowance, at the same point `TSocket` does
   
   **Both are needed.** Charging without resetting would leave the budget only 
ever shrinking, so a long-lived connection would run itself out of it — which 
is exactly THRIFT-6160, fixed just before this.
   
   ### Tests
   
   Two, in `StreamTest`, failing before and passing after. The first reads past 
the limit 32 bytes at a time, so nothing but cumulative accounting can catch 
it; the second checks the allowance comes back.
   
   Verified on neko — full suite, including the existing stream, constants and 
recursion-limit tests, **all of which read through `TStreamTransport`** and now 
have charged reads. The python and php targets cross-compile clean.
   
   🤖 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