fix(format): a VL element at the undefined heap address is an error

libhdf5 fails to read a VL element whose global heap address is
undefined (all 0xff), even at length 0 ("addr undefined"); we returned
"" (or an empty sequence) in every reader. Checked with h5py first:
libhdf5 writes a null element with address 0, which still reads as
empty, and h5py writes "" as a zero-size heap object at a real address,
so no file they write relies on the old behaviour. read_vl_bytes now
treats address 0 as null whatever the length, as VlResolver does.

Tests, each failing before: vl_data unit test (8- and 4-byte offsets,
lengths 0 and 1); clawhdf5 vl_data_interop
a_vl_element_at_the_undefined_heap_address_fails_like_h5py (also checks
where h5py writes ""); h5rs dump --json and check --data on the patched
`undef` dataset; clawhdf5-wasm vl_strings.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

crates/clawhdf5-format/src/vl_data.rs

@@ -239,9 +239,6 @@ impl<'a> VlResolver<'a> {
         if addr == 0 {
             return Ok(None);
         }
-        if vl.length == 0 && is_undefined_address(addr, self.offset_size) {
-            return Ok(Some(&[]));
-        }
         let data = self.object(vl)?;
         let expected = (vl.length as usize)
             .checked_mul(base_size)
@@ -372,10 +369,8 @@ pub fn read_vl_bytes(
     let mut result = Vec::with_capacity(refs.len());
 
     for vl in &refs {
-        if vl.length == 0
-            && (is_undefined_address(vl.collection_address, offset_size)
-                || vl.collection_address == 0)
-        {
+        // A heap address of 0 is a null element, as in VlResolver.
+        if vl.collection_address == 0 {
             result.push(Vec::new());
             continue;
         }
@@ -394,6 +389,15 @@ impl<'a> VlResolver<'a> {
     /// parsed on first use.
     fn object(&mut self, vl: &VlElement) -> Result<&'a [u8], FormatError> {
         let addr = vl.collection_address;
+        // libhdf5 writes a null element with address 0, never the undefined
+        // address, and fails to read one ("addr undefined") even when its
+        // length is 0; we returned an empty value.
+        if is_undefined_address(addr, self.offset_size) {
+            return Err(FormatError::VlDataError(format!(
+                "variable-length element (length {}) has the undefined global heap address",
+                vl.length
+            )));
+        }
         if !self.cache.contains_key(&addr) {
             let offset = usize::try_from(addr).map_err(|_| FormatError::UnexpectedEof {
                 expected: usize::MAX,
@@ -546,16 +550,27 @@ mod tests {
     }
 
     #[test]
-    fn null_vl_element_empty_string() {
-        // length=0, address=undefined
-        let mut raw = Vec::new();
-        raw.extend_from_slice(&0u32.to_le_bytes()); // length=0
-        raw.extend_from_slice(&u64::MAX.to_le_bytes()); // undefined address
-        raw.extend_from_slice(&0u32.to_le_bytes()); // index
-
-        let file_data = vec![0u8; 16];
-        let strings = read_vl_strings(&file_data, &raw, 1, 8, 8).unwrap();
-        assert_eq!(strings, vec![""]);
+    fn an_undefined_heap_address_is_an_error_even_at_length_0() {
+        // libhdf5 fails the read ("addr undefined"); h5py and libhdf5 write
+        // a null element with address 0. We returned "".
+        let mut file_data = vec![0u8; 256];
+        build_gcol_at(&mut file_data, 64, &[(1, b"x")]);
+        for (os, undef) in [(8u8, u64::MAX), (4, 0xFFFF_FFFF)] {
+            for length in [0, 1] {
+                let mut raw = element(1, 64, 1, os);
+                raw.extend(element(length, undef, 1, os));
+                let mut r = VlResolver::new(&file_data, os, 8);
+                let e = r.string_bytes(&raw).unwrap_err().to_string();
+                assert!(e.contains("undefined"), "{e}");
+                assert!(r.sequences(&raw, 1).is_err());
+                assert!(r.string_element(&raw[raw.len() / 2..]).is_err());
+                let n = 2;
+                assert!(read_vl_strings(&file_data, &raw, n, os, 8).is_err());
+                assert!(read_vl_bytes(&file_data, &raw, n, os, 8).is_err());
+                // The defined element alone still reads.
+                assert_eq!(r.strings(&raw[..raw.len() / 2]).unwrap(), ["x"]);
+            }
+        }
     }
 
     #[test]

crates/clawhdf5-tools/tests/gen_vl_files.py

@@ -6,7 +6,8 @@ For 8-byte (`vl8`) and 4-byte (`vl4`) offsets, writes OUTDIR/vl8.h5 and
 OUTDIR/vl4.h5, which libhdf5 reads in full, and OUTDIR/bad8.h5 and
 OUTDIR/bad4.h5, whose `bad` and `badseq` elements 0 have a length that
 disagrees with their global heap object (libhdf5: "Expected global heap
-object size does not match"). h5py cannot write a VL string with a NUL in
+object size does not match"), and whose `undef` element 1 has length 0 and
+the undefined heap address (libhdf5: "addr undefined"). h5py cannot write a VL string with a NUL in
 it or a null element in a contiguous dataset, so those are patched in.
 
 Prints one JSON object: for each file, each dataset's values as h5py reads
@@ -76,11 +77,17 @@ def bad(path, sizes):
         s = f.create_dataset("badseq", shape=(2,), dtype=I4)
         s[0] = [1, 2, 3]
         s[1] = [4]
+        f.create_dataset("undef", data=np.array(["x", "", "yz"], dtype=object), dtype=S)
         off, soff = f["bad"].id.get_offset(), f["badseq"].id.get_offset()
+        uoff = f["undef"].id.get_offset()
     b = bytearray(open(path, "rb").read())
     gcol = int.from_bytes(b[off + 4 : off + 4 + os_], "little")
     struct.pack_into("<I", b, off, 3)  # "cdefgh": length 6 -> 3
     struct.pack_into("<I", b, soff, 2)  # [1, 2, 3]: length 3 -> 2
+    # "": length 0 at the undefined address (all 0xff), which libhdf5 fails
+    # to read ("addr undefined"); it writes a null element as address 0.
+    es = 8 + os_
+    b[uoff + es : uoff + 2 * es] = element(0, (1 << (8 * os_)) - 1, 1, os_)
     open(path, "wb").write(bytes(b))
     return gcol
 
@@ -116,6 +123,6 @@ for tag, sizes in (("8", None), ("4", (4, 4))):
         result[f"vl{tag}"] = {n: read(f[n]) for n in ("d", "u", "seq", "sequ", "cmp")}
         result[f"vl{tag}"]["va"] = [value(s) for s in f.attrs["va"]]
     with h5py.File(x, "r") as f:
-        result[f"bad{tag}"] = {n: read(f[n]) for n in ("bad", "badseq")}
+        result[f"bad{tag}"] = {n: read(f[n]) for n in ("bad", "badseq", "undef")}
         result[f"bad{tag}"]["gcol"] = gcol
 json.dump(result, sys.stdout)

crates/clawhdf5-tools/tests/h5rs_interop.rs

@@ -828,7 +828,8 @@ fn dump_prints_vl_data_like_h5dump() {
 
 /// `dump --json` gives the values h5py reads, element by element; and an
 /// element whose heap object is not its length × base size is an error, as
-/// in h5py, not a truncated value (it printed "cde" and (1, 2)).
+/// in h5py, not a truncated value (it printed "cde" and (1, 2)); so is a
+/// length-0 element at the undefined heap address (it printed "").
 #[test]
 fn dump_json_vl_values_match_h5py() {
     let Some(f) = generate_vl() else { return };
@@ -860,7 +861,12 @@ fn dump_json_vl_values_match_h5py() {
                     let e = g["error"]
                         .as_str()
                         .unwrap_or_else(|| panic!("{bad}: {path}: {g}"));
-                    assert!(e.contains("holds"), "{bad}: {path}: {e}");
+                    let why = if path == "/undef" {
+                        "undefined"
+                    } else {
+                        "holds"
+                    };
+                    assert!(e.contains(why), "{bad}: {path}: {e}");
                 } else {
                     assert_eq!(g, w, "{bad}: {path}");
                 }
@@ -871,7 +877,8 @@ fn dump_json_vl_values_match_h5py() {
 
 /// `check --data` holds VL elements to libhdf5's rule: a heap object whose
 /// size is not exactly the element's length × base size is a problem (it
-/// only caught objects shorter than the element).
+/// only caught objects shorter than the element), and so is an element at
+/// the undefined heap address.
 #[test]
 fn check_data_flags_mis_sized_vl_heap_objects() {
     let Some(f) = generate_vl() else { return };
@@ -893,5 +900,11 @@ fn check_data_flags_mis_sized_vl_heap_objects() {
             assert!(s.contains(&want), "bad{tag}: no {want:?} in\n{s}");
             assert!(s.contains(what), "bad{tag}: {s}");
         }
+        // A length-0 element at the undefined heap address: libhdf5 fails
+        // to read it; check skipped it.
+        let undef: u64 = if tag == "8" { u64::MAX } else { 0xffff_ffff };
+        let want = format!("problem: {undef:#x} /undef: variable-length data: global heap:");
+        assert!(s.contains(&want), "bad{tag}: no {want:?} in\n{s}");
+        assert!(s.contains("undefined global heap address"), "bad{tag}: {s}");
     }
 }

crates/clawhdf5-wasm/tests/vl_strings.rs

@@ -1,8 +1,9 @@
 //! The wasm reader resolves VL strings with the library's `VlResolver`, so
 //! it returns what `File::read_string` and h5py return: a string ends at
 //! its first NUL, a null element is empty, a heap object of the wrong size
-//! is an error, and a VL datatype whose stored element size disagrees with
-//! the file's offset size is refused. Checked with 8- and 4-byte offsets.
+//! is an error, an element at the undefined heap address is an error, and a
+//! VL datatype whose stored element size disagrees with the file's offset
+//! size is refused. Checked with 8- and 4-byte offsets.
 //!
 //! Skipped when python3 with h5py is missing, unless
 //! `CLAWHDF5_REQUIRE_INTEROP=1`. `CLAWHDF5_PYTHON` names the interpreter.
@@ -35,7 +36,9 @@ fn h5py_available() -> bool {
 /// For each offset size: `vl{8,4}.h5` with dataset `d` = "a\0b", "", null,
 /// "zz" (patched: h5py writes neither a NUL nor a null element);
 /// `bad{8,4}.h5` whose element 0 claims 3 bytes of a 6-byte heap object;
-/// and `size{8,4}.h5` whose VL datatype message stores a 24-byte element.
+/// `size{8,4}.h5` whose VL datatype message stores a 24-byte element; and
+/// `undef{8,4}.h5` whose element 1 has length 0 and the undefined heap
+/// address.
 /// Prints h5py's reading of each element as hex, or "error".
 const SCRIPT: &str = r#"
 import struct, sys, h5py, numpy as np
@@ -77,7 +80,12 @@ for os_ in (8, 4):
     i = b.index(pat)
     struct.pack_into('<I', b, i + 4, 24)
     open(p, 'wb').write(bytes(b))
-    for name in ('vl', 'bad', 'size'):
+    p = '%s/undef%d.h5' % (out, os_)
+    off = make(p, os_, ['x', '', 'yz'])
+    b = bytearray(open(p, 'rb').read())
+    b[off + es:off + 2 * es] = elem(0, (1 << (8 * os_)) - 1, 1, os_)
+    open(p, 'wb').write(bytes(b))
+    for name in ('vl', 'bad', 'size', 'undef'):
         with h5py.File('%s/%s%d.h5' % (out, name, os_), 'r') as f:
             got = []
             for i in range(f['d'].shape[0]):
@@ -146,5 +154,16 @@ fn vl_strings_read_like_file_and_h5py() {
         let e = wasm.unwrap_err();
         assert!(e.contains("stores 24-byte elements"), "size{os}: {e}");
         assert!(file.is_err(), "size{os}");
+
+        // Length 0 at the undefined heap address: libhdf5 fails the read
+        // ("addr undefined"); both readers returned "".
+        assert_eq!(h5py[&format!("undef{os}")], "78,error,797a");
+        let (wasm, file) = read(&format!("undef{os}"));
+        let e = wasm.unwrap_err();
+        assert!(
+            e.contains("undefined global heap address"),
+            "undef{os}: {e}"
+        );
+        assert!(file.is_err(), "undef{os}");
     }
 }

crates/clawhdf5/tests/vl_data_interop.rs

@@ -442,3 +442,53 @@ with h5py.File(path, 'r') as f:
     assert!(one(0).is_err());
     assert_eq!(one(1).unwrap(), vec![vec![4]]);
 }
+
+#[test]
+fn a_vl_element_at_the_undefined_heap_address_fails_like_h5py() {
+    // libhdf5 writes a null element with heap address 0 (h5py reads it as
+    // b''), and an empty string as a real zero-size heap object; neither
+    // uses the undefined address. An element of length 0 at the undefined
+    // address fails in libhdf5 ("addr undefined"); we returned "".
+    skip_if_no_python!();
+    let dir = tempfile::tempdir().unwrap();
+    let path = dir.path().join("undef.h5");
+    let script = format!(
+        r#"
+import struct, h5py, numpy as np
+path = {path:?}
+with h5py.File(path, 'w') as f:
+    f.create_dataset('d', data=np.array(['x', '', 'yz', ''], dtype=object), dtype=h5py.string_dtype())
+    off = f['d'].id.get_offset()
+b = bytearray(open(path, 'rb').read())
+# h5py's '' (element 3): length 0 at a real heap address, not 0 or all 0xff.
+length, addr, _ = struct.unpack_from('<IQI', b, off + 48)
+print('empty\t%d %d' % (length, addr not in (0, 2**64 - 1)))
+struct.pack_into('<IQI', b, off + 16, 0, 2**64 - 1, 1)
+open(path, 'wb').write(bytes(b))
+with h5py.File(path, 'r') as f:
+    for i in range(4):
+        try:
+            print('d%d\t%s' % (i, f['d'][i].hex()))
+        except OSError as e:
+            print('d%d\terror %s' % (i, 'addr undefined' in str(e)))
+"#,
+        path = path.display().to_string()
+    );
+    let expected = run_python(&script);
+    assert_eq!(expected["empty"], "0 1", "h5py writes '' at a real address");
+    assert_eq!(expected["d0"], "78");
+    assert_eq!(expected["d1"], "error True");
+    assert_eq!(expected["d2"], "797a");
+    assert_eq!(expected["d3"], "");
+
+    let file = File::open(&path).unwrap();
+    let d = file.dataset("d").unwrap();
+    let one = |i: u64| d.read_string_selection(&Selection::slice(&[i..i + 1]));
+    assert_eq!(one(0).unwrap(), vec!["x"]);
+    let e = one(1).unwrap_err().to_string();
+    assert!(e.contains("undefined global heap address"), "{e}");
+    assert_eq!(one(2).unwrap(), vec!["yz"]);
+    assert_eq!(one(3).unwrap(), vec![""]);
+    assert!(d.read_string().is_err());
+    assert!(d.read_string_bytes().is_err());
+}