From 85eb7f5ce2e0c237f07697af3b209f691782e9e2 Mon Sep 17 00:00:00 2001 From: osobh Date: Fri, 25 Sep 2026 21:52:19 -0500 Subject: [PATCH 1/3] feat(format): read Data Layout message versions 1 and 2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HDF5 1.4/1.6-era files store the layout as version 1 or 2: version, dimensionality, class, 5 reserved bytes, an address (contiguous and chunked only), dimensionality 32-bit sizes (with the trailing element-size dimension) and, for compact storage, a 32-bit size and the raw data. They failed with InvalidLayoutVersion — 84 of the 686 files in the audit sweep, 205 datasets. Map them onto the existing variants: chunked uses the same version-1 B-tree chunk index as version 3 and is reported as version 3, so every chunked read path (filters, selections, caches) applies unchanged. Contiguous size is the product of the stored dimensions, which is what libhdf5 computes from the dataspace; a disagreement fails the reader's size check instead of returning wrong data. Fixtures are HDF5's own deflate.h5 (v1, chunked + deflate) and h5ex_g_iterate.h5 (v2, contiguous, one unallocated dataset); the new interop test compares every dataset byte for byte against h5py. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 6 + crates/clawhdf5-format/src/data_layout.rs | 190 +++++++++++++++++- .../tests/fixtures/legacy/README.md | 11 + .../tests/fixtures/legacy/deflate.h5 | Bin 0 -> 6240 bytes .../tests/fixtures/legacy/h5ex_g_iterate.h5 | Bin 0 -> 2928 bytes .../clawhdf5/tests/legacy_format_interop.rs | 121 +++++++++++ docs/known-issues.md | 2 + 7 files changed, 328 insertions(+), 2 deletions(-) create mode 100644 crates/clawhdf5-format/tests/fixtures/legacy/README.md create mode 100644 crates/clawhdf5-format/tests/fixtures/legacy/deflate.h5 create mode 100644 crates/clawhdf5-format/tests/fixtures/legacy/h5ex_g_iterate.h5 create mode 100644 crates/clawhdf5/tests/legacy_format_interop.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b87415..cf1b36d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -236,6 +236,12 @@ - A pipeline with Fletcher32 ahead of the compressor (h5py `set_fletcher32()` then `set_deflate()`) no longer fails with "deflate: output exceeds size limit". +- `clawhdf5-format`: **HDF5 1.4/1.6-era files are readable.** Data Layout + message versions 1 and 2 (compact, contiguous, and chunked through the + version-1 B-tree) failed with `InvalidLayoutVersion` — 84 of the 686 files in + the 2026-09-25 audit sweep, 205 datasets. They now read as libhdf5 does; + checked byte for byte against h5py on HDF5's own test files + (`tests/legacy_format_interop.rs`). ### Storage - `clawhdf5-format`: **half-precision datasets.** diff --git a/crates/clawhdf5-format/src/data_layout.rs b/crates/clawhdf5-format/src/data_layout.rs index 8429c5d..ece868c 100644 --- a/crates/clawhdf5-format/src/data_layout.rs +++ b/crates/clawhdf5-format/src/data_layout.rs @@ -1,7 +1,7 @@ //! HDF5 Data Layout message parsing (message type 0x0008). #[cfg(not(feature = "std"))] -use alloc::{string::String, vec::Vec}; +use alloc::{format, string::String, vec::Vec}; #[cfg(feature = "std")] use std::string::String; @@ -45,7 +45,9 @@ pub enum DataLayout { chunk_dimensions: Vec, /// B-tree address, or `None` if undefined. btree_address: Option, - /// Layout version (3 or 4). + /// Layout version (3 or 4). Version 1/2 messages (HDF5 1.4/1.6-era) + /// use the same version-1 B-tree chunk index as version 3 and are + /// reported as 3. version: u8, /// Chunk index type (v4 only). chunk_index_type: Option, @@ -261,6 +263,7 @@ impl DataLayout { let layout_class = data[1]; match version { + 1 | 2 => Self::parse_v1_v2(data, offset_size), 3 => Self::parse_v3(data, layout_class, offset_size, length_size), // v5 (emitted by HDF5 1.14+/2.0 with `libver=latest`) uses the same // message structure as v4 — only the version number was bumped. @@ -269,6 +272,87 @@ impl DataLayout { } } + /// Layout message versions 1 and 2 (HDF5 before 1.6.3): + /// + /// ```text + /// version(1) · dimensionality(1) · layout class(1) · reserved(5) + /// · address(offset_size) — contiguous and chunked only + /// · dimension sizes(4 × dimensionality) + /// · compact data size(4) · compact raw data — compact only + /// ``` + /// + /// The dimension sizes are the dataset's (contiguous/compact) or the + /// chunk's (chunked) extent plus a trailing element-size dimension, as in + /// version 3's chunked form. libhdf5 ignores them for contiguous storage + /// and sizes the data from the dataspace; the product of the stored + /// dimensions is that same size, and a disagreement (a dimension that was + /// truncated to 32 bits) is caught by the reader's size check rather than + /// returning wrong data. + fn parse_v1_v2(data: &[u8], offset_size: u8) -> Result { + ensure_len(data, 0, 8)?; + let dimensionality = data[1] as usize; + let layout_class = data[2]; + // H5O_LAYOUT_NDIMS: 32 dataspace dimensions + the element-size one. + if dimensionality > 33 { + return Err(FormatError::Overflow(format!( + "data layout dimensionality {dimensionality} exceeds 33" + ))); + } + let mut p = 8; + let os = offset_size as usize; + let address = match layout_class { + 1 | 2 => { + ensure_len(data, p, os)?; + let a = if is_undefined(data, p, offset_size) { + None + } else { + Some(read_offset(data, p, offset_size)?) + }; + p += os; + a + } + 0 => None, + _ => return Err(FormatError::InvalidLayoutClass(layout_class)), + }; + ensure_len(data, p, dimensionality * 4)?; + let dims: Vec = data[p..p + dimensionality * 4] + .as_chunks::<4>() + .0 + .iter() + .map(|c| u32::from_le_bytes(*c)) + .collect(); + p += dimensionality * 4; + match layout_class { + 0 => { + ensure_len(data, p, 4)?; + let size = + u32::from_le_bytes([data[p], data[p + 1], data[p + 2], data[p + 3]]) as usize; + ensure_len(data, p + 4, size)?; + Ok(DataLayout::Compact { + data: data[p + 4..p + 4 + size].to_vec(), + }) + } + 1 => { + let size = dims + .iter() + .try_fold(1u64, |acc, &d| acc.checked_mul(d as u64)) + .ok_or_else(|| { + FormatError::Overflow(format!("contiguous layout size {dims:?}")) + })?; + Ok(DataLayout::Contiguous { address, size }) + } + _ => Ok(DataLayout::Chunked { + chunk_dimensions: dims, + btree_address: address, + version: 3, + chunk_index_type: None, + single_chunk_filtered_size: None, + single_chunk_filter_mask: None, + dont_filter_partial_edge_chunks: false, + }), + } + } + fn parse_v3( data: &[u8], layout_class: u8, @@ -546,6 +630,108 @@ impl DataLayout { mod tests { use super::*; + /// Version 1/2 header: version, dimensionality, class, reserved(5). + fn v1v2_header(version: u8, ndims: u8, class: u8) -> Vec { + vec![version, ndims, class, 0, 0, 0, 0, 0] + } + + #[test] + fn v2_compact() { + let mut buf = v1v2_header(2, 2, 0); + // dims (3 elements of 2 bytes) — no address for compact + buf.extend_from_slice(&3u32.to_le_bytes()); + buf.extend_from_slice(&2u32.to_le_bytes()); + buf.extend_from_slice(&6u32.to_le_bytes()); // compact size (u32 in v1/v2) + buf.extend_from_slice(&[1, 0, 2, 0, 3, 0]); + assert_eq!( + DataLayout::parse(&buf, 8, 8).unwrap(), + DataLayout::Compact { + data: vec![1, 0, 2, 0, 3, 0] + } + ); + } + + #[test] + fn v1_contiguous_size_from_dimensions() { + let mut buf = v1v2_header(1, 3, 1); + buf.extend_from_slice(&0x800u32.to_le_bytes()); // 4-byte address + for d in [10u32, 20, 4] { + buf.extend_from_slice(&d.to_le_bytes()); + } + assert_eq!( + DataLayout::parse(&buf, 4, 4).unwrap(), + DataLayout::Contiguous { + address: Some(0x800), + size: 800, + } + ); + } + + #[test] + fn v1_contiguous_undefined_address() { + let mut buf = v1v2_header(1, 2, 1); + buf.extend_from_slice(&[0xFF; 8]); + buf.extend_from_slice(&5u32.to_le_bytes()); + buf.extend_from_slice(&8u32.to_le_bytes()); + assert_eq!( + DataLayout::parse(&buf, 8, 8).unwrap(), + DataLayout::Contiguous { + address: None, + size: 40, + } + ); + } + + #[test] + fn v1_chunked_maps_to_btree_v1_index() { + let mut buf = v1v2_header(1, 3, 2); + buf.extend_from_slice(&0x1234u64.to_le_bytes()); + for d in [50u32, 50, 4] { + buf.extend_from_slice(&d.to_le_bytes()); + } + assert_eq!( + DataLayout::parse(&buf, 8, 8).unwrap(), + DataLayout::Chunked { + chunk_dimensions: vec![50, 50, 4], + btree_address: Some(0x1234), + version: 3, + chunk_index_type: None, + single_chunk_filtered_size: None, + single_chunk_filter_mask: None, + dont_filter_partial_edge_chunks: false, + } + ); + } + + #[test] + fn v1v2_rejects_bad_class_dimensionality_and_truncation() { + assert_eq!( + DataLayout::parse(&v1v2_header(1, 1, 3), 8, 8).unwrap_err(), + FormatError::InvalidLayoutClass(3) + ); + assert!(matches!( + DataLayout::parse(&v1v2_header(2, 34, 1), 8, 8).unwrap_err(), + FormatError::Overflow(_) + )); + // Chunked, dims cut short. + let mut buf = v1v2_header(1, 2, 2); + buf.extend_from_slice(&0x10u64.to_le_bytes()); + buf.extend_from_slice(&7u32.to_le_bytes()); + assert!(matches!( + DataLayout::parse(&buf, 8, 8).unwrap_err(), + FormatError::UnexpectedEof { .. } + )); + // Compact, raw data shorter than its declared size. + let mut buf = v1v2_header(2, 1, 0); + buf.extend_from_slice(&4u32.to_le_bytes()); + buf.extend_from_slice(&100u32.to_le_bytes()); + buf.extend_from_slice(&[0; 4]); + assert!(matches!( + DataLayout::parse(&buf, 8, 8).unwrap_err(), + FormatError::UnexpectedEof { .. } + )); + } + #[test] fn v3_compact() { let mut buf = vec![3u8, 0]; // version=3, class=0 (compact) diff --git a/crates/clawhdf5-format/tests/fixtures/legacy/README.md b/crates/clawhdf5-format/tests/fixtures/legacy/README.md new file mode 100644 index 0000000..b7ee62d --- /dev/null +++ b/crates/clawhdf5-format/tests/fixtures/legacy/README.md @@ -0,0 +1,11 @@ +# Legacy (HDF5 1.4/1.6-era) fixtures + +Unmodified copies of the HDF Group's own test files from +https://github.com/HDFGroup/hdf5 at a3cf1ea82cc7a66e50029a688121e1b105a7ce88 +(BSD-style license, see that repository's `LICENSE`). Current libraries cannot +write these structures, so they are kept as files. + +| File | Upstream path | Exercises | +|---|---|---| +| `deflate.h5` | `test/testfiles/deflate.h5` | Data Layout message v1, chunked + deflate (v1 B-tree index) | +| `h5ex_g_iterate.h5` | `HDF5Examples/C/H5G/h5ex_g_iterate.h5` | Data Layout message v2, contiguous; an unallocated dataset | diff --git a/crates/clawhdf5-format/tests/fixtures/legacy/deflate.h5 b/crates/clawhdf5-format/tests/fixtures/legacy/deflate.h5 new file mode 100644 index 0000000000000000000000000000000000000000..2f62e2599eee6a832a07f6048b0daba59bd8cf08 GIT binary patch literal 6240 zcmeD5aB<`1lHy_j0S*oZ76t(ZW-tdr{D*=B2~<8z$pWZiMyNmol#u}Cd$>9VfSFKn zs4)x;PhoLXWC!F@5o#|Z*jz@2l+?7G#FA77PN+IQp!pzRWME)qXlQ6= zWNd6?3e43UF#XIB3pCgu8jL_{ff$<1fh-S*1eM5OKYtfSpvz(T=K^x!MkPB&f-#_S z3KWX4@(D&;6YzWjBsnmks{_S3GMJ4+9V{Kf)Lz4(ZW>Ghlok|(Fktqg+XqwbgF_v< z`gR=Z(A{?khdOlk{e`N7xdUbnOdTRWz*LOVqaiRF0;3@?8UmvsFd71*Auu#TpyJls z6DK(t3J5PjYa!3siJs1TLQv~YqT?ZOr)L~IU_l9D4tqJbMwQgRt2N{&HEfdo6A zc?6 File { + File::open(format!("{FIXTURES}/{name}")).unwrap() +} + +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) +} + +/// Layout v1, chunked (50x50 chunks of a 100x200 dataset), deflate: every +/// read path goes through the version-1 B-tree chunk index. +#[test] +fn layout_v1_chunked_deflate() { + let file = open("deflate.h5"); + let ds = file.dataset("Dataset1").unwrap(); + assert_eq!(ds.shape().unwrap(), [100, 200]); + let expected: Vec = (0..100).flat_map(|_| (0..200).map(|j| j % 5)).collect(); + assert_eq!(ds.read_i32().unwrap(), expected); + + // A hyperslab that straddles four chunks. + let slab = Selection::Hyperslab { + start: vec![48, 48], + stride: vec![1, 1], + count: vec![4, 4], + block: vec![1, 1], + }; + let raw = ds.read_selection(&slab).unwrap(); + let got: Vec = raw + .as_chunks::<4>() + .0 + .iter() + .map(|b| i32::from_le_bytes(*b)) + .collect(); + assert_eq!(got, [3, 4, 0, 1, 3, 4, 0, 1, 3, 4, 0, 1, 3, 4, 0, 1]); +} + +/// Layout v2, contiguous: one dataset with storage, one never written (reads +/// as its fill value, 0). +#[test] +fn layout_v2_contiguous() { + let file = open("h5ex_g_iterate.h5"); + assert_eq!(file.dataset("G1/DS2").unwrap().read_i32().unwrap(), [1]); + assert_eq!(file.dataset("DS1").unwrap().read_i32().unwrap(), [0]); +} + +/// Every dataset in every fixture, byte for byte against h5py. +#[test] +fn legacy_fixtures_match_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; + } + for (name, datasets) in [ + ("deflate.h5", &["Dataset1"][..]), + ("h5ex_g_iterate.h5", &["DS1", "G1/DS2"][..]), + ] { + let path = format!("{FIXTURES}/{name}"); + let script = format!( + r#" +import h5py, numpy as np +f = h5py.File({path:?}, "r") +for n in {datasets:?}: + print(n, np.ascontiguousarray(f[n][()]).tobytes().hex()) +"# + ); + let out = Command::new(python()) + .args(["-c", &script]) + .output() + .unwrap(); + assert!( + out.status.success(), + "h5py: {}", + String::from_utf8_lossy(&out.stderr) + ); + let file = File::open(&path).unwrap(); + for line in String::from_utf8(out.stdout).unwrap().lines() { + let (ds, hex) = line.split_once(' ').unwrap(); + let ours = file + .dataset(ds) + .unwrap() + .read_selection(&Selection::All) + .unwrap(); + let ours: String = ours.iter().map(|b| format!("{b:02x}")).collect(); + assert_eq!(ours, hex, "{name}:{ds}"); + } + } +} diff --git a/docs/known-issues.md b/docs/known-issues.md index 6dfa369..135ff49 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -68,6 +68,8 @@ the VDS item, which is marked. - **Layout message versions 1 and 2** (HDF5 1.6-era files): 84 of the 686 sweep files, `InvalidLayoutVersion`. This is the largest single gap. + **Fixed 2026-09-25:** versions 1 and 2 are parsed (compact, contiguous, + chunked via the v1 B-tree). - **Virtual datasets:** - **Wrong data:** unmapped regions read as 0 instead of the fill value. - `%b` printf-style source names are not expanded. From 36356ba8a10628448f8581a141a4ce2b115ea499 Mon Sep 17 00:00:00 2001 From: osobh Date: Fri, 25 Sep 2026 21:53:11 -0500 Subject: [PATCH 2/3] fix(format): keep the array dimensions of compound v1 members Compound datatype version 1 carries, per member, a dimensionality and four dimension sizes (HDF5 before 1.4 had no array class). The parser skipped those 28 bytes, so a member such as `f: f32[4]` came back as a single f32 at the member's offset: the compound's size was right but its members were wrong. libhdf5 wraps such a member in an array type of the first `dimensionality` sizes and ignores the permutation; do the same, and reject a dimensionality above 4 as libhdf5 does. Only files old enough to also use layout message v1 have these, so this became reachable with the previous commit (tarrold.h5, tcompound.h5). Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 5 ++ crates/clawhdf5-format/src/datatype.rs | 85 +++++++++++++++++- .../tests/fixtures/legacy/README.md | 1 + .../tests/fixtures/legacy/tarrold.h5 | Bin 0 -> 6032 bytes .../clawhdf5/tests/legacy_format_interop.rs | 26 +++++- docs/known-issues.md | 4 + 6 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 crates/clawhdf5-format/tests/fixtures/legacy/tarrold.h5 diff --git a/CHANGELOG.md b/CHANGELOG.md index cf1b36d..bb55b44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -300,6 +300,11 @@ - Two threads reading two chunked datasets through one `File` could get each other's chunks (the shared chunk cache was switched between datasets across separate lock acquisitions). The cache is now keyed by dataset. + - Compound datatype version 1 members with legacy array dimensions (HDF5 + before 1.4, which had no array class) were read as a single scalar at + the member's offset; they are now array members, as in libhdf5 + (`tarrold.h5`, `tcompound.h5`). Only reachable once layout versions 1/2 + were readable, since the files that use it are that old. - `clawhdf5-format` reader — errors on valid files: enum and bool datasets through the numeric readers; the "don't filter partial edge chunks" layout flag; Fletcher32 ahead of deflate (NetCDF-4's order). Unknown-message flags diff --git a/crates/clawhdf5-format/src/datatype.rs b/crates/clawhdf5-format/src/datatype.rs index 436a6cf..6bcb810 100644 --- a/crates/clawhdf5-format/src/datatype.rs +++ b/crates/clawhdf5-format/src/datatype.rs @@ -423,13 +423,36 @@ impl Datatype { ensure_len(data, pos, 4)?; let byte_offset = LittleEndian::read_u32(&data[pos..pos + 4]) as u64; pos += 4; + // v1 members can be fixed-size arrays of their + // datatype (HDF5 before 1.4 had no array class): + // libhdf5 wraps such a member in an array type of the + // first `ndims` of the four stored dimensions and + // ignores the permutation. + let mut legacy_dims = Vec::new(); if version == 1 { ensure_len(data, pos, 28)?; + let ndims = data[pos] as usize; + if ndims > 4 { + return Err(FormatError::InvalidDatatypeVersion { + class: class_id, + version, + }); + } + for i in 0..ndims { + let at = pos + 12 + 4 * i; + legacy_dims.push(LittleEndian::read_u32(&data[at..at + 4])); + } pos += 28; } - let (member_dt, consumed) = + let (mut member_dt, consumed) = Self::parse_with_depth(&data[pos..], depth + 1)?; pos += consumed; + if !legacy_dims.is_empty() { + member_dt = Datatype::Array { + base_type: Box::new(member_dt), + dimensions: legacy_dims, + }; + } members.push(CompoundMember { name, byte_offset, @@ -1318,6 +1341,66 @@ mod tests { assert_xyid_compound(dt); } + /// A v1 compound member with legacy array dimensions (HDF5 before 1.4, + /// e.g. `tarrold.h5`): `{ i: i16, f: f32[2][3] }`. The member must become + /// an array type, not a scalar at the member's offset. + #[test] + fn test_compound_v1_legacy_array_member() { + let i16le: [u8; 12] = [ + 0x10, 0x08, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, + ]; + let f32le: [u8; 20] = [ + 0x11, 0x20, 0x1f, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x17, 0x08, + 0x00, 0x17, 0x7f, 0x00, 0x00, 0x00, + ]; + let mut b = vec![0x16, 0x02, 0x00, 0x00, 28, 0x00, 0x00, 0x00]; + for (name, offset, ndims, dims, dt) in [ + (&b"i"[..], 0u32, 0u8, [0u32; 4], &i16le[..]), + (&b"f"[..], 4, 2, [2, 3, 0, 0], &f32le[..]), + ] { + let mut padded = name.to_vec(); + padded.resize((name.len() + 1 + 7) & !7, 0); + b.extend_from_slice(&padded); + b.extend_from_slice(&offset.to_le_bytes()); + b.extend_from_slice(&[ndims, 0, 0, 0]); + b.extend_from_slice(&[0, 1, 2, 3]); // dimension permutation + b.extend_from_slice(&[0; 4]); + for d in dims { + b.extend_from_slice(&d.to_le_bytes()); + } + b.extend_from_slice(dt); + } + let (dt, consumed) = Datatype::parse(&b).unwrap(); + assert_eq!(consumed, b.len()); + let Datatype::Compound { size, members } = dt else { + panic!("expected Compound, got {dt:?}"); + }; + assert_eq!(size, 28); + assert!(matches!( + members[0].datatype, + Datatype::FixedPoint { size: 2, .. } + )); + match &members[1].datatype { + Datatype::Array { + base_type, + dimensions, + } => { + assert_eq!(dimensions, &[2, 3]); + assert!(matches!( + **base_type, + Datatype::FloatingPoint { size: 4, .. } + )); + } + other => panic!("expected an array member, got {other:?}"), + } + assert_eq!(members[1].datatype.type_size(), 24); + + // More than four legacy dimensions is not a valid message. + let mut bad = b.clone(); + bad[8 + 8 + 4] = 5; // first member's dimensionality + assert!(Datatype::parse(&bad).is_err()); + } + #[test] fn test_compound_v2_padded_names_no_array_fields() { // v2 = v1 without the 28 bytes of per-member array fields; names are diff --git a/crates/clawhdf5-format/tests/fixtures/legacy/README.md b/crates/clawhdf5-format/tests/fixtures/legacy/README.md index b7ee62d..a574759 100644 --- a/crates/clawhdf5-format/tests/fixtures/legacy/README.md +++ b/crates/clawhdf5-format/tests/fixtures/legacy/README.md @@ -9,3 +9,4 @@ write these structures, so they are kept as files. |---|---|---| | `deflate.h5` | `test/testfiles/deflate.h5` | Data Layout message v1, chunked + deflate (v1 B-tree index) | | `h5ex_g_iterate.h5` | `HDF5Examples/C/H5G/h5ex_g_iterate.h5` | Data Layout message v2, contiguous; an unallocated dataset | +| `tarrold.h5` | `test/testfiles/tarrold.h5` | Compound datatype v1 members with legacy array dimensions | diff --git a/crates/clawhdf5-format/tests/fixtures/legacy/tarrold.h5 b/crates/clawhdf5-format/tests/fixtures/legacy/tarrold.h5 new file mode 100644 index 0000000000000000000000000000000000000000..7747ce463fe03cb7ed6d0fa71b5ded8c93e94cb9 GIT binary patch literal 6032 zcmeHLy-veG4E8k@4dq7{2{nj?42%p(DFYKhYEUOYz`#f)Ktf`pJFILS8F_?`JW?N_ zTe0tK1eMYvA*71h!+rL}cb9y%U7cS#?c=rjRvk#f5UOAyaE2eoEdBVqEiUgBuNj_r zWxQtW6h~*IrfiF!ZSX`1T%H#NfB`vQP~5L-UfYxj#f4(PKn0@%AmOi$FmB17j6Z&i z&z9VCKLEz~z^M%k_EmJc7snZBL%@?sEZnhgm9Y$>WE=B}B!M`D_zATKXJg7SmI0k7 zYstrek9fm*C+nAa1B>rPl5>9L&Z0p)KPOaC29$w6V&LNBv`sZivTn%vT6UcLkG;u) zGRFm*H&F8>n~FP%)VF7@ZOK>Ni>tXOPf0B zi!x`Z0WoJkh`x?8pbRJj%78MU3@8K2fHLs!7-)BU-N)NrCLhLjQ*6PX-n&NJRF-=e z_1s7T7`ppe1DjlQ7yE4=j<17m@5#R0v_9Mhu9&v=Yai~$?GCWRSO!sY$oKyoS}V{R JBL}nkcOTQoMppm; literal 0 HcmV?d00001 diff --git a/crates/clawhdf5/tests/legacy_format_interop.rs b/crates/clawhdf5/tests/legacy_format_interop.rs index 5e1767a..e174c61 100644 --- a/crates/clawhdf5/tests/legacy_format_interop.rs +++ b/crates/clawhdf5/tests/legacy_format_interop.rs @@ -9,7 +9,7 @@ use std::process::Command; -use clawhdf5::File; +use clawhdf5::{DType, File}; use clawhdf5_format::selection::Selection; const FIXTURES: &str = concat!( @@ -73,6 +73,29 @@ fn layout_v2_contiguous() { assert_eq!(file.dataset("DS1").unwrap().read_i32().unwrap(), [0]); } +/// Compound datatype version 1 members carrying legacy array dimensions +/// (HDF5 before 1.4 had no array class). h5py: `[('i', ' Date: Fri, 25 Sep 2026 21:54:20 -0500 Subject: [PATCH 3/3] fix(format): locate the address in version-1 shared messages A version-1 shared message reference is version, type, six reserved bytes and then an old-style symbol table entry: link-name offset (length size), object header address, cache type, reserved, scratch. We read the address straight after the reserved bytes, i.e. the link-name offset, and the committed datatype lookup failed with InvalidObjectHeaderVersion (the bytes checked in tcompound.h5: name offset 0x10, then 0x590 = /type1). Datasets of 1.4/1.6-era files that use a committed datatype were unreadable. Skip the name offset. parse_shared_ref has no length size, so add parse_shared_ref_sized and use it in every internal caller; parse_shared_ref keeps its signature and assumes length size == offset size. The old parse_v1_ref unit test encoded the wrong layout and now uses the real bytes. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 8 ++ crates/clawhdf5-format/src/attribute.rs | 5 +- crates/clawhdf5-format/src/shared_message.rs | 49 +++++++++--- .../tests/fixtures/legacy/README.md | 1 + .../tests/fixtures/legacy/tcompound.h5 | Bin 0 -> 8192 bytes .../clawhdf5/tests/legacy_format_interop.rs | 75 ++++++++++++++++++ docs/known-issues.md | 2 + 7 files changed, 127 insertions(+), 13 deletions(-) create mode 100644 crates/clawhdf5-format/tests/fixtures/legacy/tcompound.h5 diff --git a/CHANGELOG.md b/CHANGELOG.md index bb55b44..2229e45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -305,6 +305,14 @@ the member's offset; they are now array members, as in libhdf5 (`tarrold.h5`, `tcompound.h5`). Only reachable once layout versions 1/2 were readable, since the files that use it are that old. +- `clawhdf5-format` reader — errors on valid files: a version-1 shared + message (a committed datatype in HDF5 1.4/1.6-era files) was read as if the + object header address followed the reserved bytes; it follows a link-name + offset (the reference is an old-style symbol table entry), so the reader + followed the name offset and failed with `InvalidObjectHeaderVersion` + (`tcompound.h5`). New `shared_message::parse_shared_ref_sized` takes the + superblock's length size; `parse_shared_ref` assumes it equals the offset + size. - `clawhdf5-format` reader — errors on valid files: enum and bool datasets through the numeric readers; the "don't filter partial edge chunks" layout flag; Fletcher32 ahead of deflate (NetCDF-4's order). Unknown-message flags diff --git a/crates/clawhdf5-format/src/attribute.rs b/crates/clawhdf5-format/src/attribute.rs index bb96bd5..36ded53 100644 --- a/crates/clawhdf5-format/src/attribute.rs +++ b/crates/clawhdf5-format/src/attribute.rs @@ -97,7 +97,7 @@ impl AttributeMessage { return Ok(Cow::Borrowed(bytes)); } let (file_data, offset_size) = file.ok_or(FormatError::UnresolvedSharedMessage)?; - let shared_ref = shared_message::parse_shared_ref(bytes, offset_size)?; + let shared_ref = shared_message::parse_shared_ref_sized(bytes, offset_size, length_size)?; shared_message::resolve_shared_message( file_data, &shared_ref, @@ -407,7 +407,8 @@ pub fn extract_attributes_full( if msg.msg_type == MessageType::Attribute { if shared_message::is_shared(msg.flags) { // Shared attribute: resolve the reference to get actual attribute data - let shared_ref = shared_message::parse_shared_ref(&msg.data, offset_size)?; + let shared_ref = + shared_message::parse_shared_ref_sized(&msg.data, offset_size, length_size)?; let resolved_data = shared_message::resolve_shared_message( file_data, &shared_ref, diff --git a/crates/clawhdf5-format/src/shared_message.rs b/crates/clawhdf5-format/src/shared_message.rs index 33334fa..9ab5d0f 100644 --- a/crates/clawhdf5-format/src/shared_message.rs +++ b/crates/clawhdf5-format/src/shared_message.rs @@ -154,13 +154,29 @@ pub fn is_shared(msg_flags: u8) -> bool { /// /// When the shared flag is set on a message, the data contains a reference /// instead of the actual message content. +/// +/// Assumes the file's length size equals its offset size, which only matters +/// for version-1 references; use [`parse_shared_ref_sized`] when the +/// superblock's length size is known. pub fn parse_shared_ref(data: &[u8], offset_size: u8) -> Result { + parse_shared_ref_sized(data, offset_size, offset_size) +} + +/// [`parse_shared_ref`] with the superblock's length size, which locates the +/// object header address in a version-1 reference. +pub fn parse_shared_ref_sized( + data: &[u8], + offset_size: u8, + length_size: u8, +) -> Result { ensure_len(data, 0, 2)?; let version = data[0]; let ref_type = data[1]; // Layouts (HDF5 spec IV.A.2 "Shared Message", and libhdf5's decoder): - // v1: version, type, reserved(6), address — always "committed" + // v1: version, type, reserved(6), then an old-style symbol table + // entry: link-name offset(length_size), object header address, + // cache type(4), reserved(4), scratch(16) — always "committed" // v2: version, type, address — always "committed" // v3: version, type, then a fractal-heap ID if type == SOHM, otherwise // an address @@ -177,7 +193,7 @@ pub fn parse_shared_ref(data: &[u8], offset_size: u8) -> Result address_at(2 + 6), + 1 => address_at(2 + 6 + length_size as usize), 2 => address_at(2), 3 if ref_type == SHARE_TYPE_SOHM => { ensure_len(data, 2, FHEAP_ID_LEN)?; @@ -434,7 +450,7 @@ pub fn message_data_with_sohm<'a>( if !is_shared(msg.flags) { return Ok(Cow::Borrowed(&msg.data)); } - let shared_ref = parse_shared_ref(&msg.data, offset_size)?; + let shared_ref = parse_shared_ref_sized(&msg.data, offset_size, length_size)?; let table = if shared_ref.heap_id.is_some() { load_sohm_table(file_data, offset_size, length_size)? } else { @@ -514,7 +530,7 @@ pub fn message_data<'a>( if !is_shared(msg.flags) { return Ok(Cow::Borrowed(&msg.data)); } - let shared_ref = parse_shared_ref(&msg.data, offset_size)?; + let shared_ref = parse_shared_ref_sized(&msg.data, offset_size, length_size)?; resolve_shared_message( file_data, &shared_ref, @@ -649,15 +665,26 @@ mod tests { #[test] fn parse_v1_ref() { - let mut data = Vec::new(); - data.push(1); // version - data.push(0); // type - data.extend_from_slice(&[0u8; 6]); // reserved - data.extend_from_slice(&0x5678u64.to_le_bytes()); + // Datatype message of `/group1/dset2` in HDF5's `tcompound.h5` + // (written in 2000): version 1, six reserved bytes, then an old-style + // symbol table entry — link-name offset 0x10, object header address + // 0x590 (the committed datatype `/type1`), cache type, reserved and + // scratch. + let mut data = vec![1, 0, 0, 0, 0, 0, 0, 0]; + data.extend_from_slice(&0x10u64.to_le_bytes()); + data.extend_from_slice(&0x590u64.to_le_bytes()); + data.extend_from_slice(&[0; 24]); - let shared = parse_shared_ref(&data, 8).unwrap(); + let shared = parse_shared_ref_sized(&data, 8, 8).unwrap(); assert_eq!(shared.version, 1); - assert_eq!(shared.object_header_address, Some(0x5678)); + assert_eq!(shared.object_header_address, Some(0x590)); + + // The name offset is a length: 4 bytes here, then an 8-byte address. + let mut data = vec![1, 0, 0, 0, 0, 0, 0, 0]; + data.extend_from_slice(&0x10u32.to_le_bytes()); + data.extend_from_slice(&0x590u64.to_le_bytes()); + let shared = parse_shared_ref_sized(&data, 8, 4).unwrap(); + assert_eq!(shared.object_header_address, Some(0x590)); } #[test] diff --git a/crates/clawhdf5-format/tests/fixtures/legacy/README.md b/crates/clawhdf5-format/tests/fixtures/legacy/README.md index a574759..0eda271 100644 --- a/crates/clawhdf5-format/tests/fixtures/legacy/README.md +++ b/crates/clawhdf5-format/tests/fixtures/legacy/README.md @@ -10,3 +10,4 @@ write these structures, so they are kept as files. | `deflate.h5` | `test/testfiles/deflate.h5` | Data Layout message v1, chunked + deflate (v1 B-tree index) | | `h5ex_g_iterate.h5` | `HDF5Examples/C/H5G/h5ex_g_iterate.h5` | Data Layout message v2, contiguous; an unallocated dataset | | `tarrold.h5` | `test/testfiles/tarrold.h5` | Compound datatype v1 members with legacy array dimensions | +| `tcompound.h5` | `tools/test/testfiles/tcompound.h5` | Version-1 shared messages (committed datatypes); compound v1 array members with data | diff --git a/crates/clawhdf5-format/tests/fixtures/legacy/tcompound.h5 b/crates/clawhdf5-format/tests/fixtures/legacy/tcompound.h5 new file mode 100644 index 0000000000000000000000000000000000000000..d1ec6504cafee27eeda2e6ea97f95bd9bfc8c97d GIT binary patch literal 8192 zcmeHMJ8u&~5T5g0f(Z{f1xa`x9i>SEazlJ45tIT!#G}Rr!b5^23PFMjmmpE31giXu zl(a}eN`pk{G9@J)d~-8%an84P3`B{*Bkj)Y?CkB_?9A-m-rJcgSC0&x7$SyZkpe1_ zpERWUsX$?-tuku`B^+pGI-cdOn)a6!ubooDfo|WNo+k3h<~MBOGl5W{G5YwwvVcbg zcn6tV(lGp%+wav1HN}QJ8ch17BKY`PLXN=MOAxBxov%NeGif(29VEmELrC{@jJl$8 z(D1pl>6pKf|~<^&kLfp<3gC+`%!7v6!RJ~tmrstwb!AtHYMY=3+yq+gJ-ho zBGtpzc~;kXCDkuXsZK;T|C&9U`aIXzZuxf=all~fD6M||zgWPPf3xvx_Tb!*#I6Rg zPvzvCVe!1v`1Lfuc{@K6@@3NqlO`Yn%>)U#+R?iKc`@7BLy@o!PkzYfK(eevs=!!jX{=do{e{;iXD%GgbUmEkw>`79BEh*0r4 zB3ecHghD8&w(*+zyqIMhD61%+P?|){<8w$JFAmYk9}TJla1f!|&H1u=#Ub=7W6~BK zKo8{U@H_A4ny=M1aVR>(5oYG2t{ho8+d(t zK>&|)4xqqi8i3C^1rTtqwEGvNwFJygvuBmzK;7 Vec<(i32, f32)> { + file.dataset(name) + .unwrap() + .read_selection(&Selection::All) + .unwrap() + .as_chunks::<8>() + .0 + .iter() + .map(|b| { + ( + i32::from_be_bytes(b[..4].try_into().unwrap()), + f32::from_be_bytes(b[4..].try_into().unwrap()), + ) + }) + .collect() + }; + assert_eq!( + be_pairs("group1/dset2"), + [(0, 0.0), (1, 1.1), (2, 2.2), (3, 3.3), (4, 4.4)] + ); + assert_eq!( + be_pairs("group2/dset5"), + [(0, 0.0), (1, 0.1), (2, 0.2), (3, 0.3), (4, 0.4)] + ); + + // `/type2`: { int_array: i32[4], float_array: f32[5][6] }, whose array + // members are compound v1 legacy dimensions. + let dset3 = file.dataset("group1/dset3").unwrap(); + assert_eq!( + dset3.dtype().unwrap(), + DType::Compound(vec![ + ( + "int_array".into(), + DType::Array(Box::new(DType::I32), vec![4]) + ), + ( + "float_array".into(), + DType::Array(Box::new(DType::F32), vec![5, 6]) + ), + ]) + ); + let raw = dset3.read_selection(&Selection::All).unwrap(); + assert_eq!(raw.len(), 3 * 6 * (16 + 120)); + assert_eq!( + &raw[..16], + &[0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 2, 0, 0, 0, 3] + ); + let first: Vec = raw[16..16 + 120] + .as_chunks::<4>() + .0 + .iter() + .map(|b| f32::from_be_bytes(*b)) + .collect(); + let expected: Vec = (0..5) + .flat_map(|i| (0..6).map(move |j| (1 + i + j) as f32)) + .collect(); + assert_eq!(first, expected); +} + /// Every dataset in every fixture, byte for byte against h5py. #[test] fn legacy_fixtures_match_h5py() { @@ -111,6 +176,16 @@ fn legacy_fixtures_match_h5py() { ("deflate.h5", &["Dataset1"][..]), ("h5ex_g_iterate.h5", &["DS1", "G1/DS2"][..]), ("tarrold.h5", &["Dataset1", "Dataset2"][..]), + ( + "tcompound.h5", + &[ + "dset1", + "group1/dset2", + "group1/dset3", + "group1/dset4", + "group2/dset5", + ][..], + ), ] { let path = format!("{FIXTURES}/{name}"); let script = format!( diff --git a/docs/known-issues.md b/docs/known-issues.md index da70ab8..e0a1cae 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -80,6 +80,8 @@ the VDS item, which is marked. - Hyperslab selection versions 1 and 2 are refused. - **Files with a user block:** the base address is not applied. - **Old-style shared messages (version 1)** read the wrong address. + **Fixed 2026-09-25:** the address follows the link-name offset of the + embedded symbol table entry. - **Groups and links:** - Groups with a user-defined link type (e.g. 187) cannot be listed. - Dense groups with more than about 22 000 links cannot be listed.