Jens Geyer created THRIFT-6165:
----------------------------------
Summary: Java: bind the read budget to the frame that carries the
message
Key: THRIFT-6165
URL: https://issues.apache.org/jira/browse/THRIFT-6165
Project: Thrift
Issue Type: Bug
Components: Java - Library
Reporter: Jens Geyer
Fix For: 0.25.0
h3. Problem
{{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.
h3. 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.
h3. 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.
h3. 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.
h3. Tests
Nine in {{TestFrameBoundReadBudget}}. Three fail against the unmodified
library; the rest describe
traffic that is legitimate today and must keep passing, because the natural
implementation of frame
binding breaks them. One 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. 346 tests
pass.
h3. Relationship to THRIFT-5371
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.
_Drafted with AI assistance (Claude Opus 5); filed by Jens Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
