Repository: segmentio/encoding (go)

Issue:
Decoder hangs when decoding invalid json
```go
func TestHangingDecoder(t *testing.T) {
	b := []byte(`{
	"userId": "blah",
	}`)

	d := NewDecoder(bytes.NewReader(b))

	var a struct {
		UserId string `json:"userId"`
	}
	err := d.Decode(&a)
	if err == nil {
		t.Fatal("should have errored")
	}
}
```

This test hangs on master. Light on details for now, will add more when I have them.

Looks like it's an issue in Decoder. It works fine with `Unmarshal`:

```go
// this works
func TestHangingDecoder(t *testing.T) {
	b := []byte(`{
	"userId": "blah",
	}`)
	var a struct {
		UserId string `json:"userId"`
	}
	err := Unmarshal(b, &a)
	if err == nil {
		t.Fatal("should have errored")
	}
}
```

Pull request description:
fix issue 18
Fixes #18 

Fixes the issue where we had an infinite loop in the decode when parsing some invalid JSON.

Patch:
diff --git a/Makefile b/Makefile
index 4134c4a..a95e7c7 100644
--- a/Makefile
+++ b/Makefile
@@ -15,6 +15,7 @@ test:
 	go test -v -cover ./json
 	go test -v -cover ./iso8601
 	go run ./json/bugs/issue11/main.go
+	go run ./json/bugs/issue18/main.go
 
 $(benchcmp):
 	GO111MODULE=off go install golang.org/x/tools/cmd/benchcmp
diff --git a/json/bugs/issue18/main.go b/json/bugs/issue18/main.go
new file mode 100644
index 0000000..eda5349
--- /dev/null
+++ b/json/bugs/issue18/main.go
@@ -0,0 +1,22 @@
+package main
+
+import (
+	"bytes"
+	"fmt"
+
+	"github.com/segmentio/encoding/json"
+)
+
+func main() {
+	b := []byte(`{
+	"userId": "blah",
+	}`)
+
+	d := json.NewDecoder(bytes.NewReader(b))
+
+	var a struct {
+		UserId string `json:"userId"`
+	}
+	fmt.Println(d.Decode(&a))
+	fmt.Println(a)
+}
diff --git a/json/json.go b/json/json.go
index b1687c3..4af5e78 100644
--- a/json/json.go
+++ b/json/json.go
@@ -251,22 +251,12 @@ func (dec *Decoder) Buffered() io.Reader {
 
 // Decode is documented at https://golang.org/pkg/encoding/json/#Decoder.Decode
 func (dec *Decoder) Decode(v interface{}) error {
-	if dec.err != nil {
-		return dec.err
-	}
-
 	raw, err := dec.readValue()
 	if err != nil {
-		dec.err = err
 		return err
 	}
-
 	_, err = Parse(raw, v, dec.flags)
-	if err != nil {
-		return err
-	}
-
-	return nil
+	return err
 }
 
 const (
@@ -287,6 +277,19 @@ func (dec *Decoder) readValue() (v []byte, err error) {
 				dec.remain = skipSpaces(r)
 				return
 			}
+			if len(r) != 0 {
+				// Parsing of the next JSON value stopped at a position other
+				// than the end of the input buffer, which indicaates that a
+				// syntax error was encountered.
+				return
+			}
+		}
+
+		if err = dec.err; err != nil {
+			if len(dec.remain) != 0 && err == io.EOF {
+				err = io.ErrUnexpectedEOF
+			}
+			return
 		}
 
 		if dec.buffer == nil {
@@ -306,10 +309,7 @@ func (dec *Decoder) readValue() (v []byte, err error) {
 		if n > 0 {
 			dec.buffer = dec.buffer[:len(dec.buffer)+n]
 		}
-		if err != nil && !(err == io.EOF && len(dec.buffer) != 0) {
-			return
-		}
-		dec.remain = skipSpaces(dec.buffer)
+		dec.remain, dec.err = skipSpaces(dec.buffer), err
 	}
 }
 
diff --git a/json/parse.go b/json/parse.go
index 9190b08..d0500d9 100644
--- a/json/parse.go
+++ b/json/parse.go
@@ -244,6 +244,9 @@ func parseNull(b []byte) ([]byte, []byte, error) {
 	if hasNullPrefix(b) {
 		return b[:4], b[4:], nil
 	}
+	if len(b) < 4 {
+		return nil, b[len(b):], unexpectedEOF(b)
+	}
 	return nil, b, syntaxError(b, "expected 'null' but found invalid token")
 }
 
@@ -251,6 +254,9 @@ func parseTrue(b []byte) ([]byte, []byte, error) {
 	if hasTruePrefix(b) {
 		return b[:4], b[4:], nil
 	}
+	if len(b) < 4 {
+		return nil, b[len(b):], unexpectedEOF(b)
+	}
 	return nil, b, syntaxError(b, "expected 'true' but found invalid token")
 }
 
@@ -258,12 +264,15 @@ func parseFalse(b []byte) ([]byte, []byte, error) {
 	if hasFalsePrefix(b) {
 		return b[:5], b[5:], nil
 	}
+	if len(b) < 5 {
+		return nil, b[len(b):], unexpectedEOF(b)
+	}
 	return nil, b, syntaxError(b, "expected 'false' but found invalid token")
 }
 
 func parseNumber(b []byte) (v, r []byte, err error) {
 	if len(b) == 0 {
-		r, err = b, syntaxError(b, "expected number but found no data")
+		r, err = b, unexpectedEOF(b)
 		return
 	}
 
@@ -373,7 +382,11 @@ func parseUnicode(b []byte) (rune, int, error) {
 }
 
 func parseStringFast(b []byte) ([]byte, []byte, bool, error) {
-	if len(b) < 2 || b[0] != '"' {
+	if len(b) < 2 {
+		return nil, b[len(b):], false, unexpectedEOF(b)
+	}
+
+	if b[0] != '"' {
 		return nil, b, false, syntaxError(b, "expected '\"' at the beginning of a string value")
 	}
 
@@ -528,7 +541,11 @@ func appendCoerceInvalidUTF8(b []byte, s []byte) []byte {
 }
 
 func parseObject(b []byte) ([]byte, []byte, error) {
-	if len(b) < 2 || b[0] != '{' {
+	if len(b) < 2 {
+		return nil, b[len(b):], unexpectedEOF(b)
+	}
+
+	if b[0] != '{' {
 		return nil, b, syntaxError(b, "expected '{' at the beginning of an object value")
 	}
 
@@ -558,6 +575,12 @@ func parseObject(b []byte) ([]byte, []byte, error) {
 				return nil, b, syntaxError(b, "expected ',' after object field value but found '%c'", b[0])
 			}
 			b = skipSpaces(b[1:])
+			if len(b) == 0 {
+				return nil, b, unexpectedEOF(b)
+			}
+			if b[0] == '}' {
+				return nil, b, syntaxError(b, "unexpected trailing comma after object field")
+			}
 		}
 
 		_, b, err = parseString(b)
@@ -584,7 +607,11 @@ func parseObject(b []byte) ([]byte, []byte, error) {
 }
 
 func parseArray(b []byte) ([]byte, []byte, error) {
-	if len(b) < 2 || b[0] != '[' {
+	if len(b) < 2 {
+		return nil, b[len(b):], unexpectedEOF(b)
+	}
+
+	if b[0] != '[' {
 		return nil, b, syntaxError(b, "expected '[' at the beginning of array value")
 	}
 
@@ -614,6 +641,12 @@ func parseArray(b []byte) ([]byte, []byte, error) {
 				return nil, b, syntaxError(b, "expected ',' after array element but found '%c'", b[0])
 			}
 			b = skipSpaces(b[1:])
+			if len(b) == 0 {
+				return nil, b, unexpectedEOF(b)
+			}
+			if b[0] == ']' {
+				return nil, b, syntaxError(b, "unexpected trailing comma after object field")
+			}
 		}
 
 		_, b, err = parseValue(b)


Test patch:
diff --git a/json/json_test.go b/json/json_test.go
index f651615..95dab27 100644
--- a/json/json_test.go
+++ b/json/json_test.go
@@ -1325,3 +1325,30 @@ func TestGithubIssue16(t *testing.T) {
 		})
 	}
 }
+
+func TestGithubIssue18(t *testing.T) {
+	// https://github.com/segmentio/encoding/issues/18
+	b := []byte(`{
+	"userId": "blah",
+	}`)
+
+	d := NewDecoder(bytes.NewReader(b))
+
+	var a struct {
+		UserId string `json:"userId"`
+	}
+	switch err := d.Decode(&a).(type) {
+	case *SyntaxError:
+	default:
+		t.Error("expected syntax error but found:", err)
+	}
+
+	for i := 1; i <= 18; i++ { // up to the invalid ',' character
+		d := NewDecoder(bytes.NewReader(b[:i])) // cut somewhere in the middle
+		switch err := d.Decode(&a); err {
+		case io.ErrUnexpectedEOF:
+		default:
+			t.Error("expected 'unexpected EOF' error but found:", err)
+		}
+	}
+}