From 9faa0563b38f5f1445392581a9cb27015dcc8dd7 Mon Sep 17 00:00:00 2001 From: Michael Bohn Date: Mon, 23 Dec 2024 14:50:01 +0100 Subject: [PATCH 1/3] clone returned byte slice in MarshalValue --- bson/marshal.go | 3 ++- bson/marshal_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/bson/marshal.go b/bson/marshal.go index fdec2fe896..5f2a182d78 100644 --- a/bson/marshal.go +++ b/bson/marshal.go @@ -9,6 +9,7 @@ package bson import ( "bytes" "encoding/json" + "slices" "sync" ) @@ -133,7 +134,7 @@ func MarshalValue(val interface{}) (Type, []byte, error) { return 0, nil, err } typ := sw.Next(2) - return Type(typ[0]), sw.Bytes(), nil + return Type(typ[0]), slices.Clone(sw.Bytes()), nil } // MarshalExtJSON returns the extended JSON encoding of val. diff --git a/bson/marshal_test.go b/bson/marshal_test.go index 8ede9627ba..43bc337521 100644 --- a/bson/marshal_test.go +++ b/bson/marshal_test.go @@ -326,3 +326,61 @@ func TestMarshalConcurrently(t *testing.T) { } wg.Wait() } + +type testStruct struct { + TestData RawValue `bson:"testData"` +} + +func TestSharedUseOfMarshalledBytes(t *testing.T) { + type testValue struct { + value string + } + + type sharedTestCase struct { + name string + testData *testStruct + wantValue string + } + + // fill the pool with some buffers + for i := 0; i < 100; i++ { + mustMarshalValue(testValue{value: fmt.Sprintf("marshalled foo bar %d", i)}) + } + + testCases := []sharedTestCase{ + { + name: "case 1", + testData: mustMarshalValue(testValue{value: "Case 1 Value"}), + wantValue: "Case 1 Value", + }, + { + name: "case 2", + testData: mustMarshalValue(testValue{value: "Case 2 Value"}), + wantValue: "Case 2 Value", + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + want := mustMarshalValue(testValue{value: tc.wantValue}) + if d := cmp.Diff(tc.testData, want); d != "" { + t.Errorf("diff: %s", d) + } + }) + } +} + +func mustMarshalValue(data any) *testStruct { + dataType, dataBytes, err := MarshalValue(data) + if err != nil { + panic(fmt.Sprintf("unable to marshal data: %s", err)) + } + + return &testStruct{ + TestData: RawValue{ + Type: dataType, + Value: dataBytes, + }, + } +} From fc53dd63a7e55fa9c65ab88978b8d7e074aed8b8 Mon Sep 17 00:00:00 2001 From: Michael Bohn Date: Sat, 4 Jan 2025 14:39:59 +0100 Subject: [PATCH 2/3] clone pre go 1.21 compatible way --- bson/marshal.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bson/marshal.go b/bson/marshal.go index 5f2a182d78..1ab60b1707 100644 --- a/bson/marshal.go +++ b/bson/marshal.go @@ -9,7 +9,6 @@ package bson import ( "bytes" "encoding/json" - "slices" "sync" ) @@ -134,7 +133,8 @@ func MarshalValue(val interface{}) (Type, []byte, error) { return 0, nil, err } typ := sw.Next(2) - return Type(typ[0]), slices.Clone(sw.Bytes()), nil + clone := append([]byte(nil), sw.Bytes()...) + return Type(typ[0]), clone, nil } // MarshalExtJSON returns the extended JSON encoding of val. From ddf29bfdd3111d35580bd6a9ddbe2bb06017dd6d Mon Sep 17 00:00:00 2001 From: Michael Bohn Date: Sat, 4 Jan 2025 14:57:15 +0100 Subject: [PATCH 3/3] add a comment explaining the clone --- bson/marshal.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bson/marshal.go b/bson/marshal.go index 1ab60b1707..5cdeece8c1 100644 --- a/bson/marshal.go +++ b/bson/marshal.go @@ -133,7 +133,9 @@ func MarshalValue(val interface{}) (Type, []byte, error) { return 0, nil, err } typ := sw.Next(2) - clone := append([]byte(nil), sw.Bytes()...) + clone := append([]byte(nil), 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 }