THRIFT-5609: Make TJSONProtocol safe to be used in deserializer pool
Client: go
Add Reset to TJSONProtocol, and call it in deserializer and serializer
to make sure that it's always safe to be used in the pool version.
diff --git a/CHANGES.md b/CHANGES.md
index 1b451a2..f480e42 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -8,6 +8,7 @@
- [THRIFT-5583](https://issues.apache.org/jira/browse/THRIFT-5583) - Add `skip_remote` arg to compiler, which can be used to skip the generating of -remote folders for services
- [THRIFT-5527](https://issues.apache.org/jira/browse/THRIFT-5527) - Compiler generated Process function will swallow exceptions defined in thrift IDL
- [THRIFT-5605](https://issues.apache.org/jira/browse/THRIFT-5605) - Provide `ExtractIDLExceptionClientMiddleware` and `ExtractExceptionFromResult` to help client middlewares to gain access to exceptions defined in thrift IDL
+- [THRIFT-5609](https://issues.apache.org/jira/browse/THRIFT-5609) - `TJSONProtocol` is now safe to be used in `TDeserializePool`
## 0.16.0
diff --git a/lib/go/test/tests/json_protocol_deserializer_test.go b/lib/go/test/tests/json_protocol_deserializer_test.go
new file mode 100644
index 0000000..468e2c6
--- /dev/null
+++ b/lib/go/test/tests/json_protocol_deserializer_test.go
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * 'License'); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package tests
+
+import (
+ "context"
+ "testing"
+ "testing/quick"
+
+ "github.com/apache/thrift/lib/go/test/gopath/src/thrifttest"
+ "github.com/apache/thrift/lib/go/thrift"
+)
+
+func TestDeserializerPoolJSONProtocol(t *testing.T) {
+ ctx := context.Background()
+
+ serializerPool := thrift.NewTSerializerPoolSizeFactory(1024, thrift.NewTJSONProtocolFactory())
+ msg := &thrifttest.Bonk{
+ Message: "foo",
+ Type: 42,
+ }
+ valid, err := serializerPool.WriteString(ctx, msg)
+ if err != nil {
+ t.Fatal(err)
+ }
+ invalid := valid[:len(valid)-2]
+
+ deserializerPool := thrift.NewTDeserializerPoolSizeFactory(1024, thrift.NewTJSONProtocolFactory())
+ msg = new(thrifttest.Bonk)
+ if err := deserializerPool.ReadString(ctx, msg, invalid); err == nil {
+ t.Fatalf("Deserializing %q did not fail", invalid)
+ }
+
+ f := func() bool {
+ msg := new(thrifttest.Bonk)
+ if err := deserializerPool.ReadString(ctx, msg, valid); err != nil {
+ t.Errorf("Deserializing string %q failed with %v", valid, err)
+ }
+ if err := deserializerPool.Read(ctx, msg, []byte(valid)); err != nil {
+ t.Errorf("Deserializing bytes %q failed with %v", valid, err)
+ }
+ return !t.Failed()
+ }
+ if err := quick.Check(f, nil); err != nil {
+ t.Error(err)
+ }
+}
diff --git a/lib/go/thrift/deserializer.go b/lib/go/thrift/deserializer.go
index cefc7ec..2f2468b 100644
--- a/lib/go/thrift/deserializer.go
+++ b/lib/go/thrift/deserializer.go
@@ -39,8 +39,15 @@
}
}
+type reseter interface {
+ Reset()
+}
+
func (t *TDeserializer) ReadString(ctx context.Context, msg TStruct, s string) (err error) {
t.Transport.Reset()
+ if r, ok := t.Protocol.(reseter); ok {
+ r.Reset()
+ }
err = nil
if _, err = t.Transport.Write([]byte(s)); err != nil {
@@ -54,6 +61,9 @@
func (t *TDeserializer) Read(ctx context.Context, msg TStruct, b []byte) (err error) {
t.Transport.Reset()
+ if r, ok := t.Protocol.(reseter); ok {
+ r.Reset()
+ }
err = nil
if _, err = t.Transport.Write(b); err != nil {
diff --git a/lib/go/thrift/serializer.go b/lib/go/thrift/serializer.go
index c449790..f4d9201 100644
--- a/lib/go/thrift/serializer.go
+++ b/lib/go/thrift/serializer.go
@@ -46,6 +46,9 @@
func (t *TSerializer) WriteString(ctx context.Context, msg TStruct) (s string, err error) {
t.Transport.Reset()
+ if r, ok := t.Protocol.(reseter); ok {
+ r.Reset()
+ }
if err = msg.Write(ctx, t.Protocol); err != nil {
return
@@ -63,6 +66,9 @@
func (t *TSerializer) Write(ctx context.Context, msg TStruct) (b []byte, err error) {
t.Transport.Reset()
+ if r, ok := t.Protocol.(reseter); ok {
+ r.Reset()
+ }
if err = msg.Write(ctx, t.Protocol); err != nil {
return
diff --git a/lib/go/thrift/simple_json_protocol.go b/lib/go/thrift/simple_json_protocol.go
index c9c450b..5cefb60 100644
--- a/lib/go/thrift/simple_json_protocol.go
+++ b/lib/go/thrift/simple_json_protocol.go
@@ -121,8 +121,7 @@
writer: bufio.NewWriter(t),
reader: bufio.NewReader(t),
}
- v.parseContextStack.push(_CONTEXT_IN_TOPLEVEL)
- v.dumpContext.push(_CONTEXT_IN_TOPLEVEL)
+ v.resetContextStack()
return v
}
@@ -1328,6 +1327,17 @@
p.cfg = conf
}
+// Reset resets this protocol's internal state.
+//
+// It's useful when a single protocol instance is reused after errors, to make
+// sure the next use will not be in a bad state to begin with. An example is
+// when it's used in serializer/deserializer pools.
+func (p *TSimpleJSONProtocol) Reset() {
+ p.resetContextStack()
+ p.writer.Reset(p.trans)
+ p.reader.Reset(p.trans)
+}
+
var (
_ TConfigurationSetter = (*TSimpleJSONProtocol)(nil)
_ TConfigurationSetter = (*TSimpleJSONProtocolFactory)(nil)