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.
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 @@
// 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 TestTHeaderTransportNoDoubleWrapping(t *testing.T) {
+ trans := NewTMemoryBuffer()
+ orig := NewTHeaderTransport(trans)
+ wrapped := NewTHeaderTransport(orig)
+
+ if wrapped != orig {
+ t.Errorf("NewTHeaderTransport double wrapped THeaderTransport")
+ }
+}