Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GODRIVER-3451 Add fuzz test for BSON MarshalValue/UnmarshalValue. #1993

Merged
merged 3 commits into from
Mar 31, 2025

Conversation

matthewdale
Copy link
Collaborator

GODRIVER-3451

Summary

  • Fuzz bson.MarshalValue and bson.UnmarshalValue.
  • Enable the data race detector during fuzzing

Background & Motivation

@mongodb-drivers-pr-bot mongodb-drivers-pr-bot bot added the priority-3-low Low Priority PR for Review label Mar 21, 2025
Copy link
Contributor

API Change Report

No changes found!

@prestonvasquez
Copy link
Member

@matthewdale Can you re-configure the evergreen tasks to include Fuzz testing?

// int64, including max and min values.
int64(0), math.MaxInt64, math.MinInt64,
// string, including empty and large string.
"", strings.Repeat("z", 10_000),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we extend this to include a nested structure and a slice?

map[string]any{"nested": []any{1, "two", map[string]any{"three", 3}}}
[]any{1, 2,3, "four"}

f.Add(byte(typ), b)
}

f.Fuzz(func(t *testing.T, bsonType byte, data []byte) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK we can run these tests in parallel.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean. I believe f.Fuzz already runs the provided funcs in parallel (limited by GOMAXPROCS). Each go test invocation can only target a single fuzz test, or you get an error like this:

testing: will not fuzz, -fuzz matches more than one fuzz test: [FuzzDecodeValue FuzzDecode]

Is there another way to run fuzz tests or test cases in parallel?

t.Fatalf("failed to marshal: %v", err)
}

if err := UnmarshalValue(typ, encoded, &v); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest decoding into a new value (e.g. var v2 any). Then we can compare the original value and the re-unmarshaled value to ensure roundtrip consistency:

if !reflect.DeepEqual(v, v2) {
	t.Fatal("roundtrip mismatch: ...")
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some values seem to not round-trip exactly the same. For example:

--- FAIL: FuzzDecodeValue (0.04s)
    --- FAIL: FuzzDecodeValue (0.00s)
        decode_value_fuzz_test.go:71: 
                Error Trace:    /mongo-go-driver/bson/decode_value_fuzz_test.go:71
                                                        /mongo-go-driver/bson/value.go:581
                                                        /mongo-go-driver/bson/value.go:365
                                                        /mongo-go-driver/bson/fuzz.go:335
                Error:          Not equal: 
                                expected: bson.Regex{Pattern:"", Options:"0 "}
                                actual  : bson.Regex{Pattern:"", Options:" 0"}
                            
                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -2,3 +2,3 @@
                                  Pattern: (string) "",
                                - Options: (string) (len=2) "0 "
                                + Options: (string) (len=2) " 0"
                                 }
                Test:           FuzzDecodeValue
                Messages:       expected unmarshaled value to match; data: [0 48 32 0]

It also doesn't work for FuzzDecode:

--- FAIL: FuzzDecode (0.23s)
    --- FAIL: FuzzDecode (0.00s)
        fuzz_test.go:47: 
                Error Trace:    /mongo-go-driver/bson/fuzz_test.go:47
                                                        /mongo-go-driver/bson/value.go:581
                                                        /mongo-go-driver/bson/value.go:365
                                                        /mongo-go-driver/bson/fuzz.go:335
                Error:          Not equal: 
                                expected: &bson.D{bson.E{Key:"d", Value:NaN}}
                                actual  : &bson.D{bson.E{Key:"d", Value:NaN}}
                            
                                Diff:
                Test:           FuzzDecode
        fuzz_test.go:47: 
                Error Trace:    /mongo-go-driver/bson/fuzz_test.go:47
                                                        /mongo-go-driver/bson/value.go:581
                                                        /mongo-go-driver/bson/value.go:365
                                                        /mongo-go-driver/bson/fuzz.go:335
                Error:          Not equal: 
                                expected: &[]bson.E{bson.E{Key:"d", Value:NaN}}
                                actual  : &[]bson.E{bson.E{Key:"d", Value:NaN}}
                            
                                Diff:
                Test:           FuzzDecode
        fuzz_test.go:47: 
                Error Trace:    /mongo-go-driver/bson/fuzz_test.go:47
                                                        /mongo-go-driver/bson/value.go:581
                                                        /mongo-go-driver/bson/value.go:365
                                                        /mongo-go-driver/bson/fuzz.go:335
                Error:          Not equal: 
                                expected: &bson.M{"d":NaN}
                                actual  : &bson.M{"d":NaN}
                            
                                Diff:
                Test:           FuzzDecode
        fuzz_test.go:47: 
                Error Trace:    /mongo-go-driver/bson/fuzz_test.go:47
                                                        /mongo-go-driver/bson/value.go:581
                                                        /mongo-go-driver/bson/value.go:365
                                                        /mongo-go-driver/bson/fuzz.go:335
                Error:          Not equal: 
                                expected: (*interface {})(0x1400043d840)
                                actual  : (*interface {})(0x1400043d870)
                Test:           FuzzDecode
        fuzz_test.go:47: 
                Error Trace:    /mongo-go-driver/bson/fuzz_test.go:47
                                                        /mongo-go-driver/bson/value.go:581
                                                        /mongo-go-driver/bson/value.go:365
                                                        /mongo-go-driver/bson/fuzz.go:335
                Error:          Not equal: 
                                expected: map[string]interface {}{"d":NaN}
                                actual  : map[string]interface {}{"d":NaN}
                            
                                Diff:
                Test:           FuzzDecode

I'm not sure how to interpret some of those differences currently. However, it seems beyond the scope of this ticket to resolve them. If you think they indicate a bug, I can create a ticket to investigate.

// to "nil". It's not clear if MarshalValue should support "nil", but
// for now we skip it.
if v == nil {
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth logging such cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test logs only seems to be printed when the fuzz test encounters another error, but I can add the log statement in case it's helpful.

@matthewdale matthewdale merged commit 8c13bc5 into mongodb:master Mar 31, 2025
34 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-3-low Low Priority PR for Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants