From 10da8f0d09833189cc11ff34384af21d1f3b9e7d Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 08:18:57 -0500 Subject: [PATCH] fix(format): read VL values in files with 4-byte offsets In a file with sizeof_addr = 4, a VL string attribute came back as AttrValue::Raw, a compound's VL member failed with GlobalHeapObjectNotFound and VL datasets failed with a size mismatch. Two bugs: Datatype::type_size() said 16 for every VL type, while the element is 4 + offset size + 4 bytes (12 here); and the global heap was parsed without the padding libhdf5 puts after its collection and object headers (both round up to 8), so with 4-byte lengths every object was looked up 4 bytes early. Datatype::VariableLength now carries the size its datatype message stores, and writes it back. Checked against h5py in tests/vl_offset4_interop.rs (fails with either fix reverted). Conformance unchanged at 575 of 697; in cve-2024-32608 a VL attribute whose datatype claims 524304-byte elements is now an error (h5py cannot iterate those attributes at all). Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 15 +++ crates/clawhdf5-format/src/datatype.rs | 28 ++++- crates/clawhdf5-format/src/global_heap.rs | 19 ++- crates/clawhdf5/tests/vl_offset4_interop.rs | 126 ++++++++++++++++++++ docs/known-issues.md | 5 +- 5 files changed, 184 insertions(+), 9 deletions(-) create mode 100644 crates/clawhdf5/tests/vl_offset4_interop.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index ef4bd2f..3af6328 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ ## Unreleased +### Variable-length data (2026-09-26) +- **VL values in files with 4-byte offsets** (`sizeof_addr = 4`). A VL + string attribute came back as `AttrValue::Raw`, a VL member of a compound + failed with `GlobalHeapObjectNotFound`, and VL datasets failed with a + size mismatch. Two causes: `Datatype::type_size()` reported 16 for every + VL type (the element is 4 + offset size + 4 bytes: 12 here), and the + global heap was parsed without the padding libhdf5 puts after its + collection and object headers (`H5HG_SIZEOF_HDR`/`H5HG_SIZEOF_OBJHDR` + round up to 8), so with 4-byte lengths every object was looked up 4 + bytes early. `Datatype::VariableLength` now carries the element `size` + stored in the datatype message (**breaking** for code that builds or + exhaustively destructures that variant; patterns with `..` are + unaffected), and it is written back as stored. Tested against h5py + (`crates/clawhdf5/tests/vl_offset4_interop.rs`). + ### 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/datatype.rs b/crates/clawhdf5-format/src/datatype.rs index 12e427f..974c249 100644 --- a/crates/clawhdf5-format/src/datatype.rs +++ b/crates/clawhdf5-format/src/datatype.rs @@ -125,6 +125,11 @@ pub enum Datatype { }, /// Class 9: Variable-length type. VariableLength { + /// Size of one element as stored in the file: a sequence length (4 + /// bytes), a global heap collection address (the file's + /// `offset_size`) and an object index (4 bytes) — 16 in a file with + /// 8-byte offsets, 12 with 4-byte offsets. + size: u32, is_string: bool, padding: Option, charset: Option, @@ -771,6 +776,7 @@ impl Datatype { pos += consumed; Ok(( Datatype::VariableLength { + size, is_string, padding, charset, @@ -1017,6 +1023,7 @@ impl Datatype { Self::build_header(3, 1, [bf0, 0, 0], *size) } Datatype::VariableLength { + size, is_string, padding, charset, @@ -1039,7 +1046,7 @@ impl Datatype { } else { 0 }; - let mut buf = Self::build_header(9, 1, [bf0, bf1, 0], 16); + let mut buf = Self::build_header(9, 1, [bf0, bf1, 0], *size); buf.extend_from_slice(&base_type.serialize()); buf } @@ -1208,7 +1215,7 @@ impl Datatype { Datatype::Compound { size, .. } => *size, Datatype::Reference { size, .. } => *size, Datatype::Enumeration { size, .. } => *size, - Datatype::VariableLength { .. } => 16, // typically pointer + length + Datatype::VariableLength { size, .. } => *size, Datatype::Array { base_type, dimensions, @@ -1889,11 +1896,13 @@ mod tests { let (dt, _) = Datatype::parse(&buf).unwrap(); match dt { Datatype::VariableLength { + size, is_string, padding, charset, base_type, } => { + assert_eq!(size, 16); assert!(is_string); assert_eq!(padding, Some(StringPadding::NullTerminate)); assert_eq!(charset, Some(CharacterSet::Utf8)); @@ -1914,11 +1923,13 @@ mod tests { let (dt, _) = Datatype::parse(&buf).unwrap(); match dt { Datatype::VariableLength { + size, is_string, padding, charset, base_type, } => { + assert_eq!(size, 16); assert!(!is_string); assert_eq!(padding, None); assert_eq!(charset, None); @@ -1928,6 +1939,19 @@ mod tests { } } + #[test] + fn variable_length_size_is_the_stored_size() { + // A file with 4-byte offsets stores 12-byte VL elements (length 4 + + // address 4 + index 4); the type used to report 16 regardless, so + // every read laid the elements out 16 bytes apart. + let mut buf = build_dt_header(9, 1, [0x01, 0x00, 0], 12); + buf.extend_from_slice(&build_fixed_point(1, false, false, 0, 8)); + let (dt, _) = Datatype::parse(&buf).unwrap(); + assert_eq!(dt.type_size(), 12); + // And it is written back as stored. + assert_eq!(dt.serialize()[4..8], 12u32.to_le_bytes()); + } + #[test] fn test_array_2d() { // Array [3][4] of i32 LE, version 3 diff --git a/crates/clawhdf5-format/src/global_heap.rs b/crates/clawhdf5-format/src/global_heap.rs index dfba3f0..5eab713 100644 --- a/crates/clawhdf5-format/src/global_heap.rs +++ b/crates/clawhdf5-format/src/global_heap.rs @@ -64,8 +64,11 @@ impl GlobalHeapCollection { offset: usize, length_size: u8, ) -> Result { - // signature(4) + version(1) + reserved(3) + collection_size(length_size) - let header_size = 8 + length_size as usize; + // signature(4) + version(1) + reserved(3) + collection_size(length_size), + // padded to a multiple of 8 as libhdf5 lays it out (`H5HG_SIZEOF_HDR`). + // With 8-byte lengths the padding is 0; with 4-byte lengths it is 4, + // and reading without it put every object 4 bytes early. + let header_size = pad8(8 + length_size as usize); ensure_len(file_data, offset, header_size)?; if file_data[offset..offset + 4] != GCOL_SIGNATURE { @@ -104,8 +107,9 @@ impl GlobalHeapCollection { break; } - // object_index(2) + reference_count(2) + reserved(4) + object_size(length_size) - let obj_header_size = 8 + length_size as usize; + // object_index(2) + reference_count(2) + reserved(4) + + // object_size(length_size), padded to 8 (`H5HG_SIZEOF_OBJHDR`). + let obj_header_size = pad8(8 + length_size as usize); ensure_len(file_data, pos, obj_header_size)?; let reference_count = u16::from_le_bytes([file_data[pos + 2], file_data[pos + 3]]); @@ -149,10 +153,11 @@ mod tests { let ls = length_size as usize; // Calculate total size - let header_size = 8 + ls; + // libhdf5 pads both headers to a multiple of 8. + let header_size = pad8(8 + ls); let mut obj_size_total = 0usize; for (_, _, data) in objects { - let obj_header = 8 + ls; + let obj_header = pad8(8 + ls); obj_size_total += obj_header + pad8(data.len()); } // Free space marker (2 bytes for index 0) @@ -170,6 +175,7 @@ mod tests { 8 => buf.extend_from_slice(&(collection_size as u64).to_le_bytes()), _ => panic!("unsupported length_size"), } + buf.resize(header_size, 0); // Objects for (index, ref_count, data) in objects { @@ -181,6 +187,7 @@ mod tests { 8 => buf.extend_from_slice(&(data.len() as u64).to_le_bytes()), _ => panic!("unsupported"), } + buf.resize(buf.len() + (pad8(8 + ls) - (8 + ls)), 0); buf.extend_from_slice(data); // Pad to 8 bytes let padded = pad8(data.len()); diff --git a/crates/clawhdf5/tests/vl_offset4_interop.rs b/crates/clawhdf5/tests/vl_offset4_interop.rs new file mode 100644 index 0000000..cc7ce08 --- /dev/null +++ b/crates/clawhdf5/tests/vl_offset4_interop.rs @@ -0,0 +1,126 @@ +//! Variable-length values in files with 4-byte offsets and lengths +//! (`sizeof_addr = 4`), checked against h5py/libhdf5 through the +//! `clawhdf5_format` decoders. +//! +//! These failed with `GlobalHeapObjectNotFound` or came back as +//! `AttrValue::Raw`: the VL datatype claimed 16-byte elements whatever the +//! file's offset size, and the global heap was read without the padding +//! libhdf5 puts after its collection and object headers. Skipped when +//! python3 with h5py is unavailable, unless `CLAWHDF5_REQUIRE_INTEROP=1`. + +use std::process::Command; + +use clawhdf5::{AttrValue, File, Selection}; +use clawhdf5_format::data_read::{read_as_i64, read_compound_fields}; +use clawhdf5_format::vl_data::{read_vl_bytes, read_vl_strings}; + +fn python() -> String { + std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string()) +} + +fn interop_required() -> bool { + std::env::var("CLAWHDF5_REQUIRE_INTEROP").is_ok_and(|v| v == "1") +} + +fn python_available() -> bool { + Command::new(python()) + .args(["-c", "import h5py, numpy"]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +#[test] +fn vl_values_in_a_file_with_4_byte_offsets_read_like_h5py() { + if !python_available() { + assert!( + !interop_required(), + "CLAWHDF5_REQUIRE_INTEROP=1 but python3 with h5py is not available" + ); + eprintln!("SKIP: python3 with h5py not available"); + return; + } + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("offset4.h5"); + let script = format!( + r#" +import h5py, numpy as np +S = h5py.string_dtype() +fcpl = h5py.h5p.create(h5py.h5p.FILE_CREATE); fcpl.set_sizes(4, 4) +with h5py.File(h5py.h5f.create({path:?}.encode(), h5py.h5f.ACC_TRUNC, fcpl=fcpl)) as f: + f.attrs['vlattr'] = 'attr-value' + f.attrs.create('vlattr_arr', np.array(['p', 'qq', ''], dtype=object), dtype=S) + ct = np.dtype([('id', ' = stdout.lines().collect(); + let (vlattr, vlattr_arr, names, seqs) = (lines[0], lines[1], lines[2], lines[3]); + + let file = File::open(&path).unwrap(); + let sb = file.superblock(); + assert_eq!((sb.offset_size, sb.length_size), (4, 4)); + + let attrs = file.root().attrs().unwrap(); + match &attrs["vlattr"] { + AttrValue::String(s) => assert_eq!(s, vlattr), + other => panic!("vlattr: {other:?}"), + } + match &attrs["vlattr_arr"] { + AttrValue::StringArray(s) => assert_eq!(s.join(","), vlattr_arr), + other => panic!("vlattr_arr: {other:?}"), + } + + // The compound's VL string member. + let ds = file.dataset("compound").unwrap(); + let dt = ds.raw_datatype().unwrap(); + assert_eq!(dt.type_size(), 24, "4 + 12-byte VL element + 8"); + let raw = ds.read_selection(&Selection::All).unwrap(); + let fields = read_compound_fields(&raw, &dt).unwrap(); + let name = fields.iter().find(|f| f.name == "name").unwrap(); + assert_eq!(name.datatype.type_size(), 12); + let got = read_vl_strings(file.as_bytes(), &name.raw_data, 3, 4, 4).unwrap(); + assert_eq!(got.join(","), names); + let id = fields.iter().find(|f| f.name == "id").unwrap(); + assert_eq!( + read_as_i64(&id.raw_data, &id.datatype).unwrap(), + vec![1, 2, 3] + ); + + // A VL sequence attribute. + let AttrValue::Raw { datatype, data, .. } = &attrs["vlen_attr"] else { + panic!("vlen_attr is Raw"); + }; + let clawhdf5_format::datatype::Datatype::VariableLength { base_type, .. } = datatype else { + panic!("vlen_attr is VL"); + }; + let got: Vec = read_vl_bytes(file.as_bytes(), data, 2, 4, 4) + .unwrap() + .iter() + .map(|b| { + let v = read_as_i64(b, base_type).unwrap(); + v.iter().map(i64::to_string).collect::>().join(" ") + }) + .collect(); + assert_eq!(got.join(";"), seqs); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index bf8f77a..b9a19b6 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -146,7 +146,10 @@ fill-value item that did is fixed). `GlobalHeapObjectNotFound` or come back as `Raw`: these paths assume the 16-byte element of an 8-byte-offset file. The datatype itself reads (it was refused as "member overlaps with previous member" until - 2026-09-26). + 2026-09-26). **Fixed 2026-09-26:** a VL type's element size is the one + its datatype message stores (12 with 4-byte offsets), and the global + heap is read with libhdf5's header padding + (`crates/clawhdf5/tests/vl_offset4_interop.rs`). - Metadata cache images are not supported. - x87 long double and binary128 are refused. - N-Bit on 64-bit scale-offset data and some N-Bit parameter layouts fail.