GODRIVER-2017 Skip empty extJSON objects correctly in Unmarshal (#677)

Author: benjirewis

bson/bsonrw/extjson_reader.go

@@ -164,6 +164,23 @@ func (ejvr *extJSONValueReader) skipObject() {
 	depth := 1
 	for depth > 0 {
 		ejvr.p.advanceState()
+
+		// If object is empty, raise depth and continue. When emptyObject is true, the
+		// parser has already read both the opening and closing brackets of an empty
+		// object ("{}"), so the next valid token will be part of the parent document,
+		// not part of the nested document.
+		//
+		// If there is a comma, there are remaining fields, emptyObject must be set back
+		// to false, and comma must be skipped with advanceState().
+		if ejvr.p.emptyObject {
+			if ejvr.p.s == jpsSawComma {
+				ejvr.p.emptyObject = false
+				ejvr.p.advanceState()
+			}
+			depth--
+			continue
+		}
+
 		switch ejvr.p.s {
 		case jpsSawBeginObject, jpsSawBeginArray:
 			depth++

bson/unmarshal_test.go

@@ -234,7 +234,7 @@ func TestUnmarshalExtJSONWithUndefinedField(t *testing.T) {
 	// When unmarshalling extJSON, fields that are undefined in the destination struct are skipped.
 	// This process must not skip other, defined fields and must not raise errors.
 	type expectedResponse struct {
-		DefinedField string
+		DefinedField interface{}
 	}
 
 	unmarshalExpectedResponse := func(t *testing.T, extJSON string) *expectedResponse {
@@ -246,98 +246,128 @@ func TestUnmarshalExtJSONWithUndefinedField(t *testing.T) {
 	}
 
 	testCases := []struct {
-		name     string
-		testJSON string
+		name          string
+		testJSON      string
+		expectedValue interface{}
 	}{
 		{
 			"no array",
 			`{
 				"UndefinedField": {"key": 1},
 				"DefinedField": "value"
 			}`,
+			"value",
 		},
 		{
 			"outer array",
 			`{
 				"UndefinedField": [{"key": 1}],
 				"DefinedField": "value"
 			}`,
+			"value",
 		},
 		{
 			"embedded array",
 			`{
 				"UndefinedField": {"keys": [2]},
 				"DefinedField": "value"
 			}`,
+			"value",
 		},
 		{
 			"outer array and embedded array",
 			`{
 				"UndefinedField": [{"keys": [2]}],
 				"DefinedField": "value"
 			}`,
+			"value",
 		},
 		{
 			"embedded document",
 			`{
 				"UndefinedField": {"key": {"one": "two"}},
 				"DefinedField": "value"
 			}`,
+			"value",
 		},
 		{
 			"doubly embedded document",
 			`{
 				"UndefinedField": {"key": {"one": {"two": "three"}}},
 				"DefinedField": "value"
 			}`,
+			"value",
 		},
 		{
 			"embedded document and embedded array",
 			`{
 				"UndefinedField": {"key": {"one": {"two": [3]}}},
 				"DefinedField": "value"
 			}`,
+			"value",
 		},
 		{
 			"embedded document and embedded array in outer array",
 			`{
 				"UndefinedField": [{"key": {"one": [3]}}],
 				"DefinedField": "value"
 			}`,
+			"value",
 		},
 		{
 			"code with scope",
 			`{
 				"UndefinedField": {"logic": {"$code": "foo", "$scope": {"bar": 1}}},
 				"DefinedField": "value"
 			}`,
+			"value",
 		},
 		{
 			"embedded array of code with scope",
 			`{
 				"UndefinedField": {"logic": [{"$code": "foo", "$scope": {"bar": 1}}]},
 				"DefinedField": "value"
 			}`,
+			"value",
 		},
 		{
 			"type definition embedded document",
 			`{
 				"UndefinedField": {"myDouble": {"$numberDouble": "1.24"}},
 				"DefinedField": "value"
 			}`,
+			"value",
 		},
 		{
 			"empty embedded document",
 			`{
-				"UndefinedField": {"empty": {}},
+				"UndefinedField": {"empty": {}, "key": 1},
 				"DefinedField": "value"
 			}`,
+			"value",
+		},
+		{
+			"empty object before",
+			`{
+				"UndefinedField": {},
+				"DefinedField": {"value": "a"}
+			}`,
+			D{{"value", "a"}},
+		},
+		{
+			"empty object after",
+			`{
+				"DefinedField": {"value": "a"},
+				"UndefinedField": {}
+			}`,
+			D{{"value", "a"}},
 		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			responseDoc := unmarshalExpectedResponse(t, tc.testJSON)
-			assert.Equal(t, "value", responseDoc.DefinedField, "expected DefinedField to be 'value', got %q", responseDoc.DefinedField)
+			assert.Equal(t, tc.expectedValue, responseDoc.DefinedField, "expected DefinedField to be %v, got %q",
+				tc.expectedValue, responseDoc.DefinedField)
 		})
 	}
 }