diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b87415..ac26b3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -273,6 +273,12 @@ - CI keeps zlib-ng building and tested; the arm64 job no longer needs cmake. ### Correctness +- `clawhdf5-format` reader: array members of version-1 compound datatypes + (HDF5 1.6-era files, e.g. libhdf5's `tcompound.h5`) were read as a single + element: a `[4] i32` member came back as one `i32`, with the wrong size. + The legacy per-member dimension fields are now decoded into an array type, + as libhdf5 does; more than four dimensions, or a zero-sized one, is an + error. - `clawhdf5-format` reader — **values returned wrong with no error:** - Fixed Array and Extensible Array chunk indexes were laid out by the dataset's current shape instead of its max shape (23 libhdf5 test files, diff --git a/crates/clawhdf5-format/src/datatype.rs b/crates/clawhdf5-format/src/datatype.rs index 436a6cf..ba85afa 100644 --- a/crates/clawhdf5-format/src/datatype.rs +++ b/crates/clawhdf5-format/src/datatype.rs @@ -423,13 +423,44 @@ 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 the member + // type (libhdf5 builds an array type from these + // fields; the permutation is ignored, as libhdf5 + // does). Skipping them read a `[4] i32` member as + // one `i32`. + let mut array_dims = Vec::new(); if version == 1 { ensure_len(data, pos, 28)?; + let ndims = data[pos] as usize; + // libhdf5 refuses more than four dimensions and, + // when building the array type, a zero-sized one. + let zero_dim = (0..ndims.min(4)).any(|j| { + let at = pos + 12 + 4 * j; + LittleEndian::read_u32(&data[at..at + 4]) == 0 + }); + if ndims > 4 || zero_dim { + return Err(FormatError::InvalidDatatypeVersion { + class: class_id, + version, + }); + } + array_dims = (0..ndims) + .map(|j| { + let at = pos + 12 + 4 * j; + LittleEndian::read_u32(&data[at..at + 4]) + }) + .collect(); pos += 28; } - let (member_dt, consumed) = + let (mut member_dt, consumed) = Self::parse_with_depth(&data[pos..], depth + 1)?; pos += consumed; + if !array_dims.is_empty() { + member_dt = Datatype::Array { + base_type: Box::new(member_dt), + dimensions: array_dims, + }; + } members.push(CompoundMember { name, byte_offset, @@ -1336,6 +1367,64 @@ mod tests { assert_xyid_compound(dt); } + #[test] + fn test_compound_v1_member_array_fields() { + // HDF5 1.6 wrote array members of a v1 compound through the legacy + // per-member fields (as in libhdf5's tools/test/testfiles/ + // tcompound.h5 `type2`: `int_array` [4] i32, `float_array` [5][6] + // f32). They used to be skipped, reading each member as a scalar. + let i32le: [u8; 12] = [ + 0x10, 0x08, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, + ]; + let mut b = vec![0x16, 0x02, 0x00, 0x00, 0x88, 0x00, 0x00, 0x00]; + for (name, offset, dims) in [ + (&b"int_array"[..], 0u32, &[4u32][..]), + (&b"xy"[..], 16, &[5u32, 6][..]), + ] { + 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.push(dims.len() as u8); + b.extend_from_slice(&[0u8; 3 + 4 + 4]); // reserved, permutation, reserved + for j in 0..4 { + b.extend_from_slice(&dims.get(j).copied().unwrap_or(0).to_le_bytes()); + } + b.extend_from_slice(&i32le); + } + let (dt, consumed) = Datatype::parse(&b).unwrap(); + assert_eq!(consumed, b.len()); + let Datatype::Compound { members, .. } = dt else { + panic!("expected Compound, got {dt:?}"); + }; + let got: Vec<(&str, u64, u32, Option>)> = members + .iter() + .map(|m| { + let dims = match &m.datatype { + Datatype::Array { dimensions, .. } => Some(dimensions.clone()), + _ => None, + }; + (m.name.as_str(), m.byte_offset, m.datatype.type_size(), dims) + }) + .collect(); + assert_eq!( + got, + vec![ + ("int_array", 0, 16, Some(vec![4])), + ("xy", 16, 120, Some(vec![5, 6])), + ] + ); + + // More than four dimensions cannot be encoded, and libhdf5 refuses a + // zero-sized dimension (a fuzzed tcompound.h5, cve-2024-32616.h5). + let mut bad = b.clone(); + bad[8 + 16 + 4] = 5; + assert!(Datatype::parse(&bad).is_err()); + let mut bad = b.clone(); + bad[8 + 16 + 4] = 2; // [4, 0] + assert!(Datatype::parse(&bad).is_err()); + } + #[test] fn test_compound_v1_truncated_is_error_not_panic() { let bytes = compound_v1_bytes(); diff --git a/docs/known-issues.md b/docs/known-issues.md index 6dfa369..660dab0 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -74,6 +74,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. +- **Array members of version-1 compound datatypes** (found while fixing the + item above) were read as one element. **Fixed 2026-09-25.** - **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.