Fix field stop read in duplicate_protocol.go (#3125)
When generated code reads a struct, it runs a `for` loop calling
`ReadFieldBegin` at the top, but breaks if the field type ID is
`thrift.STOP`.
With TDuplicateToProtocol naively writing everything read, this results
in extra writes, which breaks just about any protocol in the
`DuplicateTo` struct field.
The proposed fix is to simply add special handling for `thrift.STOP` to
`ReadFieldBegin`.
I'm no thrift expert, so I have no idea how other libraries handle this
concern. Ideally, it seems like each protocol should understand and
enforce the invariant that an attempt to call `WriteFieldBegin` with
type ID 0 either isn't valid or is a misguided attempt to call
`WriteFieldStop`.
Co-authored-by: Yuxuan 'fishy' Wang <fishywang@gmail.com>
diff --git a/lib/go/thrift/duplicate_protocol.go b/lib/go/thrift/duplicate_protocol.go
index 6413909..a9eb7eb 100644
--- a/lib/go/thrift/duplicate_protocol.go
+++ b/lib/go/thrift/duplicate_protocol.go
@@ -198,6 +198,10 @@
func (tdtp *TDuplicateToProtocol) ReadFieldBegin(ctx context.Context) (name string, typeId TType, id int16, err error) {
name, typeId, id, err = tdtp.Delegate.ReadFieldBegin(ctx)
+ if typeId == STOP {
+ tdtp.DuplicateTo.WriteFieldStop(ctx)
+ return
+ }
tdtp.DuplicateTo.WriteFieldBegin(ctx, name, typeId, id)
return
}
diff --git a/lib/go/thrift/duplicate_protocol_test.go b/lib/go/thrift/duplicate_protocol_test.go
new file mode 100644
index 0000000..6674f04
--- /dev/null
+++ b/lib/go/thrift/duplicate_protocol_test.go
@@ -0,0 +1,52 @@
+package thrift
+
+import (
+ "bytes"
+ "context"
+ "encoding/json"
+ "testing"
+)
+
+func TestDuplicateToJSONReadMakesEqualJSON(t *testing.T) {
+ delegateBuf := NewTMemoryBuffer()
+ iprot := NewTJSONProtocolFactory().GetProtocol(delegateBuf)
+
+ s := NewMyTestStruct()
+ ctx := context.Background()
+ s.Write(ctx, iprot)
+
+ iprot.Flush(ctx)
+ jsonBytesWritten := delegateBuf.Bytes()
+
+ if err := json.Unmarshal(jsonBytesWritten, NewMyTestStruct()); err != nil {
+ t.Errorf("error unmarshaling written bytes: %s", err)
+ }
+
+ duplicateBuf := NewTMemoryBuffer()
+ dupProto := NewTJSONProtocolFactory().GetProtocol(duplicateBuf)
+ proto := &TDuplicateToProtocol{
+ Delegate: NewTJSONProtocolFactory().GetProtocol(delegateBuf),
+ DuplicateTo: dupProto,
+ }
+
+ if err := s.Read(ctx, proto); err != nil {
+ t.Errorf("error reading struct from DuplicateTo: %s", err)
+ }
+ dupProto.Flush(ctx)
+
+ jsonBytesRead := duplicateBuf.Bytes()
+
+ if !bytes.Equal(jsonBytesWritten, jsonBytesRead) {
+ t.Errorf(`bytes read into duplicate do not equal bytes written
+ read:
+ %s
+ written:
+ %s
+ `, jsonBytesRead, jsonBytesWritten)
+ }
+
+ dup := NewMyTestStruct()
+ if err := dup.Read(ctx, dupProto); err != nil {
+ t.Errorf("error reading struct from duplicated protocol: %s", err)
+ }
+}