From 36356ba8a10628448f8581a141a4ce2b115ea499 Mon Sep 17 00:00:00 2001 From: osobh Date: Fri, 25 Sep 2026 21:53:11 -0500 Subject: [PATCH] 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', '