From 71e12979902aa37cfb18c4dd30f51e247e6ee4cc Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Wed, 16 Sep 2026 23:49:25 +0200 Subject: [PATCH] THRIFT-6280: Pass the current TConfiguration on from THeaderTransportFactory Client: go THeaderTransportFactory.SetTConfiguration handed the factory it wraps the configuration it held before, not the one it was given, and NewTHeaderTransportFactoryConf did not pass the configuration to the wrapped factory at all. Both now do what the framed transport factory does: the constructor and SetTConfiguration propagate the given configuration, and GetTransport also passes it to the transport it is handed before the wrapped factory builds on it. The deprecated NewTHeaderTransportFactory still keeps its configuration to itself. Co-Authored-By: Claude Opus 5 (1M context) --- lib/go/thrift/header_transport.go | 4 +- lib/go/thrift/header_transport_test.go | 75 ++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/lib/go/thrift/header_transport.go b/lib/go/thrift/header_transport.go index dcd9c94c8da..b1e6469c1d2 100644 --- a/lib/go/thrift/header_transport.go +++ b/lib/go/thrift/header_transport.go @@ -856,6 +856,7 @@ func NewTHeaderTransportFactory(factory TTransportFactory) TTransportFactory { // NewTHeaderTransportFactoryConf creates a new *THeaderTransportFactory with // the given *TConfiguration. func NewTHeaderTransportFactoryConf(factory TTransportFactory, conf *TConfiguration) TTransportFactory { + PropagateTConfiguration(factory, conf) return &THeaderTransportFactory{ Factory: factory, @@ -866,6 +867,7 @@ func NewTHeaderTransportFactoryConf(factory TTransportFactory, conf *TConfigurat // GetTransport implements TTransportFactory. func (f *THeaderTransportFactory) GetTransport(trans TTransport) (TTransport, error) { if f.Factory != nil { + PropagateTConfiguration(trans, f.cfg) t, err := f.Factory.GetTransport(trans) if err != nil { return nil, err @@ -877,7 +879,7 @@ func (f *THeaderTransportFactory) GetTransport(trans TTransport) (TTransport, er // SetTConfiguration implements TConfigurationSetter. func (f *THeaderTransportFactory) SetTConfiguration(cfg *TConfiguration) { - PropagateTConfiguration(f.Factory, f.cfg) + PropagateTConfiguration(f.Factory, cfg) f.cfg = cfg } diff --git a/lib/go/thrift/header_transport_test.go b/lib/go/thrift/header_transport_test.go index 9c4bc4f93c5..87042ae80b8 100644 --- a/lib/go/thrift/header_transport_test.go +++ b/lib/go/thrift/header_transport_test.go @@ -149,6 +149,81 @@ func TestTHeaderTransportNoDoubleWrapping(t *testing.T) { } } +// headerConfTransport is a memory buffer that remembers the configuration it +// was handed. +type headerConfTransport struct { + *TMemoryBuffer + conf *TConfiguration +} + +func (h *headerConfTransport) SetTConfiguration(conf *TConfiguration) { + h.conf = conf +} + +// headerConfFactory hands out headerConfTransports and remembers the +// configuration it was handed. +type headerConfFactory struct { + conf *TConfiguration + transport *headerConfTransport +} + +func (f *headerConfFactory) GetTransport(TTransport) (TTransport, error) { + f.transport = &headerConfTransport{TMemoryBuffer: NewTMemoryBuffer()} + return f.transport, nil +} + +func (f *headerConfFactory) SetTConfiguration(conf *TConfiguration) { + f.conf = conf +} + +// The factory hands its configuration to the factory it wraps when it is made +// and whenever it is given a new one, and to the transports it makes. +func TestTHeaderTransportFactoryConfPropagation(t *testing.T) { + conf := &TConfiguration{MaxFrameSize: 1024} + inner := &headerConfFactory{} + factory := NewTHeaderTransportFactoryConf(inner, conf) + if inner.conf != conf { + t.Errorf("wrapped factory has configuration %v, want %v", inner.conf, conf) + } + + base := &headerConfTransport{TMemoryBuffer: NewTMemoryBuffer()} + trans, err := factory.GetTransport(base) + if err != nil { + t.Fatal(err) + } + if base.conf != conf { + t.Errorf("transport handed to the factory has configuration %v, want %v", base.conf, conf) + } + if inner.transport.conf != conf { + t.Errorf("wrapped transport has configuration %v, want %v", inner.transport.conf, conf) + } + if got := trans.(*THeaderTransport).cfg; got != conf { + t.Errorf("header transport has configuration %v, want %v", got, conf) + } + + updated := &TConfiguration{MaxFrameSize: 2048} + PropagateTConfiguration(factory, updated) + if inner.conf != updated { + t.Errorf("wrapped factory has configuration %v after the update, want %v", inner.conf, updated) + } + trans, err = factory.GetTransport(NewTMemoryBuffer()) + if err != nil { + t.Fatal(err) + } + if got := trans.(*THeaderTransport).cfg; got != updated { + t.Errorf("header transport has configuration %v after the update, want %v", got, updated) + } +} + +// The deprecated constructor keeps its configuration to itself, as before. +func TestTHeaderTransportFactoryWithoutConf(t *testing.T) { + inner := &headerConfFactory{} + NewTHeaderTransportFactory(inner) + if inner.conf != nil { + t.Errorf("wrapped factory has configuration %v, want none", inner.conf) + } +} + func TestTHeaderTransportNoReadBeyondFrame(t *testing.T) { trans := NewTMemoryBuffer() writeContent := func(writer TTransport, content string) error {