Merge pull request #2024 from sydduckworth/blank-checksums-invalid

Fixed empty checksum validation

Author: sydduckworth

asdf/_block/reader.py

@@ -76,11 +76,14 @@ def data(self):
             data = self._data()
         else:
             data = self._data
+
         if self.validate_checksum:
-            checksum = bio.calculate_block_checksum(data)
-            if not self._header["flags"] & constants.BLOCK_FLAG_STREAMED and checksum != self._header["checksum"]:
-                msg = f"Block at {self.offset} does not match given checksum"
-                raise ValueError(msg)
+            if any(b != 0 for b in self._header["checksum"]):
+                # Only validate if the header actually contains a checksum
+                checksum = bio.calculate_block_checksum(data)
+                if not self._header["flags"] & constants.BLOCK_FLAG_STREAMED and checksum != self._header["checksum"]:
+                    msg = f"Block at {self.offset} does not match given checksum"
+                    raise ValueError(msg)
             # only validate data the first time it's read
             self.validate_checksum = False
         return data

asdf/_tests/_block/test_reader.py

@@ -175,22 +175,37 @@ def test_closed_file(tmp_path):
         blk.load()
 
 
-@pytest.mark.parametrize("validate_checksums", [True, False])
-def test_bad_checksum(validate_checksums):
-    buff = io.BytesIO(
+def empty_header(checksum):
+    """Return a header with all fields other than checksum set to zero."""
+
+    return io.BytesIO(
         constants.BLOCK_MAGIC
         + b"\x000"  # header size = 2
         + b"\0\0\0\0"  # flags = 4
         + b"\0\0\0\0"  # compression = 4
         + b"\0\0\0\0\0\0\0\0"  # allocated size = 8
         + b"\0\0\0\0\0\0\0\0"  # used size = 8
         + b"\0\0\0\0\0\0\0\0"  # data size = 8
-        + b"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"  # invalid checksum = 16
+        + checksum  # checksum = 16
     )
 
+
+@pytest.mark.parametrize("validate_checksums", [True, False])
+def test_missing_checksum(validate_checksums):
+    buff = empty_header(b"\0" * 16)
+    with generic_io.get_file(buff, mode="r") as fd:
+        # All-zero checksum should always be allowed
+        read_blocks(fd, lazy_load=False, validate_checksums=validate_checksums)[0].data
+
+
+@pytest.mark.parametrize("validate_checksums", [True, False])
+def test_bad_checksum(validate_checksums):
+    buff = empty_header(b"\1" + b"\0" * 15)
     with generic_io.get_file(buff, mode="r") as fd:
+        block = read_blocks(fd, lazy_load=False, validate_checksums=validate_checksums)[0]
         if validate_checksums:
+            # Invalid checksum should raise an exception if `validate_checksums=True`
             with pytest.raises(ValueError, match=r".* does not match given checksum"):
-                read_blocks(fd, lazy_load=False, validate_checksums=validate_checksums)[0].data
+                _ = block.data
         else:
-            read_blocks(fd, lazy_load=False, validate_checksums=validate_checksums)[0].data
+            _ = block.data

changes/2024.bugfix.rst

@@ -0,0 +1 @@
+Fixed bug in which empty/all-zero block checksums were treated as invalid when ``validate_headers`` is enabled.