Jens Geyer created THRIFT-6181:
----------------------------------

             Summary: Bound the response frame size in the Java async client
                 Key: THRIFT-6181
                 URL: https://issues.apache.org/jira/browse/THRIFT-6181
             Project: Thrift
          Issue Type: Bug
          Components: Java - Library
            Reporter: Jens Geyer
             Fix For: 0.25.0


TAsyncMethodCall sizes the response frame from the four bytes the peer sends 
ahead of it, with no check of any kind 
(lib/java/src/main/java/org/apache/thrift/async/TAsyncMethodCall.java, 
doReadingResponseSize):

    if (sizeBuffer.remaining() == 0) {
      state = State.READING_RESPONSE_BODY;
      frameBuffer = 
ByteBuffer.allocate(TFramedTransport.decodeFrameSize(sizeBufferArray));
    }

decodeFrameSize returns a signed int, so the peer chooses any value from 
-2147483648 to 2147483647, and ByteBuffer.allocate is handed it directly. A 
negative one raises IllegalArgumentException, which is not a 
TTransportException and reaches the callback as-is; a large positive one is 
allocated, and ByteBuffer.allocate zeroes the array, so the memory is resident 
rather than reserved.

This is the odd one out among the three places lib/java reads a frame size. 
TFramedTransport.readFrame and TFastFramedTransport.readFrame both refuse a 
negative size and one over getConfiguration().getMaxFrameSize(). 
AbstractNonblockingServer.FrameBuffer refuses a size <= 0, one over 
trans_.getMaxFrameSize(), and additionally defers a frame that would push the 
server past MAX_READ_BUFFER_BYTES. Only the async client takes the number as it 
comes.

The fix applies the two checks the framed transports already apply, with the 
same messages, so that the two ways of reading a framed response agree.

Compatibility, worth a release note: maxFrameSize defaults to 16384000, so an 
async client now refuses a response frame larger than that where it previously 
allocated it. Callers who expect larger responses raise it with 
TNonblockingTransport.setMaxFrameSize, and the bound follows.

Tests are in 
lib/java/src/test/java/org/apache/thrift/async/TestTAsyncMethodCallFrameSize.java.
 They drive a real TAsyncMethodCall against a socket that answers with a frame 
size and no body, and assert the buffer that was allocated rather than only 
that the call failed -- a response body that never arrives ends the call either 
way. Two of the three fail before the change: "allocated 65536 for a frame that 
is over the maximum", and "a negative size reached ByteBuffer.allocate: 
java.lang.IllegalArgumentException: capacity < 0: (-1 < 0)". The third reads a 
frame of exactly the maximum end to end and passes either way.




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

Reply via email to