Merge pull request #965 from sbunce/master

Add overflow check for DecodeVarint.

util/encoding/encoding.go

@@ -275,6 +275,9 @@ func DecodeVarint(b []byte) ([]byte, int64) {
 	}
 
 	b, v := DecodeUvarint(b)
+	if v > math.MaxInt64 {
+		panic(fmt.Sprintf("varint %d overflows int64", v))
+	}
 	return b, int64(v)
 }
 

util/encoding/encoding_test.go

@@ -19,6 +19,7 @@ package encoding
 
 import (
 	"bytes"
+	"fmt"
 	"math"
 	"reflect"
 	"regexp"
@@ -345,20 +346,94 @@ func TestEncodeDecodeUvarint(t *testing.T) {
 	testCustomEncodeUint64(testCases, EncodeUvarint, t)
 }
 
-// TestDecodeInvalidUvarint tests that invalid uvarint encodings panic.
-func TestDecodeInvalidUvarint(t *testing.T) {
-	testCases := [][]byte{
-		// length of 9 bytes should cause an error.
-		{0x11, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01},
-	}
-	for _, c := range testCases {
+// TestDecodeInvalid tests that decoding invalid bytes panics.
+func TestDecodeInvalid(t *testing.T) {
+	tests := []struct {
+		name    string       // name printed with errors.
+		buf     []byte       // buf contains an invalid uvarint to decode.
+		pattern string       // pattern matches the panic string.
+		decode  func([]byte) // decode is called with buf.
+	}{
+		{
+			name:    "DecodeUvarint, length of 9 bytes",
+			buf:     []byte{0x11, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01},
+			pattern: "invalid uvarint length of [0-9]+",
+			decode:  func(b []byte) { DecodeUvarint(b) },
+		},
+		{
+			name:    "DecodeVarint, overflows int64",
+			buf:     []byte{0x10, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
+			pattern: "varint [0-9]+ overflows int64",
+			decode:  func(b []byte) { DecodeVarint(b) },
+		},
+		{
+			name:    "Bytes, no terminator",
+			buf:     []byte{'a'},
+			pattern: "did not find terminator",
+			decode:  func(b []byte) { DecodeBytes(b, nil) },
+		},
+		{
+			name:    "Bytes, malformed escape",
+			buf:     []byte{'a', 0x00},
+			pattern: "malformed escape",
+			decode:  func(b []byte) { DecodeBytes(b, nil) },
+		},
+		{
+			name:    "Bytes, invalid escape 1",
+			buf:     []byte{'a', 0x00, 0x00},
+			pattern: "unknown escape",
+			decode:  func(b []byte) { DecodeBytes(b, nil) },
+		},
+		{
+			name:    "Bytes, invalid escape 2",
+			buf:     []byte{'a', 0x00, 0x02},
+			pattern: "unknown escape",
+			decode:  func(b []byte) { DecodeBytes(b, nil) },
+		},
+		{
+			name:    "BytesDecreasing, no terminator",
+			buf:     []byte{^byte('a')},
+			pattern: "did not find terminator",
+			decode:  func(b []byte) { DecodeBytesDecreasing(b, nil) },
+		},
+		{
+			name:    "BytesDecreasing, malformed escape",
+			buf:     []byte{^byte('a'), 0xff},
+			pattern: "malformed escape",
+			decode:  func(b []byte) { DecodeBytesDecreasing(b, nil) },
+		},
+		{
+			name:    "BytesDecreasing, invalid escape 1",
+			buf:     []byte{^byte('a'), 0xff, 0xff},
+			pattern: "unknown escape",
+			decode:  func(b []byte) { DecodeBytesDecreasing(b, nil) },
+		},
+		{
+			name:    "BytesDecreasing, invalid escape 2",
+			buf:     []byte{^byte('a'), 0xff, 0xfd},
+			pattern: "unknown escape",
+			decode:  func(b []byte) { DecodeBytesDecreasing(b, nil) },
+		},
+	}
+	for _, test := range tests {
 		func() {
 			defer func() {
-				if r := recover(); r == nil {
-					t.Error("expected panic")
+				r := recover()
+				if r == nil {
+					t.Errorf("%q, expected panic", test.name)
+					return
+				}
+				str := fmt.Sprint(r)
+				match, err := regexp.MatchString(test.pattern, str)
+				if err != nil {
+					t.Errorf("%q, couldn't match regexp: %v", test.name, err)
+					return
+				}
+				if !match {
+					t.Errorf("%q, pattern %q doesn't match %q", test.name, test.pattern, str)
 				}
 			}()
-			DecodeUvarint(c)
+			test.decode(test.buf)
 		}()
 	}
 }
@@ -469,48 +544,6 @@ func TestEncodeDecodeBytesDecreasing(t *testing.T) {
 	}
 }
 
-func TestDecodeInvalidBytes(t *testing.T) {
-	testCases := []struct {
-		value []byte
-	}{
-		{[]byte{'a'}},             // no terminator
-		{[]byte{'a', 0x00}},       // malformed escape
-		{[]byte{'a', 0x00, 0x00}}, // invalid escape
-		{[]byte{'a', 0x00, 0x02}}, // invalid escape
-	}
-	for _, c := range testCases {
-		func() {
-			defer func() {
-				if r := recover(); r == nil {
-					t.Error("expected panic")
-				}
-			}()
-			DecodeBytes(c.value, nil)
-		}()
-	}
-}
-
-func TestDecodeInvalidBytesDecreasing(t *testing.T) {
-	testCases := []struct {
-		value []byte
-	}{
-		{[]byte{^byte('a')}},             // no terminator
-		{[]byte{^byte('a'), 0xff}},       // malformed escape
-		{[]byte{^byte('a'), 0xff, 0xff}}, // invalid escape
-		{[]byte{^byte('a'), 0xff, 0xfd}}, // invalid escape
-	}
-	for _, c := range testCases {
-		func() {
-			defer func() {
-				if r := recover(); r == nil {
-					t.Error("expected panic")
-				}
-			}()
-			DecodeBytesDecreasing(c.value, nil)
-		}()
-	}
-}
-
 func TestEncodeDecodeKey(t *testing.T) {
 	testCases := []struct {
 		format   string