fix(format): read array members of version-1 compound datatypes

HDF5 1.6 encoded a compound member that is a fixed-size array through
legacy per-member fields (dimensionality, permutation, four dimension
sizes) that the v1 decoder skipped, so a [4] i32 member read as one i32
with the wrong size. Build the array type from those fields as libhdf5
does (ignoring the permutation) and refuse more than four dimensions.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
osobh
2026-09-25 22:07:50 -05:00
co-authored by Claude Opus 5.5
parent 42b81d9f1c
commit efc2dc53c9
3 changed files with 98 additions and 1 deletions
+6
View File
@@ -273,6 +273,12 @@
- CI keeps zlib-ng building and tested; the arm64 job no longer needs cmake. - CI keeps zlib-ng building and tested; the arm64 job no longer needs cmake.
### Correctness ### 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:** - `clawhdf5-format` reader — **values returned wrong with no error:**
- Fixed Array and Extensible Array chunk indexes were laid out by the - 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, dataset's current shape instead of its max shape (23 libhdf5 test files,
+90 -1
View File
@@ -423,13 +423,44 @@ impl Datatype {
ensure_len(data, pos, 4)?; ensure_len(data, pos, 4)?;
let byte_offset = LittleEndian::read_u32(&data[pos..pos + 4]) as u64; let byte_offset = LittleEndian::read_u32(&data[pos..pos + 4]) as u64;
pos += 4; 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 { if version == 1 {
ensure_len(data, pos, 28)?; 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; pos += 28;
} }
let (member_dt, consumed) = let (mut member_dt, consumed) =
Self::parse_with_depth(&data[pos..], depth + 1)?; Self::parse_with_depth(&data[pos..], depth + 1)?;
pos += consumed; pos += consumed;
if !array_dims.is_empty() {
member_dt = Datatype::Array {
base_type: Box::new(member_dt),
dimensions: array_dims,
};
}
members.push(CompoundMember { members.push(CompoundMember {
name, name,
byte_offset, byte_offset,
@@ -1336,6 +1367,64 @@ mod tests {
assert_xyid_compound(dt); 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<Vec<u32>>)> = 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] #[test]
fn test_compound_v1_truncated_is_error_not_panic() { fn test_compound_v1_truncated_is_error_not_panic() {
let bytes = compound_v1_bytes(); let bytes = compound_v1_bytes();
+2
View File
@@ -74,6 +74,8 @@ the VDS item, which is marked.
- Hyperslab selection versions 1 and 2 are refused. - Hyperslab selection versions 1 and 2 are refused.
- **Files with a user block:** the base address is not applied. - **Files with a user block:** the base address is not applied.
- **Old-style shared messages (version 1)** read the wrong address. - **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 and links:**
- Groups with a user-defined link type (e.g. 187) cannot be listed. - 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. - Dense groups with more than about 22 000 links cannot be listed.