[
https://issues.apache.org/jira/browse/THRIFT-820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12965952#action_12965952
]
Tony Kinnis commented on THRIFT-820:
------------------------------------
I think this change is necessary and a resetLength() style solution won't be
appropriate in many cases, namely when a client instance is reused for multiple
service calls. In this case each service call deducts from readLength and can
cause the client to fail when it eventually hits zero.
For example:
TProtocolFactory factory = new TBinaryProtocol.Factory(false, true, 100);
Foo.Iface client = new Foo.Client(factory.getProtocol(someTransport));
// pass client off to some other object
client.doSomething();
client.doSomethingElse();
In this example doSomethingElse() can fail if doSomething() was large enough.
It seems reasonable to me that a client should be able to be reused for
multiple service calls without concern for this readLength limit. In most cases
the consumer of the client would not even know such a limit exists so asking
them to reset it wouldn't be appropriate.
I started using this limit and then I ran into this exact issue with clients
and had to stop using it. I would like to set a reasonable limit to avoid the
well-known OutOfMemoryException that can occur when the protocol gets an
unexpected message and tries to allocate a MAX_INT buffer.
I also have a question about the patch. It is unclear to me when
checkReadLength() is called. Is it called once for the entire message? Or, is
it called once per struct or property it is decoding? Looking at the places
were checkReadLength() is called it seems like it is invoked multiple times per
message.
If this is called once for the entire message then the patch seems reasonable.
If not then I think the patch is only setting a limit per struct/property,
which may be ok but there should be comments indicating that is the case.
> The readLength attribute of TBinaryProtocol is used as an instance variable
> and is decremented on each call of checkReadLength
> ------------------------------------------------------------------------------------------------------------------------------
>
> Key: THRIFT-820
> URL: https://issues.apache.org/jira/browse/THRIFT-820
> Project: Thrift
> Issue Type: Bug
> Components: Java - Library
> Affects Versions: 0.2, 0.3
> Reporter: Nate McCall
> Attachments: tbinaryprotocol-length-check-patch-THRIFT-820.txt
>
>
> When coding towards readLength, I saw this more as a check on the message
> length as it comes in. Perhaps its me, but I just dont see why this should
> decrement giving the length header for each message.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
