Fixes PYTHON-5126 and adopts agreement that ignored bits raise if non-zero

Author: caseyclements

bson/binary.py

@@ -462,6 +462,10 @@ def from_vector(
                 raise ValueError(f"{padding=}. It must be in [0,1, ..7].")
             if padding and not vector:
                 raise ValueError("Empty vector with non-zero padding.")
+            if padding and not (vector[-1] & ((1 << padding) - 1)) == 0:
+                raise ValueError(
+                    "If padding p is provided, all bits in the final byte lower than p must be 0."
+                )
         elif dtype == BinaryVectorDtype.FLOAT32:  # pack floats as float32
             format_str = "f"
             if padding:
@@ -490,6 +494,11 @@ def as_vector(self) -> BinaryVector:
         dtype = BinaryVectorDtype(dtype)
         n_values = len(self) - position
 
+        if padding and dtype != BinaryVectorDtype.PACKED_BIT:
+            raise ValueError(
+                f"Corrupt data. Padding ({padding}) must be 0 for all but PACKED_BIT dtypes. ({dtype=})"
+            )
+
         if dtype == BinaryVectorDtype.INT8:
             dtype_format = "b"
             format_string = f"<{n_values}{dtype_format}"
@@ -513,6 +522,12 @@ def as_vector(self) -> BinaryVector:
             dtype_format = "B"
             format_string = f"<{n_values}{dtype_format}"
             unpacked_uint8s = list(struct.unpack_from(format_string, self, position))
+            if padding and not n_values:
+                raise ValueError("Corrupt data. Vector has a padding P, but no data.")
+            if padding and n_values and not (unpacked_uint8s[-1] & ((1 << padding) - 1)) == 0:
+                raise ValueError(
+                    "Corrupt data. Vector has a padding P, but bits in the final byte lower than P are non-zero."
+                )
             return BinaryVector(unpacked_uint8s, dtype, padding)
 
         else:

test/bson_binary_vector/packed_bit.json

@@ -14,30 +14,39 @@
     {
       "description": "Simple Vector PACKED_BIT",
       "valid": true,
-      "vector": [127, 7],
+      "vector": [127, 8],
       "dtype_hex": "0x10",
       "dtype_alias": "PACKED_BIT",
       "padding": 0,
-      "canonical_bson": "1600000005766563746F7200040000000910007F0700"
+      "canonical_bson": "1600000005766563746F7200040000000910007F0800"
     },
     {
-      "description": "Empty Vector PACKED_BIT",
+      "description": "PACKED_BIT with padding",
       "valid": true,
-      "vector": [],
+      "vector": [127, 8],
       "dtype_hex": "0x10",
       "dtype_alias": "PACKED_BIT",
-      "padding": 0,
-      "canonical_bson": "1400000005766563746F72000200000009100000"
+      "padding": 3,
+      "canonical_bson": "1600000005766563746F7200040000000910037F0800"
     },
     {
-      "description": "PACKED_BIT with padding",
-      "valid": true,
+      "description": "PACKED_BIT with inconsistent padding",
+      "valid": false,
       "vector": [127, 7],
       "dtype_hex": "0x10",
       "dtype_alias": "PACKED_BIT",
       "padding": 3,
       "canonical_bson": "1600000005766563746F7200040000000910037F0700"
     },
+    {
+      "description": "Empty Vector PACKED_BIT",
+      "valid": true,
+      "vector": [],
+      "dtype_hex": "0x10",
+      "dtype_alias": "PACKED_BIT",
+      "padding": 0,
+      "canonical_bson": "1400000005766563746F72000200000009100000"
+    },
     {
       "description": "Overflow Vector PACKED_BIT",
       "valid": false,

test/test_bson_binary_vector.py

@@ -48,11 +48,11 @@ def create_test(case_spec):
     def run_test(self):
         for test_case in case_spec.get("tests", []):
             description = test_case["description"]
-            vector_exp = test_case.get("vector", [])
+            vector_exp = test_case.get("vector", None)
             dtype_hex_exp = test_case["dtype_hex"]
             dtype_alias_exp = test_case.get("dtype_alias")
             padding_exp = test_case.get("padding", 0)
-            canonical_bson_exp = test_case.get("canonical_bson")
+            canonical_bson_exp = test_case.get("canonical_bson", None)
             # Convert dtype hex string into bytes
             dtype_exp = BinaryVectorDtype(int(dtype_hex_exp, 16).to_bytes(1, byteorder="little"))
 
@@ -85,14 +85,25 @@ def run_test(self):
                 self.assertEqual(cB_obs, canonical_bson_exp, description)
 
             else:
-                with self.assertRaises((struct.error, ValueError), msg=description):
-                    # Tests Binary.from_vector
-                    Binary.from_vector(vector_exp, dtype_exp, padding_exp)
-                    # Tests Binary.as_vector
-                    cB_exp = binascii.unhexlify(canonical_bson_exp.encode("utf8"))
-                    decoded_doc = decode(cB_exp)
-                    binary_obs = decoded_doc[test_key]
-                    binary_obs.as_vector()
+                """
+                #### To prove correct in an invalid case (`valid:false`), one MUST
+                - if the vector field is present, raise an exception when attempting to encode a document from the numeric values,
+                dtype, and padding.
+                - if the canonical_bson field is present, raise an exception when attempting to deserialize it into the corresponding
+                    numeric values, as the field contains corrupted data.
+                """
+                # Tests Binary.from_vector()
+                if vector_exp is not None:
+                    with self.assertRaises((struct.error, ValueError), msg=description):
+                        Binary.from_vector(vector_exp, dtype_exp, padding_exp)
+
+                # Tests Binary.as_vector()
+                if canonical_bson_exp is not None:
+                    with self.assertRaises((struct.error, ValueError), msg=description):
+                        cB_exp = binascii.unhexlify(canonical_bson_exp.encode("utf8"))
+                        decoded_doc = decode(cB_exp)
+                        binary_obs = decoded_doc[test_key]
+                        binary_obs.as_vector()
 
     return run_test