[thrift] branch master updated: THRIFT-4612: Avoid double wrapping THeaderTransport

[dcelasun]

This is an automated email from the ASF dual-hosted git repository.

dcelasun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git


The following commit(s) were added to refs/heads/master by this push:
     new 6ae2b18  THRIFT-4612: Avoid double wrapping THeaderTransport
6ae2b18 is described below

commit 6ae2b189efbe83caa11d341e69359159a773525e
Author: Yuxuan 'fishy' Wang <yuxuan.w...@reddit.com>
AuthorDate: Mon Aug 5 04:59:08 2019 -0700

    THRIFT-4612: Avoid double wrapping THeaderTransport
    
    Client: go
    
    Previously the library didn't check against double wrapping, so when
    NewTSimpleServerN was used with both THeaderTransportFactory and
    THeaderProtocolFactory, inside THeaderProtocolFactory the transport
    was double wrapped with THeaderTransport.
    
    Worse, the transport still appeared to work, because THeaderTransport
    is backwards compatible with TBinaryProtocol and TCompactProtocol
    so the outer layer of THeaderTransport wrapper (the one directly accessible
    from the protocol) would assume the client doesn't support THeader and
    fallback. So when double wrapping happened, it appeared like everything
    was fine, except you couldn't get the headers from the protocol (because
    they were in the inner THeaderTransport, not the outer one that's directly
    accessible from the protocol), making it very hard to debug.
    
    This commit adds protection against such double wrapping.
    
    This closes #1839.
---
 lib/go/thrift/header_transport.go      |  5 +++++
 lib/go/thrift/header_transport_test.go | 10 ++++++++++
 2 files changed, 15 insertions(+)

diff --git a/lib/go/thrift/header_transport.go 
b/lib/go/thrift/header_transport.go
index 4302635..5343ccb 100644
--- a/lib/go/thrift/header_transport.go
+++ b/lib/go/thrift/header_transport.go
@@ -271,7 +271,12 @@ var _ TTransport = (*THeaderTransport)(nil)
 // Please note that THeaderTransport handles framing and zlib by itself,
 // so the underlying transport should be the raw socket transports (TSocket or 
TSSLSocket),
 // instead of rich transports like TZlibTransport or TFramedTransport.
+//
+// If trans is already a *THeaderTransport, it will be returned as is.
 func NewTHeaderTransport(trans TTransport) *THeaderTransport {
+       if ht, ok := trans.(*THeaderTransport); ok {
+               return ht
+       }
        return &THeaderTransport{
                transport:    trans,
                reader:       bufio.NewReader(trans),
diff --git a/lib/go/thrift/header_transport_test.go 
b/lib/go/thrift/header_transport_test.go
index 94af010..7462dd5 100644
--- a/lib/go/thrift/header_transport_test.go
+++ b/lib/go/thrift/header_transport_test.go
@@ -106,3 +106,13 @@ func TestTHeaderHeadersReadWrite(t *testing.T) {
                )
        }
 }
+
+func TestTHeaderTransportNoDoubleWrapping(t *testing.T) {
+       trans := NewTMemoryBuffer()
+       orig := NewTHeaderTransport(trans)
+       wrapped := NewTHeaderTransport(orig)
+
+       if wrapped != orig {
+               t.Errorf("NewTHeaderTransport double wrapped THeaderTransport")
+       }
+}