Merge branch 'master' into godriver3098-36-eol

Author: matthewdale

.evergreen/config.yml

@@ -561,6 +561,25 @@ functions:
           KMS_MOCK_SERVERS_RUNNING: "true"
         args: [*task-runner, evg-test-kmip]
 
+  run-retry-kms-requests:
+    - command: subprocess.exec
+      type: test
+      params:
+        binary: "bash"
+        env: 
+          GO_BUILD_TAGS: cse
+        include_expansions_in_env: [AUTH, SSL, MONGODB_URI, TOPOLOGY, 
+          MONGO_GO_DRIVER_COMPRESSOR]
+        args: [*task-runner, setup-test]
+    - command: subprocess.exec
+      type: test
+      params:
+        binary: "bash"
+        env:
+          KMS_FAILPOINT_CA_FILE: "${DRIVERS_TOOLS}/.evergreen/x509gen/ca.pem"
+          KMS_FAILPOINT_SERVER_RUNNING: "true"
+        args: [*task-runner, evg-test-retry-kms-requests]
+
   run-fuzz-tests:
     - command: subprocess.exec
       type: test
@@ -1468,7 +1487,7 @@ tasks:
           SSL: "nossl"
 
   - name: "test-kms-tls-invalid-cert"
-    tags: ["kms-tls"]
+    tags: ["kms-test"]
     commands:
       - func: bootstrap-mongo-orchestration
         vars:
@@ -1484,7 +1503,7 @@ tasks:
           SSL: "nossl"
 
   - name: "test-kms-tls-invalid-hostname"
-    tags: ["kms-tls"]
+    tags: ["kms-test"]
     commands:
       - func: bootstrap-mongo-orchestration
         vars:
@@ -1514,6 +1533,17 @@ tasks:
           AUTH: "noauth"
           SSL: "nossl"
 
+  - name: "test-retry-kms-requests"
+    tags: ["kms-test"]
+    commands:
+      - func: bootstrap-mongo-orchestration
+        vars:
+          TOPOLOGY: "server"
+          AUTH: "noauth"
+          SSL: "nossl"
+      - func: start-cse-servers
+      - func: run-retry-kms-requests
+
   - name: "test-serverless"
     tags: ["serverless"]
     commands:
@@ -2184,11 +2214,11 @@ buildvariants:
     tasks:
       - name: ".versioned-api"
 
-  - matrix_name: "kms-tls-test"
+  - matrix_name: "kms-test"
     matrix_spec: { version: ["7.0"], os-ssl-40: ["rhel87-64"] }
-    display_name: "KMS TLS ${os-ssl-40}"
+    display_name: "KMS TEST ${os-ssl-40}"
     tasks:
-      - name: ".kms-tls"
+      - name: ".kms-test"
 
   - matrix_name: "load-balancer-test"
     tags: ["pullrequest"]

Taskfile.yml

@@ -143,6 +143,9 @@ tasks:
   evg-test-kms:
     - go test -exec "env PKG_CONFIG_PATH=${PKG_CONFIG_PATH} LD_LIBRARY_PATH=${LD_LIBRARY_PATH}" ${BUILD_TAGS} -v -timeout {{.TEST_TIMEOUT}}s ./internal/integration -run TestClientSideEncryptionProse/kms_tls_tests >> test.suite
 
+  evg-test-retry-kms-requests:
+    - go test -exec "env PKG_CONFIG_PATH=${PKG_CONFIG_PATH} LD_LIBRARY_PATH=${LD_LIBRARY_PATH}" ${BUILD_TAGS} -v -timeout {{.TEST_TIMEOUT}}s ./internal/integration -run TestClientSideEncryptionProse/kms_retry_tests >> test.suite
+
   evg-test-load-balancers:
     # Load balancer should be tested with all unified tests as well as tests in the following
     # components: retryable reads, retryable writes, change streams, initial DNS seedlist discovery.

bson/bson_binary_vector_spec_test.go

@@ -9,7 +9,6 @@ package bson
 import (
 	"encoding/hex"
 	"encoding/json"
-	"fmt"
 	"math"
 	"os"
 	"path"
@@ -64,53 +63,13 @@ func TestBsonBinaryVectorSpec(t *testing.T) {
 		})
 	}
 
-	t.Run("FLOAT32 with padding", func(t *testing.T) {
-		t.Parallel()
-
-		t.Run("Unmarshaling", func(t *testing.T) {
-			val := D{{"vector", Binary{Subtype: TypeBinaryVector, Data: []byte{Float32Vector, 3}}}}
-			b, err := Marshal(val)
-			require.NoError(t, err, "marshaling test BSON")
-			var got struct {
-				Vector Vector
-			}
-			err = Unmarshal(b, &got)
-			require.ErrorContains(t, err, errNonZeroVectorPadding.Error())
-		})
-	})
-
-	t.Run("INT8 with padding", func(t *testing.T) {
-		t.Parallel()
-
-		t.Run("Unmarshaling", func(t *testing.T) {
-			val := D{{"vector", Binary{Subtype: TypeBinaryVector, Data: []byte{Int8Vector, 3}}}}
-			b, err := Marshal(val)
-			require.NoError(t, err, "marshaling test BSON")
-			var got struct {
-				Vector Vector
-			}
-			err = Unmarshal(b, &got)
-			require.ErrorContains(t, err, errNonZeroVectorPadding.Error())
-		})
-	})
-
 	t.Run("Padding specified with no vector data PACKED_BIT", func(t *testing.T) {
 		t.Parallel()
 
 		t.Run("Marshaling", func(t *testing.T) {
 			_, err := NewPackedBitVector(nil, 1)
 			require.EqualError(t, err, errNonZeroVectorPadding.Error())
 		})
-		t.Run("Unmarshaling", func(t *testing.T) {
-			val := D{{"vector", Binary{Subtype: TypeBinaryVector, Data: []byte{PackedBitVector, 1}}}}
-			b, err := Marshal(val)
-			require.NoError(t, err, "marshaling test BSON")
-			var got struct {
-				Vector Vector
-			}
-			err = Unmarshal(b, &got)
-			require.ErrorContains(t, err, errNonZeroVectorPadding.Error())
-		})
 	})
 
 	t.Run("Exceeding maximum padding PACKED_BIT", func(t *testing.T) {
@@ -120,47 +79,9 @@ func TestBsonBinaryVectorSpec(t *testing.T) {
 			_, err := NewPackedBitVector(nil, 8)
 			require.EqualError(t, err, errVectorPaddingTooLarge.Error())
 		})
-		t.Run("Unmarshaling", func(t *testing.T) {
-			val := D{{"vector", Binary{Subtype: TypeBinaryVector, Data: []byte{PackedBitVector, 8}}}}
-			b, err := Marshal(val)
-			require.NoError(t, err, "marshaling test BSON")
-			var got struct {
-				Vector Vector
-			}
-			err = Unmarshal(b, &got)
-			require.ErrorContains(t, err, errVectorPaddingTooLarge.Error())
-		})
 	})
 }
 
-// TODO: This test may be added into the spec tests.
-func TestFloat32VectorWithInsufficientData(t *testing.T) {
-	t.Parallel()
-
-	val := Binary{Subtype: TypeBinaryVector}
-
-	for _, tc := range [][]byte{
-		{Float32Vector, 0, 42},
-		{Float32Vector, 0, 42, 42},
-		{Float32Vector, 0, 42, 42, 42},
-
-		{Float32Vector, 0, 42, 42, 42, 42, 42},
-		{Float32Vector, 0, 42, 42, 42, 42, 42, 42},
-		{Float32Vector, 0, 42, 42, 42, 42, 42, 42, 42},
-	} {
-		t.Run(fmt.Sprintf("marshaling %d bytes", len(tc)-2), func(t *testing.T) {
-			val.Data = tc
-			b, err := Marshal(D{{"vector", val}})
-			require.NoError(t, err, "marshaling test BSON")
-			var got struct {
-				Vector Vector
-			}
-			err = Unmarshal(b, &got)
-			require.ErrorContains(t, err, errInsufficientVectorData.Error())
-		})
-	}
-}
-
 func convertSlice[T int8 | float32 | byte](s []interface{}) []T {
 	v := make([]T, len(s))
 	for i, e := range s {
@@ -208,33 +129,44 @@ func runBsonBinaryVectorTest(t *testing.T, testKey string, test bsonBinaryVector
 
 	t.Run("Unmarshaling", func(t *testing.T) {
 		skipCases := map[string]string{
-			"FLOAT32 with padding":                             "run in alternative case",
-			"Overflow Vector INT8":                             "compile-time restriction",
-			"Underflow Vector INT8":                            "compile-time restriction",
-			"INT8 with padding":                                "run in alternative case",
-			"INT8 with float inputs":                           "compile-time restriction",
-			"Overflow Vector PACKED_BIT":                       "compile-time restriction",
-			"Underflow Vector PACKED_BIT":                      "compile-time restriction",
-			"Vector with float values PACKED_BIT":              "compile-time restriction",
-			"Padding specified with no vector data PACKED_BIT": "run in alternative case",
-			"Exceeding maximum padding PACKED_BIT":             "run in alternative case",
-			"Negative padding PACKED_BIT":                      "compile-time restriction",
+			"Overflow Vector INT8":                "compile-time restriction",
+			"Underflow Vector INT8":               "compile-time restriction",
+			"INT8 with float inputs":              "compile-time restriction",
+			"Overflow Vector PACKED_BIT":          "compile-time restriction",
+			"Underflow Vector PACKED_BIT":         "compile-time restriction",
+			"Vector with float values PACKED_BIT": "compile-time restriction",
+			"Negative padding PACKED_BIT":         "compile-time restriction",
 		}
 		if reason, ok := skipCases[test.Description]; ok {
 			t.Skipf("skip test case %s: %s", test.Description, reason)
 		}
 
+		errMap := map[string]string{
+			"FLOAT32 with padding":                             "padding must be 0",
+			"INT8 with padding":                                "padding must be 0",
+			"Padding specified with no vector data PACKED_BIT": "padding must be 0",
+			"Exceeding maximum padding PACKED_BIT":             "padding cannot be larger than 7",
+		}
+
 		t.Parallel()
 
 		var got map[string]Vector
 		err := Unmarshal(testBSON, &got)
-		require.NoError(t, err)
-		require.Equal(t, testVector, got)
+		if test.Valid {
+			require.NoError(t, err)
+			require.Equal(t, testVector, got)
+		} else if errMsg, ok := errMap[test.Description]; ok {
+			require.ErrorContains(t, err, errMsg)
+		} else {
+			require.Error(t, err)
+		}
 	})
 
 	t.Run("Marshaling", func(t *testing.T) {
 		skipCases := map[string]string{
 			"FLOAT32 with padding":                             "private padding field",
+			"Insufficient vector data with 3 bytes FLOAT32":    "invalid case",
+			"Insufficient vector data with 5 bytes FLOAT32":    "invalid case",
 			"Overflow Vector INT8":                             "compile-time restriction",
 			"Underflow Vector INT8":                            "compile-time restriction",
 			"INT8 with padding":                                "private padding field",

bson/default_value_decoders.go

@@ -1221,7 +1221,13 @@ func valueUnmarshalerDecodeValue(_ DecodeContext, vr ValueReader, val reflect.Va
 		return ValueDecoderError{Name: "ValueUnmarshalerDecodeValue", Types: []reflect.Type{tValueUnmarshaler}, Received: val}
 	}
 
-	if vr.Type() == TypeNull {
+	// If BSON value is null and the go value is a pointer, then don't call
+	// UnmarshalBSONValue. Even if the Go pointer is already initialized (i.e.,
+	// non-nil), encountering null in BSON will result in the pointer being
+	// directly set to nil here. Since the pointer is being replaced with nil,
+	// there is no opportunity (or reason) for the custom UnmarshalBSONValue logic
+	// to be called.
+	if vr.Type() == TypeNull && val.Kind() == reflect.Ptr {
 		val.Set(reflect.Zero(val.Type()))
 
 		return vr.ReadNull()

bson/unmarshal.go

@@ -36,7 +36,11 @@ type ValueUnmarshaler interface {
 }
 
 // Unmarshal parses the BSON-encoded data and stores the result in the value
-// pointed to by val. If val is nil or not a pointer, Unmarshal returns an error.
+// pointed to by val. If val is nil or not a pointer, Unmarshal returns an
+// error.
+//
+// When unmarshaling BSON, if the BSON value is null and the Go value is a
+// pointer, the pointer is set to nil without calling UnmarshalBSONValue.
 func Unmarshal(data []byte, val interface{}) error {
 	vr := newDocumentReader(bytes.NewReader(data))
 	if l, err := vr.peekLength(); err != nil {

bson/unmarshal_value_test.go

@@ -13,6 +13,7 @@ import (
 	"testing"
 
 	"go.mongodb.org/mongo-driver/v2/internal/assert"
+	"go.mongodb.org/mongo-driver/v2/internal/require"
 	"go.mongodb.org/mongo-driver/v2/x/bsonx/bsoncore"
 )
 
@@ -33,6 +34,26 @@ func TestUnmarshalValue(t *testing.T) {
 	}
 }
 
+func TestInitializedPointerDataWithBSONNull(t *testing.T) {
+	// Set up the test case with initialized pointers.
+	tc := unmarshalBehaviorTestCase{
+		BSONValuePtrTracker: &unmarshalBSONValueCallTracker{},
+		BSONPtrTracker:      &unmarshalBSONCallTracker{},
+	}
+	// Create BSON data where the '*_ptr_tracker' fields are explicitly set to
+	// null.
+	bytes := docToBytes(D{
+		{Key: "bv_ptr_tracker", Value: nil},
+		{Key: "b_ptr_tracker", Value: nil},
+	})
+	// Unmarshal the BSON data into the test case struct. This should set the
+	// pointer fields to nil due to the BSON null value.
+	err := Unmarshal(bytes, &tc)
+	require.NoError(t, err)
+	assert.Nil(t, tc.BSONValuePtrTracker)
+	assert.Nil(t, tc.BSONPtrTracker)
+}
+
 // tests covering GODRIVER-2779
 func BenchmarkSliceCodecUnmarshal(b *testing.B) {
 	benchmarks := []struct {