Merge pull request #334 from yarpc/dev

Release v0.19.1

CHANGELOG.md

@@ -1,6 +1,9 @@
 Changelog
 =========
 
+# 0.19.1 (2021-04-02)
+* Fix byte parsing to allow 8-bit signed integers to match the Thrift spec & other language implementations.
+
 # 0.19.0 (2021-03-24)
 * Add support for gRPC streaming:
    - Support of benchmark and curl-like fashion mode

thrift/types.go

@@ -25,6 +25,7 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"math"
 	"strconv"
 	"strings"
 )
@@ -115,11 +116,15 @@ func parseBinaryList(vl []interface{}) ([]byte, error) {
 	bs := make([]byte, 0, len(vl))
 	for _, v := range vl {
 		switch v := v.(type) {
+		// YAML uses int for all small values. For large values, it may use int64 or uint64
+		// but those are not valid bool values anyway, so we don't need to check them.
 		case int:
-			// YAML uses int for all small values. For large values, it may use int64 or uint64
-			// but those are not valid bool values anyway, so we don't need to check them.
-			if v < 0 || v >= (1<<8) {
-				return nil, fmt.Errorf("failed to parse list of bytes: %v is not a byte", v)
+			// from https://thrift.apache.org/docs/types, byte is an 8-bit signed integer
+			// In Go, byte is an unsigned int8 (uint8) which has a range [0, 255].
+			// In Java, byte is a signed int8 which has a range of [-128,127].
+			// To make both sides happy, check between [-128,255]
+			if v < math.MinInt8 || v > math.MaxUint8 {
+				return nil, fmt.Errorf("failed to parse list of bytes: %v is not a valid thrift byte", v)
 			}
 			bs = append(bs, byte(v))
 		case string:

thrift/types_test.go

@@ -272,6 +272,18 @@ func TestParseBinary(t *testing.T) {
 			value: []interface{}{65, 66},
 			want:  []byte("AB"),
 		},
+		{
+			value: []interface{}{1, -1},
+			want:  []byte{0x1, 0xff},
+		},
+		{
+			value: []interface{}{-128, 127, 255}, // boundary for diff int types
+			want:  []byte{0x80, 0x7f, 0xff},
+		},
+		{
+			value: []interface{}{-1, 255}, // overflow causes the same results
+			want:  []byte{0xff, 0xff},
+		},
 		{
 			value: []interface{}{104, "ello", "", " ", "world"},
 			want:  []byte("hello world"),
@@ -292,6 +304,10 @@ func TestParseBinary(t *testing.T) {
 			value:  []interface{}{256},
 			errMsg: "failed to parse list of bytes",
 		},
+		{
+			value:  []interface{}{-129},
+			errMsg: "failed to parse list of bytes",
+		},
 		{
 			value:  []interface{}{1.5},
 			errMsg: "can only parse list of bytes",

version.go

@@ -22,4 +22,4 @@ package main
 
 // versionString is the sem-ver version string for yab.
 // It will be bumped explicitly on releases.
-var versionString = "0.19.0"
+var versionString = "0.19.1"