[
https://issues.apache.org/jira/browse/THRIFT-6160?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer updated THRIFT-6160:
-------------------------------
Description:
h3. Problem
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}}, having consumed a few kilobytes of a 100 MB allowance.
h3. 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
({{CountConsumedMessageBytes}}).
* {{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 immediately.
h3. Why request/response hides it
The only resets on the socket path are in {{flush()}} and on connect. A normal
exchange
flushes when the reply is written, so the budget is restored before the next
request is
read. 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 one.
h3. Reproduction
Two frames of 16 and 4096 bytes back to back, default configuration:
{noformat}
frame 1: read 16 bytes OK
frame 2: FAILED type=6 : CountConsumedMessageBytes(4): message size exceeds
limit 104857600
{noformat}
h3. Fix
Reset the budget at the top of {{readFrame()}}, which is what the Delphi
binding does at the
same point in the message and for the same reason (its endpoint charges 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 no code outside the tree has to implement it.
Three tests are added in {{lib/haxe/test/src/tests/FramedTransportTest.hx}} and
wired into the
suite. The third declares a 64 MB field inside a 64-byte frame and passes both
before and after
the change; it is there to show the budget has not been loosened while the
reset was widened.
h3. Affected versions
Present since 2dcefadba (THRIFT-5370), i.e. 0.15.0 onward.
h3. Notes
{{ResetConsumedMessageSize}} recognised only an omitted argument as "back to
the configured
maximum", where netstd, Delphi, Java and C++ all use a negative one; a negative
therefore fell
through to the shrink path and left the budget at -1, refusing every later
read. It has no caller
in the tree, so the exposure is callers outside it and anyone porting the
idiom. Folded in here
rather than tracked separately: it is two tokens in the same guard this fix
already depends on,
and it comes with its own test.
One inconsistency found alongside is *not* addressed here:
{{TStreamTransport.read}} does not
charge reads against the budget, while {{TSocket.read}} does, so the two
endpoints disagree on
accounting. That one needs a decision about which behaviour is correct.
_Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens Geyer._
was:
h3. Problem
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}}, having consumed a few kilobytes of a 100 MB allowance.
h3. 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
({{CountConsumedMessageBytes}}).
* {{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 immediately.
h3. Why request/response hides it
The only resets on the socket path are in {{flush()}} and on connect. A normal
exchange
flushes when the reply is written, so the budget is restored before the next
request is
read. 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 one.
h3. Reproduction
Two frames of 16 and 4096 bytes back to back, default configuration:
{noformat}
frame 1: read 16 bytes OK
frame 2: FAILED type=6 : CountConsumedMessageBytes(4): message size exceeds
limit 104857600
{noformat}
h3. Fix
Reset the budget at the top of {{readFrame()}}, which is what the Delphi
binding does at the
same point in the message and for the same reason (its endpoint charges 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 no code outside the tree has to implement it.
Three tests are added in {{lib/haxe/test/src/tests/FramedTransportTest.hx}} and
wired into the
suite. The third declares a 64 MB field inside a 64-byte frame and passes both
before and after
the change; it is there to show the budget has not been loosened while the
reset was widened.
h3. Affected versions
Present since 2dcefadba (THRIFT-5370), i.e. 0.15.0 onward.
h3. Notes
Two smaller inconsistencies were found alongside and are *not* addressed here:
* {{TStreamTransport.read}} does not charge reads against the budget, while
{{TSocket.read}} does, so the two endpoints disagree on accounting.
* {{ResetConsumedMessageSize}} tests {{newSize == null}} where netstd and
Delphi test {{< 0}}, so {{UpdateKnownMessageSize(-1)}} sets the Haxe budget to
-1 instead of performing a full reset. That idiom is what netstd relies on, so
it is a trap when porting.
_Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens Geyer._
> Haxe framed transport cannot read consecutive frames without an intervening
> flush
> ---------------------------------------------------------------------------------
>
> Key: THRIFT-6160
> URL: https://issues.apache.org/jira/browse/THRIFT-6160
> Project: Thrift
> Issue Type: Bug
> Components: Haxe - Library
> Reporter: Jens Geyer
> Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> h3. Problem
> 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}}, having consumed a few kilobytes of a 100 MB allowance.
> h3. 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
> ({{CountConsumedMessageBytes}}).
> * {{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 immediately.
> h3. Why request/response hides it
> The only resets on the socket path are in {{flush()}} and on connect. A
> normal exchange
> flushes when the reply is written, so the budget is restored before the next
> request is
> read. 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 one.
> h3. Reproduction
> Two frames of 16 and 4096 bytes back to back, default configuration:
> {noformat}
> frame 1: read 16 bytes OK
> frame 2: FAILED type=6 : CountConsumedMessageBytes(4): message size exceeds
> limit 104857600
> {noformat}
> h3. Fix
> Reset the budget at the top of {{readFrame()}}, which is what the Delphi
> binding does at the
> same point in the message and for the same reason (its endpoint charges 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 no code outside the tree has to implement it.
> Three tests are added in {{lib/haxe/test/src/tests/FramedTransportTest.hx}}
> and wired into the
> suite. The third declares a 64 MB field inside a 64-byte frame and passes
> both before and after
> the change; it is there to show the budget has not been loosened while the
> reset was widened.
> h3. Affected versions
> Present since 2dcefadba (THRIFT-5370), i.e. 0.15.0 onward.
> h3. Notes
> {{ResetConsumedMessageSize}} recognised only an omitted argument as "back to
> the configured
> maximum", where netstd, Delphi, Java and C++ all use a negative one; a
> negative therefore fell
> through to the shrink path and left the budget at -1, refusing every later
> read. It has no caller
> in the tree, so the exposure is callers outside it and anyone porting the
> idiom. Folded in here
> rather than tracked separately: it is two tokens in the same guard this fix
> already depends on,
> and it comes with its own test.
> One inconsistency found alongside is *not* addressed here:
> {{TStreamTransport.read}} does not
> charge reads against the budget, while {{TSocket.read}} does, so the two
> endpoints disagree on
> accounting. That one needs a decision about which behaviour is correct.
> _Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens
> Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
