[ https://issues.apache.org/jira/browse/THRIFT-4326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16177489#comment-16177489 ]
ASF GitHub Bot commented on THRIFT-4326: ---------------------------------------- Github user benweint commented on the issue: https://github.com/apache/thrift/pull/1352 Ok, I've finally figured out how to reproduce that test failure locally on macOS. For posterity, here's what I did: ``` PATH="/usr/local/opt/bison/bin:$PATH" ./configure --with-openssl=/usr/local/opt/openssl --with-boost=/usr/local --without-lua --without-csharp --without-c_glib --without-erlang --without-php --without-cpp --without-haskell make -j3 precross test/test.py --server rb --client nodejs ``` After doing that, the tests that use a `BufferedTransport` on the Ruby server side fail. Since I'm only dealing with Ruby on the client side in my usage, I didn't realize that `BufferedTransport` can be used to wrap a server socket as well (this is what the Ruby `TestServer.rb` is doing when testing a buffered transport). The issue stems from my misunderstanding of the semantics of `#flush`. In the nodejs -> Ruby cross test, I think the node client writes multiple requests to the socket before the Ruby server responds to any of them. The Ruby server calls `#flush` after each response to flush the write buffer, but my change drops the read buffer as well, discarding the buffered requests before they've been processed. I'm going to close this and think about whether there's a better way to accomplish what I'm after. > Ruby BufferedTransport not safe for reuse after reading corrupted input > ----------------------------------------------------------------------- > > Key: THRIFT-4326 > URL: https://issues.apache.org/jira/browse/THRIFT-4326 > Project: Thrift > Issue Type: Bug > Components: Ruby - Library > Affects Versions: 0.10.0 > Environment: Originally observed with Thrift 0.9.3 on Linux with Ruby > 2.3.4, but have also reproduced on Mac OS X with Thrift 0.10.0. > Reporter: Ben Weintraub > > We've experimented with the Ruby {{BufferedTransport}} class as a wrapper > around the {{HttpClientTransport}} class, and found that we were getting > clusters sporadic {{Thrift::ProtocolException}} errors in Ruby client > processes after network issues caused corruption of some Thrift response > bodies. > Using a bare {{HttpClientTransport}} makes these issues disappear. > For a given service, we retain a long-lived protocol instance > ({{CompactProtocol}} in our case), which in turn holds a reference to a > long-lived {{BufferedTransport}} instance. > The problem seems to stem from the case where the Thrift client is > interrupted (e.g. by a Ruby timeout exception) before consuming to the end of > the {{@rbuf}} instance variable in {{BufferedTransport}}, leaving {{@index}} > pointing to the middle of the read buffer, and meaning that when the > transport is re-used upon the next service call, the {{BufferedTransport}} > continues reading where it left off in the old buffer, rather than calling > through to the wrapped {{HttpClientTransport}} to read the new response > obtained from the last call to {{#flush}}. > Now I know {{Timeout}} is fundamentally unsafe in Ruby and can lead to all > kinds of issues like this, but I've also found that this same issue can be > triggered by another fairly plausible scenario: if the Thrift service returns > a well-formed Thrift response but with N extra bytes of garbage tacked onto > the end, then the next N following service calls through the same > {{BufferedTransport}} instance will fail with a > {{Thrift::ProtocolException}}, as the {{BufferedTransport}} will continue > attempting to read the left-over bytes in {{@rbuf}}. > The naive solution seems like it would be to just reset {{@rbuf}} from > {{#flush}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)