THRIFT-3302 Go JSON protocol should encode Thrift byte type as signed integer string

Client: Go
Patch: Nobuaki Sukegawa <nsukeg@gmail.com>

This closes #591

13 files changed, 52 insertions, 50 deletions

diff --git a/compiler/cpp/src/generate/t_go_generator.cc b/compiler/cpp/src/generate/t_go_generator.cc
index 74d4b2ff8..cc649c45b 100644
--- a/compiler/cpp/src/generate/t_go_generator.cc
+++ b/compiler/cpp/src/generate/t_go_generator.cc
@@ -3049,7 +3049,7 @@ void t_go_generator::generate_serialize_field(ofstream& out,
         break;
 
       case t_base_type::TYPE_BYTE:
-        out << "WriteByte(byte(" << name << "))";
+        out << "WriteByte(int8(" << name << "))";
         break;
 
       case t_base_type::TYPE_I16:
diff --git a/lib/go/test/tests/protocol_mock.go b/lib/go/test/tests/protocol_mock.go
index 2238074db..9197fedab 100644
--- a/lib/go/test/tests/protocol_mock.go
+++ b/lib/go/test/tests/protocol_mock.go
@@ -80,9 +80,9 @@ func (_mr *_MockTProtocolRecorder) ReadBool() *gomock.Call {
 	return _mr.mock.ctrl.RecordCall(_mr.mock, "ReadBool")
 }
 
-func (_m *MockTProtocol) ReadByte() (byte, error) {
+func (_m *MockTProtocol) ReadByte() (int8, error) {
 	ret := _m.ctrl.Call(_m, "ReadByte")
-	ret0, _ := ret[0].(byte)
+	ret0, _ := ret[0].(int8)
 	ret1, _ := ret[1].(error)
 	return ret0, ret1
 }
@@ -320,7 +320,7 @@ func (_mr *_MockTProtocolRecorder) WriteBool(arg0 interface{}) *gomock.Call {
 	return _mr.mock.ctrl.RecordCall(_mr.mock, "WriteBool", arg0)
 }
 
-func (_m *MockTProtocol) WriteByte(_param0 byte) error {
+func (_m *MockTProtocol) WriteByte(_param0 int8) error {
 	ret := _m.ctrl.Call(_m, "WriteByte", _param0)
 	ret0, _ := ret[0].(error)
 	return ret0
diff --git a/lib/go/thrift/binary_protocol.go b/lib/go/thrift/binary_protocol.go
index d1059f276..e1b405664 100644
--- a/lib/go/thrift/binary_protocol.go
+++ b/lib/go/thrift/binary_protocol.go
@@ -92,7 +92,7 @@ func (p *TBinaryProtocol) WriteMessageBegin(name string, typeId TMessageType, se
 		if e != nil {
 			return e
 		}
-		e = p.WriteByte(byte(typeId))
+		e = p.WriteByte(int8(typeId))
 		if e != nil {
 			return e
 		}
@@ -115,7 +115,7 @@ func (p *TBinaryProtocol) WriteStructEnd() error {
 }
 
 func (p *TBinaryProtocol) WriteFieldBegin(name string, typeId TType, id int16) error {
-	e := p.WriteByte(byte(typeId))
+	e := p.WriteByte(int8(typeId))
 	if e != nil {
 		return e
 	}
@@ -133,11 +133,11 @@ func (p *TBinaryProtocol) WriteFieldStop() error {
 }
 
 func (p *TBinaryProtocol) WriteMapBegin(keyType TType, valueType TType, size int) error {
-	e := p.WriteByte(byte(keyType))
+	e := p.WriteByte(int8(keyType))
 	if e != nil {
 		return e
 	}
-	e = p.WriteByte(byte(valueType))
+	e = p.WriteByte(int8(valueType))
 	if e != nil {
 		return e
 	}
@@ -150,7 +150,7 @@ func (p *TBinaryProtocol) WriteMapEnd() error {
 }
 
 func (p *TBinaryProtocol) WriteListBegin(elemType TType, size int) error {
-	e := p.WriteByte(byte(elemType))
+	e := p.WriteByte(int8(elemType))
 	if e != nil {
 		return e
 	}
@@ -163,7 +163,7 @@ func (p *TBinaryProtocol) WriteListEnd() error {
 }
 
 func (p *TBinaryProtocol) WriteSetBegin(elemType TType, size int) error {
-	e := p.WriteByte(byte(elemType))
+	e := p.WriteByte(int8(elemType))
 	if e != nil {
 		return e
 	}
@@ -182,8 +182,8 @@ func (p *TBinaryProtocol) WriteBool(value bool) error {
 	return p.WriteByte(0)
 }
 
-func (p *TBinaryProtocol) WriteByte(value byte) error {
-	e := p.trans.WriteByte(value)
+func (p *TBinaryProtocol) WriteByte(value int8) error {
+	e := p.trans.WriteByte(byte(value))
 	return NewTProtocolException(e)
 }
 
@@ -392,8 +392,9 @@ func (p *TBinaryProtocol) ReadBool() (bool, error) {
 	return v, e
 }
 
-func (p *TBinaryProtocol) ReadByte() (value byte, err error) {
-	return p.trans.ReadByte()
+func (p *TBinaryProtocol) ReadByte() (int8, error) {
+	v, err := p.trans.ReadByte()
+	return int8(v), err
 }
 
 func (p *TBinaryProtocol) ReadI16() (value int16, err error) {
diff --git a/lib/go/thrift/compact_protocol.go b/lib/go/thrift/compact_protocol.go
index 731bd163b..0bc5fddeb 100644
--- a/lib/go/thrift/compact_protocol.go
+++ b/lib/go/thrift/compact_protocol.go
@@ -267,8 +267,8 @@ func (p *TCompactProtocol) WriteBool(value bool) error {
 }
 
 // Write a byte. Nothing to see here!
-func (p *TCompactProtocol) WriteByte(value byte) error {
-	err := p.writeByteDirect(value)
+func (p *TCompactProtocol) WriteByte(value int8) error {
+	err := p.writeByteDirect(byte(value))
 	return NewTProtocolException(err)
 }
 
@@ -330,7 +330,7 @@ func (p *TCompactProtocol) WriteBinary(bin []byte) error {
 // Read a message header.
 func (p *TCompactProtocol) ReadMessageBegin() (name string, typeId TMessageType, seqId int32, err error) {
 
-	protocolId, err := p.ReadByte()
+	protocolId, err := p.readByteDirect()
 	if err != nil {
 		return
 	}
@@ -340,7 +340,7 @@ func (p *TCompactProtocol) ReadMessageBegin() (name string, typeId TMessageType,
 		return "", typeId, seqId, NewTProtocolExceptionWithType(BAD_VERSION, e)
 	}
 
-	versionAndType, err := p.ReadByte()
+	versionAndType, err := p.readByteDirect()
 	if err != nil {
 		return
 	}
@@ -382,7 +382,7 @@ func (p *TCompactProtocol) ReadStructEnd() error {
 
 // Read a field header off the wire.
 func (p *TCompactProtocol) ReadFieldBegin() (name string, typeId TType, id int16, err error) {
-	t, err := p.ReadByte()
+	t, err := p.readByteDirect()
 	if err != nil {
 		return
 	}
@@ -441,7 +441,7 @@ func (p *TCompactProtocol) ReadMapBegin() (keyType TType, valueType TType, size
 
 	keyAndValueType := byte(STOP)
 	if size != 0 {
-		keyAndValueType, err = p.ReadByte()
+		keyAndValueType, err = p.readByteDirect()
 		if err != nil {
 			return
 		}
@@ -458,7 +458,7 @@ func (p *TCompactProtocol) ReadMapEnd() error { return nil }
 // of the element type header will be 0xF, and a varint will follow with the
 // true size.
 func (p *TCompactProtocol) ReadListBegin() (elemType TType, size int, err error) {
-	size_and_type, err := p.ReadByte()
+	size_and_type, err := p.readByteDirect()
 	if err != nil {
 		return
 	}
@@ -503,17 +503,17 @@ func (p *TCompactProtocol) ReadBool() (value bool, err error) {
 		p.boolValueIsNotNull = false
 		return p.boolValue, nil
 	}
-	v, err := p.ReadByte()
+	v, err := p.readByteDirect()
 	return v == COMPACT_BOOLEAN_TRUE, err
 }
 
 // Read a single byte off the wire. Nothing interesting here.
-func (p *TCompactProtocol) ReadByte() (value byte, err error) {
-	value, err = p.trans.ReadByte()
+func (p *TCompactProtocol) ReadByte() (int8, error) {
+	v, err := p.readByteDirect()
 	if err != nil {
 		return 0, NewTProtocolException(err)
 	}
-	return
+	return int8(v), err
 }
 
 // Read an i16 from the wire as a zigzag varint.
@@ -721,7 +721,7 @@ func (p *TCompactProtocol) readVarint64() (int64, error) {
 	shift := uint(0)
 	result := int64(0)
 	for {
-		b, err := p.ReadByte()
+		b, err := p.readByteDirect()
 		if err != nil {
 			return 0, err
 		}
@@ -734,6 +734,11 @@ func (p *TCompactProtocol) readVarint64() (int64, error) {
 	return result, nil
 }
 
+// Read a byte, unlike ReadByte that reads Thrift-byte that is i8.
+func (p *TCompactProtocol) readByteDirect() (byte, error) {
+	return p.trans.ReadByte()
+}
+
 //
 // encoding helpers
 //
diff --git a/lib/go/thrift/debug_protocol.go b/lib/go/thrift/debug_protocol.go
index ee341b2f7..d37252cc6 100644
--- a/lib/go/thrift/debug_protocol.go
+++ b/lib/go/thrift/debug_protocol.go
@@ -117,7 +117,7 @@ func (tdp *TDebugProtocol) WriteBool(value bool) error {
 	log.Printf("%sWriteBool(value=%#v) => %#v", tdp.LogPrefix, value, err)
 	return err
 }
-func (tdp *TDebugProtocol) WriteByte(value byte) error {
+func (tdp *TDebugProtocol) WriteByte(value int8) error {
 	err := tdp.Delegate.WriteByte(value)
 	log.Printf("%sWriteByte(value=%#v) => %#v", tdp.LogPrefix, value, err)
 	return err
@@ -218,7 +218,7 @@ func (tdp *TDebugProtocol) ReadBool() (value bool, err error) {
 	log.Printf("%sReadBool() (value=%#v, err=%#v)", tdp.LogPrefix, value, err)
 	return
 }
-func (tdp *TDebugProtocol) ReadByte() (value byte, err error) {
+func (tdp *TDebugProtocol) ReadByte() (value int8, err error) {
 	value, err = tdp.Delegate.ReadByte()
 	log.Printf("%sReadByte() (value=%#v, err=%#v)", tdp.LogPrefix, value, err)
 	return
diff --git a/lib/go/thrift/json_protocol.go b/lib/go/thrift/json_protocol.go
index 41cd218ae..669a7bddc 100644
--- a/lib/go/thrift/json_protocol.go
+++ b/lib/go/thrift/json_protocol.go
@@ -69,7 +69,7 @@ func (p *TJSONProtocol) WriteMessageBegin(name string, typeId TMessageType, seqI
 	if e := p.WriteString(name); e != nil {
 		return e
 	}
-	if e := p.WriteByte(byte(typeId)); e != nil {
+	if e := p.WriteByte(int8(typeId)); e != nil {
 		return e
 	}
 	if e := p.WriteI32(seqId); e != nil {
@@ -170,7 +170,7 @@ func (p *TJSONProtocol) WriteBool(b bool) error {
 	return p.WriteI32(0)
 }
 
-func (p *TJSONProtocol) WriteByte(b byte) error {
+func (p *TJSONProtocol) WriteByte(b int8) error {
 	return p.WriteI32(int32(b))
 }
 
@@ -349,9 +349,9 @@ func (p *TJSONProtocol) ReadBool() (bool, error) {
 	return (value != 0), err
 }
 
-func (p *TJSONProtocol) ReadByte() (byte, error) {
+func (p *TJSONProtocol) ReadByte() (int8, error) {
 	v, err := p.ReadI64()
-	return byte(v), err
+	return int8(v), err
 }
 
 func (p *TJSONProtocol) ReadI16() (int16, error) {
diff --git a/lib/go/thrift/json_protocol_test.go b/lib/go/thrift/json_protocol_test.go
index cd4927332..7104ce3a0 100644
--- a/lib/go/thrift/json_protocol_test.go
+++ b/lib/go/thrift/json_protocol_test.go
@@ -101,7 +101,7 @@ func TestWriteJSONProtocolByte(t *testing.T) {
 		if s != fmt.Sprint(value) {
 			t.Fatalf("Bad value for %s %v: %s", thetype, value, s)
 		}
-		v := byte(0)
+		v := int8(0)
 		if err := json.Unmarshal([]byte(s), &v); err != nil || v != value {
 			t.Fatalf("Bad json-decoded value for %s %v, wrote: '%s', expected: '%v'", thetype, value, s, v)
 		}
diff --git a/lib/go/thrift/protocol.go b/lib/go/thrift/protocol.go
index 6fb01778c..45fa202e7 100644
--- a/lib/go/thrift/protocol.go
+++ b/lib/go/thrift/protocol.go
@@ -43,7 +43,7 @@ type TProtocol interface {
 	WriteSetBegin(elemType TType, size int) error
 	WriteSetEnd() error
 	WriteBool(value bool) error
-	WriteByte(value byte) error
+	WriteByte(value int8) error
 	WriteI16(value int16) error
 	WriteI32(value int32) error
 	WriteI64(value int64) error
@@ -64,7 +64,7 @@ type TProtocol interface {
 	ReadSetBegin() (elemType TType, size int, err error)
 	ReadSetEnd() error
 	ReadBool() (value bool, err error)
-	ReadByte() (value byte, err error)
+	ReadByte() (value int8, err error)
 	ReadI16() (value int16, err error)
 	ReadI32() (value int32, err error)
 	ReadI64() (value int64, err error)
diff --git a/lib/go/thrift/protocol_test.go b/lib/go/thrift/protocol_test.go
index 7e7950da5..613eae6bc 100644
--- a/lib/go/thrift/protocol_test.go
+++ b/lib/go/thrift/protocol_test.go
@@ -34,7 +34,7 @@ var (
 	data           string // test data for writing
 	protocol_bdata []byte // test data for writing; same as data
 	BOOL_VALUES    []bool
-	BYTE_VALUES    []byte
+	BYTE_VALUES    []int8
 	INT16_VALUES   []int16
 	INT32_VALUES   []int32
 	INT64_VALUES   []int64
@@ -49,7 +49,7 @@ func init() {
 	}
 	data = string(protocol_bdata)
 	BOOL_VALUES = []bool{false, true, false, false, true}
-	BYTE_VALUES = []byte{117, 0, 1, 32, 127, 128, 255}
+	BYTE_VALUES = []int8{117, 0, 1, 32, 127, -128, -1}
 	INT16_VALUES = []int16{459, 0, 1, -1, -128, 127, 32767, -32768}
 	INT32_VALUES = []int32{459, 0, 1, -1, -128, 127, 32767, 2147483647, -2147483535}
 	INT64_VALUES = []int64{459, 0, 1, -1, -128, 127, 32767, 2147483647, -2147483535, 34359738481, -35184372088719, -9223372036854775808, 9223372036854775807}
diff --git a/lib/go/thrift/serializer_types_test.go b/lib/go/thrift/serializer_types_test.go
index 4b25b1f9e..10f473742 100644
--- a/lib/go/thrift/serializer_types_test.go
+++ b/lib/go/thrift/serializer_types_test.go
@@ -459,7 +459,7 @@ func (p *MyTestStruct) writeField2(oprot TProtocol) (err error) {
 	if err := oprot.WriteFieldBegin("b", BYTE, 2); err != nil {
 		return PrependError(fmt.Sprintf("%T write field begin error 2:b: ", p), err)
 	}
-	if err := oprot.WriteByte(byte(p.B)); err != nil {
+	if err := oprot.WriteByte(int8(p.B)); err != nil {
 		return PrependError(fmt.Sprintf("%T.b (2) field write error: ", p), err)
 	}
 	if err := oprot.WriteFieldEnd(); err != nil {
diff --git a/lib/go/thrift/simple_json_protocol.go b/lib/go/thrift/simple_json_protocol.go
index c5ee9dbc6..d30e2bc02 100644
--- a/lib/go/thrift/simple_json_protocol.go
+++ b/lib/go/thrift/simple_json_protocol.go
@@ -162,7 +162,7 @@ func (p *TSimpleJSONProtocol) WriteMessageBegin(name string, typeId TMessageType
 	if e := p.WriteString(name); e != nil {
 		return e
 	}
-	if e := p.WriteByte(byte(typeId)); e != nil {
+	if e := p.WriteByte(int8(typeId)); e != nil {
 		return e
 	}
 	if e := p.WriteI32(seqId); e != nil {
@@ -204,10 +204,10 @@ func (p *TSimpleJSONProtocol) WriteMapBegin(keyType TType, valueType TType, size
 	if e := p.OutputListBegin(); e != nil {
 		return e
 	}
-	if e := p.WriteByte(byte(keyType)); e != nil {
+	if e := p.WriteByte(int8(keyType)); e != nil {
 		return e
 	}
-	if e := p.WriteByte(byte(valueType)); e != nil {
+	if e := p.WriteByte(int8(valueType)); e != nil {
 		return e
 	}
 	return p.WriteI32(int32(size))
@@ -237,7 +237,7 @@ func (p *TSimpleJSONProtocol) WriteBool(b bool) error {
 	return p.OutputBool(b)
 }
 
-func (p *TSimpleJSONProtocol) WriteByte(b byte) error {
+func (p *TSimpleJSONProtocol) WriteByte(b int8) error {
 	return p.WriteI32(int32(b))
 }
 
@@ -463,9 +463,9 @@ func (p *TSimpleJSONProtocol) ReadBool() (bool, error) {
 	return value, p.ParsePostValue()
 }
 
-func (p *TSimpleJSONProtocol) ReadByte() (byte, error) {
+func (p *TSimpleJSONProtocol) ReadByte() (int8, error) {
 	v, err := p.ReadI64()
-	return byte(v), err
+	return int8(v), err
 }
 
 func (p *TSimpleJSONProtocol) ReadI16() (int16, error) {
@@ -736,7 +736,7 @@ func (p *TSimpleJSONProtocol) OutputElemListBegin(elemType TType, size int) erro
 	if e := p.OutputListBegin(); e != nil {
 		return e
 	}
-	if e := p.WriteByte(byte(elemType)); e != nil {
+	if e := p.WriteByte(int8(elemType)); e != nil {
 		return e
 	}
 	if e := p.WriteI64(int64(size)); e != nil {
diff --git a/lib/go/thrift/simple_json_protocol_test.go b/lib/go/thrift/simple_json_protocol_test.go
index 1abff75b5..8f0dcc9df 100644
--- a/lib/go/thrift/simple_json_protocol_test.go
+++ b/lib/go/thrift/simple_json_protocol_test.go
@@ -95,7 +95,7 @@ func TestWriteSimpleJSONProtocolByte(t *testing.T) {
 		if s != fmt.Sprint(value) {
 			t.Fatalf("Bad value for %s %v: %s", thetype, value, s)
 		}
-		v := byte(0)
+		v := int8(0)
 		if err := json.Unmarshal([]byte(s), &v); err != nil || v != value {
 			t.Fatalf("Bad json-decoded value for %s %v, wrote: '%s', expected: '%v'", thetype, value, s, v)
 		}
diff --git a/test/known_failures_Linux.json b/test/known_failures_Linux.json
index eef9f4656..2f21c3ada 100644
--- a/test/known_failures_Linux.json
+++ b/test/known_failures_Linux.json
@@ -180,10 +180,6 @@
   "go-nodejs_json_framed-ip-ssl",
   "go-perl_binary_buffered-ip-ssl",
   "go-perl_binary_framed-ip-ssl",
-  "go-py_json_buffered-ip",
-  "go-py_json_buffered-ip-ssl",
-  "go-py_json_framed-ip",
-  "go-py_json_framed-ip-ssl",
   "go-rb_binary-accel_buffered-ip",
   "go-rb_binary-accel_framed-ip",
   "go-rb_binary_buffered-ip",