GODRIVER-3307 clone returned byte slice in MarshalValue (#1913)

Co-authored-by: Michael Bohn <[email protected]>
Co-authored-by: Matt Dale <[email protected]>

Author: 3 people

bson/marshal.go

@@ -133,7 +133,10 @@ func MarshalValue(val interface{}) (Type, []byte, error) {
 		return 0, nil, err
 	}
 	typ := sw.Next(2)
-	return Type(typ[0]), sw.Bytes(), nil
+	clone := append([]byte{}, sw.Bytes()...) // Don't hand out a shared reference to byte buffer bytes
+	// and fully copy the data. The byte buffer is (potentially) reused
+	// and handing out only a reference to the bytes may lead to race-conditions with the buffer.
+	return Type(typ[0]), clone, nil
 }
 
 // MarshalExtJSON returns the extended JSON encoding of val.

bson/marshal_value_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 
 	"go.mongodb.org/mongo-driver/v2/internal/assert"
+	"go.mongodb.org/mongo-driver/v2/internal/require"
 )
 
 func TestMarshalValue(t *testing.T) {
@@ -33,6 +34,25 @@ func TestMarshalValue(t *testing.T) {
 			})
 		}
 	})
+
+	t.Run("returns distinct address ranges", func(t *testing.T) {
+		// Call MarshalValue in a loop with the same large value (make sure to
+		// trigger the buffer pooling, which currently doesn't happen for very
+		// small values). Compare the previous and current BSON byte slices and
+		// make sure they always have distinct memory ranges.
+		//
+		// Don't run this test in parallel to maximize the chance that we get
+		// the same pooled buffer for most/all calls.
+		largeVal := strings.Repeat("1234567890", 100_000)
+		var prev []byte
+		for i := 0; i < 20; i++ {
+			_, b, err := MarshalValue(largeVal)
+			require.NoError(t, err)
+
+			assert.DifferentAddressRanges(t, b, prev)
+			prev = b
+		}
+	})
 }
 
 func compareMarshalValueResults(t *testing.T, tc marshalValueTestCase, gotType Type, gotBytes []byte) {