Improve sjson package fuzzing (#3071)

Previously, only FuzzDocument was reaching the marshal+unmarshal steps due to the document type assertion. If it wasn't a documentType, the rest of the fuzzing function was skipped. Since data validation is available only via `types.Document.ValidateData`, a helper `isValidDocumentData` function is introduced. It may go away in the future. But for now, it allows all sjson types to pass the validation. The other change is related to the way we fuzz sjson. Instead of generating both values and schemes at the same time, focus on one thing at the time: a fixed scheme against the generated and fixed documents against the generated schemes. This approach can be further improved by the increased number of relevant corpus seed entries, but that's a different issue (#3067). To answer the question is it any better: yes, it is. The old approach skipped any cases where either of those two was invalid; it was a very rare occasion for a fuzz runner to generate a valid JSON for both document and scheme. It's even more rare to get both of them even remotely compatible. Closes #1273
FerretDB · Jul 19, 2023 · 4106994 · 4106994
1 parent fbe7748
commit 4106994
Show file tree

Hide file tree

Showing 15 changed files with 306 additions and 106 deletions.
diff --git a/Taskfile.yml b/Taskfile.yml
@@ -263,13 +263,15 @@ tasks:
       - go test -count=0 ./...
 
   fuzz:
-    desc: "Fuzz for about 2 minutes (with default FUZZ_TIME)"
+    desc: "Fuzz for about 3 minutes (with default FUZZ_TIME)"
     cmds:
       - go test -list='Fuzz.*' ./...
+      - go test -run=XXX -fuzz=FuzzArrayWithFixedSchemas      -fuzztime={{.FUZZ_TIME}} ./internal/handlers/sjson/
+      - go test -run=XXX -fuzz=FuzzArrayWithFixedDocuments    -fuzztime={{.FUZZ_TIME}} ./internal/handlers/sjson/
+      - go test -run=XXX -fuzz=FuzzDocumentWithFixedSchemas   -fuzztime={{.FUZZ_TIME}} ./internal/handlers/sjson/
+      - go test -run=XXX -fuzz=FuzzDocumentWithFixedDocuments -fuzztime={{.FUZZ_TIME}} ./internal/handlers/sjson/
       - go test -run=XXX -fuzz=FuzzArray    -fuzztime={{.FUZZ_TIME}} ./internal/bson/
       - go test -run=XXX -fuzz=FuzzDocument -fuzztime={{.FUZZ_TIME}} ./internal/bson/
-      - go test -run=XXX -fuzz=FuzzArray    -fuzztime={{.FUZZ_TIME}} ./internal/handlers/sjson/
-      - go test -run=XXX -fuzz=FuzzDocument -fuzztime={{.FUZZ_TIME}} ./internal/handlers/sjson/
       - go test -run=XXX -fuzz=FuzzMsg      -fuzztime={{.FUZZ_TIME}} ./internal/wire/
       - go test -run=XXX -fuzz=FuzzQuery    -fuzztime={{.FUZZ_TIME}} ./internal/wire/
       - go test -run=XXX -fuzz=FuzzReply    -fuzztime={{.FUZZ_TIME}} ./internal/wire/

diff --git a/internal/handlers/sjson/array_test.go b/internal/handlers/sjson/array_test.go
@@ -58,6 +58,63 @@ var arrayTestCases = []testCase{
 			},
 		},
 		j: `[[],"Qg==",true,1627378542123,{},42.13,42,42,"foo",null]`,
+	}, {
+		// Unlike "array_all" case, this one passes the Fuzz checks successfully.
+		// Most notably, it doesn't have a nested array that is rejected by the data validation.
+		name: "array_without_nested",
+		v: convertArray(must.NotFail(types.NewArray(
+			"",
+			false,
+			must.NotFail(types.NewDocument("key", "value")),
+			-32589.300000101,
+			int32(349),
+			int64(-21424182),
+			types.Null,
+			"a string that is somewhat longer",
+		))),
+		sch: &elem{
+			Type: elemTypeArray,
+			Items: []*elem{
+				stringSchema,
+				boolSchema,
+				{
+					Type: elemTypeObject,
+					Schema: &schema{
+						Properties: map[string]*elem{
+							"key": stringSchema,
+						},
+						Keys: []string{"key"},
+					},
+				},
+				doubleSchema,
+				intSchema,
+				longSchema,
+				nullSchema,
+				stringSchema,
+			},
+		},
+		j: `["",false,{"key":"value"},-32589.300000101,349,-21424182,null,"a string that is somewhat longer"]`,
+	}, {
+		name: "schema_null_error",
+		v:    convertArray(must.NotFail(types.NewArray("a"))),
+		sch: &elem{
+			Type:  elemTypeArray,
+			Items: []*elem{nullSchema},
+		},
+		j:    `["a"]`,
+		jErr: `sjson.unmarshalSingleValue: expected null, got "a"`,
+	}, {
+		name: "array10",
+		v:    convertArray(must.NotFail(types.NewArray(0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0))),
+		sch: &elem{
+			Type: elemTypeArray,
+			Items: []*elem{
+				doubleSchema, doubleSchema, doubleSchema, doubleSchema, doubleSchema,
+				doubleSchema, doubleSchema, doubleSchema, doubleSchema, doubleSchema,
+			},
+		},
+		j:      `[0,1.0,2,3.0,4.0,5.0,6,7,8,9]`,
+		canonJ: `[0,1,2,3,4,5,6,7,8,9]`,
 	}, {
 		name: "EOF",
 		sch:  &elem{Type: elemTypeArray, Items: []*elem{}},
@@ -86,8 +143,12 @@ func TestArray(t *testing.T) {
 	testJSON(t, arrayTestCases, func() sjsontype { return new(arrayType) })
 }
 
-func FuzzArray(f *testing.F) {
-	fuzzJSON(f, arrayTestCases, func() sjsontype { return new(arrayType) })
+func FuzzArrayWithFixedSchemas(f *testing.F) {
+	fuzzJSONWithFixedSchemas(f, arrayTestCases, func() sjsontype { return new(arrayType) })
+}
+
+func FuzzArrayWithFixedDocuments(f *testing.F) {
+	fuzzJSONWithFixedDocuments(f, arrayTestCases, func() sjsontype { return new(arrayType) })
 }
 
 func BenchmarkArray(b *testing.B) {

diff --git a/internal/handlers/sjson/binary_test.go b/internal/handlers/sjson/binary_test.go
@@ -63,8 +63,12 @@ func TestBinary(t *testing.T) {
 	testJSON(t, binaryTestCases, func() sjsontype { return new(binaryType) })
 }
 
-func FuzzBinary(f *testing.F) {
-	fuzzJSON(f, binaryTestCases, func() sjsontype { return new(binaryType) })
+func FuzzBinaryWithFixedSchemas(f *testing.F) {
+	fuzzJSONWithFixedSchemas(f, binaryTestCases, func() sjsontype { return new(binaryType) })
+}
+
+func FuzzBinaryWithFixedDocuments(f *testing.F) {
+	fuzzJSONWithFixedDocuments(f, binaryTestCases, func() sjsontype { return new(binaryType) })
 }
 
 func BenchmarkBinary(b *testing.B) {

diff --git a/internal/handlers/sjson/bool_test.go b/internal/handlers/sjson/bool_test.go
@@ -35,8 +35,12 @@ func TestBool(t *testing.T) {
 	testJSON(t, boolTestCases, func() sjsontype { return new(boolType) })
 }
 
-func FuzzBool(f *testing.F) {
-	fuzzJSON(f, boolTestCases, func() sjsontype { return new(boolType) })
+func FuzzBoolWithFixedSchemas(f *testing.F) {
+	fuzzJSONWithFixedSchemas(f, boolTestCases, func() sjsontype { return new(boolType) })
+}
+
+func FuzzBoolWithFixedDocuments(f *testing.F) {
+	fuzzJSONWithFixedDocuments(f, boolTestCases, func() sjsontype { return new(boolType) })
 }
 
 func BenchmarkBool(b *testing.B) {

diff --git a/internal/handlers/sjson/datetime_test.go b/internal/handlers/sjson/datetime_test.go
@@ -48,8 +48,12 @@ func TestDateTime(t *testing.T) {
 	testJSON(t, dateTimeTestCases, func() sjsontype { return new(dateTimeType) })
 }
 
-func FuzzDateTime(f *testing.F) {
-	fuzzJSON(f, dateTimeTestCases, func() sjsontype { return new(dateTimeType) })
+func FuzzDateTimeWithFixedSchemas(f *testing.F) {
+	fuzzJSONWithFixedSchemas(f, dateTimeTestCases, func() sjsontype { return new(dateTimeType) })
+}
+
+func FuzzDateTimeWithFixedDocuments(f *testing.F) {
+	fuzzJSONWithFixedDocuments(f, dateTimeTestCases, func() sjsontype { return new(dateTimeType) })
 }
 
 func BenchmarkDateTime(b *testing.B) {

diff --git a/internal/handlers/sjson/document_test.go b/internal/handlers/sjson/document_test.go
@@ -464,8 +464,12 @@ func TestDocument(t *testing.T) {
 	testJSON(t, documentTestCases, func() sjsontype { return new(documentType) })
 }
 
-func FuzzDocument(f *testing.F) {
-	fuzzJSON(f, documentTestCases, func() sjsontype { return new(documentType) })
+func FuzzDocumentWithFixedSchemas(f *testing.F) {
+	fuzzJSONWithFixedSchemas(f, documentTestCases, func() sjsontype { return new(documentType) })
+}
+
+func FuzzDocumentWithFixedDocuments(f *testing.F) {
+	fuzzJSONWithFixedDocuments(f, documentTestCases, func() sjsontype { return new(documentType) })
 }
 
 func BenchmarkDocument(b *testing.B) {

diff --git a/internal/handlers/sjson/double_test.go b/internal/handlers/sjson/double_test.go
@@ -48,8 +48,12 @@ func TestDouble(t *testing.T) {
 	testJSON(t, doubleTestCases, func() sjsontype { return new(doubleType) })
 }
 
-func FuzzDouble(f *testing.F) {
-	fuzzJSON(f, doubleTestCases, func() sjsontype { return new(doubleType) })
+func FuzzDoubleWithFixedSchemas(f *testing.F) {
+	fuzzJSONWithFixedSchemas(f, doubleTestCases, func() sjsontype { return new(doubleType) })
+}
+
+func FuzzDoubleWithFixedDocuments(f *testing.F) {
+	fuzzJSONWithFixedDocuments(f, doubleTestCases, func() sjsontype { return new(doubleType) })
 }
 
 func BenchmarkDouble(b *testing.B) {

diff --git a/internal/handlers/sjson/int32_test.go b/internal/handlers/sjson/int32_test.go
@@ -44,8 +44,12 @@ func TestInt32(t *testing.T) {
 	testJSON(t, int32TestCases, func() sjsontype { return new(int32Type) })
 }
 
-func FuzzInt32(f *testing.F) {
-	fuzzJSON(f, int32TestCases, func() sjsontype { return new(int32Type) })
+func FuzzInt32WithFixedSchemas(f *testing.F) {
+	fuzzJSONWithFixedSchemas(f, int32TestCases, func() sjsontype { return new(int32Type) })
+}
+
+func FuzzInt32WithFixedDocuments(f *testing.F) {
+	fuzzJSONWithFixedDocuments(f, int32TestCases, func() sjsontype { return new(int32Type) })
 }
 
 func BenchmarkInt32(b *testing.B) {

diff --git a/internal/handlers/sjson/int64_test.go b/internal/handlers/sjson/int64_test.go
@@ -48,8 +48,12 @@ func TestInt64(t *testing.T) {
 	testJSON(t, int64TestCases, func() sjsontype { return new(int64Type) })
 }
 
-func FuzzInt64(f *testing.F) {
-	fuzzJSON(f, int64TestCases, func() sjsontype { return new(int64Type) })
+func FuzzInt64WithFixedSchemas(f *testing.F) {
+	fuzzJSONWithFixedSchemas(f, int64TestCases, func() sjsontype { return new(int64Type) })
+}
+
+func FuzzInt64WithFixedDocuments(f *testing.F) {
+	fuzzJSONWithFixedDocuments(f, int64TestCases, func() sjsontype { return new(int64Type) })
 }
 
 func BenchmarkInt64(b *testing.B) {

diff --git a/internal/handlers/sjson/object_id_test.go b/internal/handlers/sjson/object_id_test.go
@@ -35,8 +35,12 @@ func TestObjectID(t *testing.T) {
 	testJSON(t, objectIDTestCases, func() sjsontype { return new(objectIDType) })
 }
 
-func FuzzObjectID(f *testing.F) {
-	fuzzJSON(f, objectIDTestCases, func() sjsontype { return new(objectIDType) })
+func FuzzObjectIDWithFixedSchemas(f *testing.F) {
+	fuzzJSONWithFixedSchemas(f, objectIDTestCases, func() sjsontype { return new(objectIDType) })
+}
+
+func FuzzObjectIDWithFixedDocuments(f *testing.F) {
+	fuzzJSONWithFixedDocuments(f, objectIDTestCases, func() sjsontype { return new(objectIDType) })
 }
 
 func BenchmarkObjectID(b *testing.B) {

diff --git a/internal/handlers/sjson/regex_test.go b/internal/handlers/sjson/regex_test.go
@@ -50,8 +50,12 @@ func TestRegex(t *testing.T) {
 	testJSON(t, regexTestCases, func() sjsontype { return new(regexType) })
 }
 
-func FuzzRegex(f *testing.F) {
-	fuzzJSON(f, regexTestCases, func() sjsontype { return new(regexType) })
+func FuzzRegexWithFixedSchemas(f *testing.F) {
+	fuzzJSONWithFixedSchemas(f, regexTestCases, func() sjsontype { return new(regexType) })
+}
+
+func FuzzRegexWithFixedDocuments(f *testing.F) {
+	fuzzJSONWithFixedDocuments(f, regexTestCases, func() sjsontype { return new(regexType) })
 }
 
 func BenchmarkRegex(b *testing.B) {

diff --git a/internal/handlers/sjson/sjson.go b/internal/handlers/sjson/sjson.go
@@ -293,7 +293,7 @@ func unmarshalSingleValue(data json.RawMessage, sch *elem) (any, error) {
 		err = d.UnmarshalJSON(data)
 		res = &d
 	case elemTypeNull:
-		panic(fmt.Sprintf("must not be called, was called with %s", string(data)))
+		return nil, lazyerrors.Errorf("sjson.unmarshalSingleValue: expected null, got %s", data)
 	case elemTypeRegex:
 		var r regexType
 		err = r.UnmarshalJSONWithSchema(data, sch)