From 5a202f3791c3365af73f1ee6b8d56ae9dd86866d Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 09:06:46 -0500 Subject: [PATCH] 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) --- CHANGELOG.md | 10 +++++ crates/clawhdf5-format/src/vl_data.rs | 49 +++++++++++++------- crates/clawhdf5-tools/tests/gen_vl_files.py | 11 ++++- crates/clawhdf5-tools/tests/h5rs_interop.rs | 19 ++++++-- crates/clawhdf5-wasm/tests/vl_strings.rs | 27 +++++++++-- crates/clawhdf5/tests/vl_data_interop.rs | 50 +++++++++++++++++++++ docs/known-issues.md | 4 +- 7 files changed, 143 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc22849..e656dff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,16 @@ `crates/clawhdf5-wasm/tests/vl_strings.rs`). New `VlResolver::element` / `string_element` resolve one element in place. +- **A VL element at the undefined heap address is an error**, as in + libhdf5 ("addr undefined"). One of length 0 read as `""` in every reader + (`File`, `h5rs`, `clawhdf5-wasm`, `read_vl_strings`, `read_vl_bytes`). + libhdf5 writes a null element with heap address 0, which still reads as + empty, and h5py writes `""` as a zero-size heap object at a real address, + so no file libhdf5 or h5py writes is affected + (`a_vl_element_at_the_undefined_heap_address_fails_like_h5py` in + `crates/clawhdf5/tests/vl_data_interop.rs`). `read_vl_bytes` now also + treats address 0 as null whatever the length, as `VlResolver` does. + ### Plugin filters (2026-09-26) - **LZF, bitshuffle, bzip2 and Blosc read and write, in pure Rust.** Files written by h5py with `compression="lzf"`, or with hdf5plugin's diff --git a/crates/clawhdf5-format/src/vl_data.rs b/crates/clawhdf5-format/src/vl_data.rs index f183ef0..3a54c1a 100644 --- a/crates/clawhdf5-format/src/vl_data.rs +++ b/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] diff --git a/crates/clawhdf5-tools/tests/gen_vl_files.py b/crates/clawhdf5-tools/tests/gen_vl_files.py index 3937c19..cb50f2d 100644 --- a/crates/clawhdf5-tools/tests/gen_vl_files.py +++ b/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(" 3 struct.pack_into(" 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) diff --git a/crates/clawhdf5-tools/tests/h5rs_interop.rs b/crates/clawhdf5-tools/tests/h5rs_interop.rs index a1b4492..202606f 100644 --- a/crates/clawhdf5-tools/tests/h5rs_interop.rs +++ b/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}"); } } diff --git a/crates/clawhdf5-wasm/tests/vl_strings.rs b/crates/clawhdf5-wasm/tests/vl_strings.rs index 1baddd0..096cc16 100644 --- a/crates/clawhdf5-wasm/tests/vl_strings.rs +++ b/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('()`, and VL values inside compounds or `AttrValue::Raw` attributes decode with `File::decode_strings` / `File::decode_vlen` (`crates/clawhdf5/tests/vl_data_interop.rs`).