[jira] [Comment Edited] (THRIFT-1324) TFramedTransport should enforce frame size limits on writes

["John R. Frank (JIRA)" <ji...@apache.org>]

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

John R. Frank edited comment on THRIFT-1324 at 5/12/13 6:25 PM:
----------------------------------------------------------------

Having just gone through such a debugging process as a result of this issue, I'd be happy to contribute a patch.

First, am I reading line 142 in TFramedTransport.java correctly?

  readBuffer_.reset(buff);

which calls this in  TMemoryInputTransport.java 

  public void reset(byte[] buf, int offset, int length) {
    buf_ = buf;   
    pos_ = offset;
    endPos_ = offset + length;
  }

That seems to *replace* the buffer, instead of gathering together multiple frames until detecting the end, e.g. by seeing the client send a frame of less than the max length.

If that understanding is correct, then why do some uses of TFramedTransport have both a max frame size and also a max message size?  For example, cassandra has 

{code:borderStyle=solid}
# Frame size for thrift (maximum field length).
thrift_framed_transport_size_in_mb: 1500

# The max length of a thrift message, including all fields and
# internal thrift overhead.
thrift_max_message_length_in_mb: 1600
{code}

which implies that a message could be made of >1 frame.  Maybe that's just specific to cassandra's use of thrift?

Would there be any interest in a transport that allowed chunking of large messages?

                
      was (Author: jrf):
    Having just gone through such a debugging process as a result of this issue, I'd be happy to contribute a patch.

First, am I reading line 142 in TFramedTransport.java correctly?

  readBuffer_.reset(buff);

which calls this in  TMemoryInputTransport.java 

  public void reset(byte[] buf, int offset, int length) {
    buf_ = buf;   
    pos_ = offset;
    endPos_ = offset + length;
  }

That seems to *replace* the buffer, instead of gathering together multiple frames until detecting the end, e.g. by seeing the client send a frame of less than the max length.

If that understanding is correct, then why do some uses of TFramedTransport have both a max frame size and also a max message size?  For example, cassandra has 

    # Frame size for thrift (maximum field length).
    thrift_framed_transport_size_in_mb: 1500

    # The max length of a thrift message, including all fields and
    # internal thrift overhead.
    thrift_max_message_length_in_mb: 1600

which implies that a message could be made of >1 frame.  Maybe that's just specific to cassandra's use of thrift?

Would there be any interest in a transport that allowed chunking of large messages?

                  
> TFramedTransport should enforce frame size limits on writes
> -----------------------------------------------------------
>
>                 Key: THRIFT-1324
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1324
>             Project: Thrift
>          Issue Type: Bug
>          Components: Java - Library
>            Reporter: Jim Ancona
>             Fix For: 1.0
>
>
> Currently TFramedTransport only enforces the maximum frame size when it receives a frame larger than its configured maxLength_ value. so there is no way to enforce a maximum frame size on the client. Because servers typically deal with oversized frames by silently dropping them (see THRIFT-1323), problems caused by oversized frames can be very hard to diagnose. Enforcing the maximum frame size on writes would enable clients to detect the frame size mismatch, assuming the client and server are configured with the same value.
> Note that the exception thrown in this case should not be a generic TTransportException--it should be either a subclass or a new TTransportException.type_ value so that clients can distinguish the frame too large error. This is important because most other TTransportException causes reflect transient conditions where retry may be appropriate, but a too-large frame will never succeed if retried.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira