Change UnmarshalExtJSON to return error for invalid JSON.

GODRIVER-694

Change-Id: I66ba147cd0d71f41cd8ed9b34e21d9b56add6044

Author: Divjot Arora

bson/bson_corpus_spec_test.go

@@ -315,8 +315,9 @@ func runTest(t *testing.T, file string) {
 				err := UnmarshalExtJSON([]byte(s), true, &doc)
 				expectError(t, err, fmt.Sprintf("%s: expected parse error", p.Description))
 			case "0x13":
-				ejvr := bsonrw.NewExtJSONValueReader(strings.NewReader(s), true)
-				_, err := ejvr.ReadDecimal128()
+				ejvr, err := bsonrw.NewExtJSONValueReader(strings.NewReader(s), true)
+				expectNoError(t, err, fmt.Sprintf("error creating value reader: %s", err))
+				_, err = ejvr.ReadDecimal128()
 				expectError(t, err, fmt.Sprintf("%s: expected parse error", p.Description))
 			default:
 				t.Errorf("Update test to check for parse errors for type %s", test.BsonType)

bson/bsonrw/extjson_reader.go

@@ -32,10 +32,9 @@ func NewExtJSONValueReaderPool() *ExtJSONValueReaderPool {
 }
 
 // Get retrieves a ValueReader from the pool and uses src as the underlying ExtJSON.
-func (bvrp *ExtJSONValueReaderPool) Get(r io.Reader, canonical bool) ValueReader {
+func (bvrp *ExtJSONValueReaderPool) Get(r io.Reader, canonical bool) (ValueReader, error) {
 	vr := bvrp.pool.Get().(*extJSONValueReader)
-	vr = vr.reset(r, canonical)
-	return vr
+	return vr.reset(r, canonical)
 }
 
 // Put inserts a ValueReader into the pool. If the ValueReader is not a ExtJSON ValueReader nothing
@@ -46,7 +45,7 @@ func (bvrp *ExtJSONValueReaderPool) Put(vr ValueReader) (ok bool) {
 		return false
 	}
 
-	bvr = bvr.reset(nil, false)
+	bvr, _ = bvr.reset(nil, false)
 	bvrp.pool.Put(bvr)
 	return true
 }
@@ -68,22 +67,21 @@ type extJSONValueReader struct {
 // NewExtJSONValueReader creates a new ValueReader from a given io.Reader
 // It will interpret the JSON of r as canonical or relaxed according to the
 // given canonical flag
-func NewExtJSONValueReader(r io.Reader, canonical bool) ValueReader {
+func NewExtJSONValueReader(r io.Reader, canonical bool) (ValueReader, error) {
 	return newExtJSONValueReader(r, canonical)
 }
 
-func newExtJSONValueReader(r io.Reader, canonical bool) *extJSONValueReader {
+func newExtJSONValueReader(r io.Reader, canonical bool) (*extJSONValueReader, error) {
 	ejvr := new(extJSONValueReader)
 	return ejvr.reset(r, canonical)
 }
 
-func (ejvr *extJSONValueReader) reset(r io.Reader, canonical bool) *extJSONValueReader {
+func (ejvr *extJSONValueReader) reset(r io.Reader, canonical bool) (*extJSONValueReader, error) {
 	p := newExtJSONParser(r, canonical)
 	typ, err := p.peekType()
 
 	if err != nil {
-		// TODO: invalid JSON--return error message?
-		return nil
+		return nil, ErrInvalidJSON
 	}
 
 	var m mode
@@ -104,7 +102,7 @@ func (ejvr *extJSONValueReader) reset(r io.Reader, canonical bool) *extJSONValue
 	return &extJSONValueReader{
 		p:     p,
 		stack: stack,
-	}
+	}, nil
 }
 
 func (ejvr *extJSONValueReader) advanceFrame() {

bson/bsonrw/extjson_reader_test.go

@@ -105,7 +105,10 @@ func TestReadMultipleTopLevelDocuments(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			r := strings.NewReader(tc.input)
-			vr := NewExtJSONValueReader(r, false)
+			vr, err := NewExtJSONValueReader(r, false)
+			if err != nil {
+				t.Fatalf("expected no error, but got %v", err)
+			}
 
 			actual, err := readAllDocuments(vr)
 			if err != nil {

bson/unmarshal.go

@@ -64,15 +64,23 @@ func UnmarshalExtJSON(data []byte, canonical bool, val interface{}) error {
 // Registry r and stores the result in the value pointed to by val. If val is
 // nil or not a pointer, UnmarshalWithRegistry returns InvalidUnmarshalError.
 func UnmarshalExtJSONWithRegistry(r *bsoncodec.Registry, data []byte, canonical bool, val interface{}) error {
-	ejvr := bsonrw.NewExtJSONValueReader(bytes.NewReader(data), canonical)
+	ejvr, err := bsonrw.NewExtJSONValueReader(bytes.NewReader(data), canonical)
+	if err != nil {
+		return err
+	}
+
 	return unmarshalFromReader(bsoncodec.DecodeContext{Registry: r}, ejvr, val)
 }
 
 // UnmarshalExtJSONWithContext parses the extended JSON-encoded data using
 // DecodeContext dc and stores the result in the value pointed to by val. If val is
 // nil or not a pointer, UnmarshalWithRegistry returns InvalidUnmarshalError.
 func UnmarshalExtJSONWithContext(dc bsoncodec.DecodeContext, data []byte, canonical bool, val interface{}) error {
-	ejvr := bsonrw.NewExtJSONValueReader(bytes.NewReader(data), canonical)
+	ejvr, err := bsonrw.NewExtJSONValueReader(bytes.NewReader(data), canonical)
+	if err != nil {
+		return err
+	}
+
 	return unmarshalFromReader(dc, ejvr, val)
 }
 

bson/unmarshal_test.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/mongodb/mongo-go-driver/bson/bsoncodec"
+	"github.com/mongodb/mongo-go-driver/bson/bsonrw"
 )
 
 func TestUnmarshal(t *testing.T) {
@@ -81,6 +82,14 @@ func TestUnmarshalExtJSONWithRegistry(t *testing.T) {
 			t.Errorf("Did not unmarshal as expected. got %v; want %v", got, want)
 		}
 	})
+
+	t.Run("UnmarshalExtJSONInvalidInput", func(t *testing.T) {
+		data := []byte("invalid")
+		err := UnmarshalExtJSONWithRegistry(DefaultRegistry, data, true, &M{})
+		if err != bsonrw.ErrInvalidJSON {
+			t.Fatalf("wanted ErrInvalidJSON, got %v", err)
+		}
+	})
 }
 
 func TestUnmarshalExtJSONWithContext(t *testing.T) {