[jira] [Commented] (THRIFT-5371) Max Message Size is eventually exceeded when using TFramedTransport

["Jens Geyer (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/THRIFT-5371?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18109703#comment-18109703 ] 

Jens Geyer commented on THRIFT-5371:
------------------------------------

Confirmed, and fixed on the way to 0.25.0 -- [PR #3762|https://github.com/apache/thrift/pull/3762].

The reported symptom reproduces as a test: 64 consecutive frames read over a transport configured
with room for any single frame but not for their sum ends in
{{TTransportException: MaxMessageSize reached}}, because {{TFramedTransport::readFrame()}} never
touches the budget and only {{readEnd()}} or {{flush()}} restores it -- neither of which a oneway
call reaches.

[~DenzilDexter]'s suggested {{resetConsumedMessageSize()}} is the right instinct, but the reset
belongs in {{readFrame()}} rather than {{readEnd()}}, and it needs a second half. {{readFrame()}}
already knows the exact size of the frame it is about to hand to the protocol, and that number is
a far better budget than {{maxMessageSize}}: it is what stops a 68-byte frame declaring a 64 MB
field, which {{readStringBody()}} would otherwise resize the caller's string to before discovering
there is nothing behind it. So {{readFrame()}} resets the budget and then binds it to the frame.
{{updateKnownMessageSize()}} had no caller anywhere in the C++ library before this -- the
mechanism was there and entirely unused.

[~ctubbsii] -- on your question about Java: partly. Java does not accumulate the way C++ does,
because {{TIOStreamTransport}} never decrements the budget on read; its only reset is the
{{flush()}} one on the write side. It does share the other half, the budget never being narrowed
to the frame. That is THRIFT-6165. c_glib has the same defect again, THRIFT-6166.

What this deliberately does not cover: {{THeaderTransport}} overrides {{readFrame()}} and is
unchanged, since in unframed mode it hands out four bytes at a time and binding there is a
separate question. THRIFT-5464 is related but not closed by this -- {{TBufferBase::read()}} still
checks rather than decrements; what changes is that for a framed transport the budget it checks
against is now one frame rather than the whole connection.

_Drafted with AI assistance (Claude Opus 5); posted by Jens Geyer._

> Max Message Size is eventually exceeded when using TFramedTransport
> -------------------------------------------------------------------
>
>                 Key: THRIFT-5371
>                 URL: https://issues.apache.org/jira/browse/THRIFT-5371
>             Project: Thrift
>          Issue Type: Bug
>          Components: C++ - Library
>    Affects Versions: 0.14.1
>         Environment: Windows 2010, Visual Studio 2017, CMake 3.15.2
>            Reporter: Paul Caswell
>            Priority: Major
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> I have been trying to upgrade our application from Thrift 0.11.0 to 0.14.1 and have noticed what I think is a bug.
> Our thrift application uses a TFramedTransport to transmit large quantities of data from the client to the server using a oneway call.  The transports are all created using the (new) default TConfiguration class giving a maximum message size of 100MB.  
> Our application sends data through the thrift library in circa 10MB blocks using a oneway call.  On the 10th call the server terminates with a TTransportException thrown on line 329 of TTransport.h.
> I believe this is happening because the TFramedTransport doesn't reset the 'knownMessageSize_' and 'remainingMessageSize_' counters when a message transfer is completed.  This means that the counter continually reduces until the exception is thrown.  I am new to the thrift library and so perhaps I have this wrong but it's what looks like is happening to me.
> I can make the library work by adding resetConsumedMessageSize(); inside TFramedTransport::readEnd() in file TBufferTransports.cpp (at line 310).  Is this the correct solution?
>  
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)