diff --git a/CHANGELOG.md b/CHANGELOG.md index b5c479f..ec610fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -296,6 +296,41 @@ - CI keeps zlib-ng building and tested; the arm64 job no longer needs cmake. ### Correctness +- `clawhdf5-format` reader: an old-style group whose local heap has a free + list pointing outside the heap was listed with names read from the broken + heap (garbage names on `cve-2021-36977.h5` once its user block was + applied). libhdf5 refuses such a heap ("bad heap free list"); so do we now, + with `FormatError::InvalidLocalHeapFreeList`. As in libhdf5 the free list + is checked when the first name is read (`LocalHeap::validate_free_list`, + new), so an empty group with a damaged heap still lists as empty. +- **Files with a user block** (`h5py.File(..., userblock_size=N)`, `h5jam`; + the superblock at 512, 1024, …) could not be read: every address in the + file is relative to the superblock, but it was applied from byte 0 + (`InvalidObjectHeaderVersion` on the root group). `File` (mmap, buffered, + `from_bytes`), `MmapFile`, `LazyFile`, `AsyncHDF5File`, the VOL readers, + the HNSW loader and external VDS sources now view the file from the + superblock on, using the signature's position as the base address as + libhdf5 does; `user_block_size()` reports the user block (h5py's + `userblock_size`), and `as_bytes()` returns the bytes from the superblock + on. **Breaking (format crate):** `Superblock::parse` refuses a non-zero + signature offset with `FormatError::UserBlockNotStripped`, since the + addresses it returns would be applied to the wrong bytes; pass the slice + from `signature::split_user_block` (new) and parse at offset 0. +- `clawhdf5-format` reader: version-1 shared messages (HDF5 1.6-era files, + e.g. a dataset using a committed datatype in libhdf5's `tcompound.h5`) + read the heap-offset field of the embedded symbol-table entry as the + target address and failed with `InvalidObjectHeaderVersion`. The address + is now read after it, as libhdf5 does. **Breaking (format crate):** + `shared_message::parse_shared_ref` takes `length_size`. A reference whose + target header has no message of the referenced type is now + `FormatError::SharedMessageTargetMissing` instead of returning the first + other message found there (which decoded as garbage). +- `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-ann/src/hnsw.rs b/crates/clawhdf5-ann/src/hnsw.rs index a526bee..a725b3f 100644 --- a/crates/clawhdf5-ann/src/hnsw.rs +++ b/crates/clawhdf5-ann/src/hnsw.rs @@ -13,7 +13,7 @@ use clawhdf5_format::filter_pipeline::FilterPipeline; use clawhdf5_format::group_v2::resolve_path_any; use clawhdf5_format::message_type::MessageType; use clawhdf5_format::object_header::ObjectHeader; -use clawhdf5_format::signature::find_signature; +use clawhdf5_format::signature::split_user_block; use clawhdf5_format::superblock::Superblock; use clawhdf5_io::FileWriter as IoFileWriter; @@ -861,8 +861,9 @@ impl HnswIndex { /// The HDF5 data must contain the `/ann/vectors`, `/ann/graph_layer_*`, /// and `/ann/config` datasets as produced by [`to_hdf5_bytes`]. pub fn load_from_hdf5(data: &[u8]) -> Result { - let sig_offset = find_signature(data)?; - let sb = Superblock::parse(data, sig_offset)?; + // Addresses are relative to the superblock: skip any user block. + let (_, data) = split_user_block(data)?; + let sb = Superblock::parse(data, 0)?; // Read config dataset and its attributes let config_attrs = read_dataset_attrs(data, &sb, "ann/config")?; diff --git a/crates/clawhdf5-format/src/data_read.rs b/crates/clawhdf5-format/src/data_read.rs index 82e6544..8d3b16e 100644 --- a/crates/clawhdf5-format/src/data_read.rs +++ b/crates/clawhdf5-format/src/data_read.rs @@ -575,11 +575,13 @@ fn read_named_dataset_raw( use crate::group_v2::resolve_path_any; use crate::message_type::MessageType; use crate::object_header::ObjectHeader; - use crate::signature::find_signature; + use crate::signature::split_user_block; use crate::superblock::Superblock; - let sig = find_signature(file_data)?; - let sb = Superblock::parse(file_data, sig)?; + // An external source file is handed over whole, user block included; + // its addresses are relative to its superblock. + let (_, file_data) = split_user_block(file_data)?; + let sb = Superblock::parse(file_data, 0)?; let addr = resolve_path_any(file_data, &sb, path)?; let hdr = ObjectHeader::parse(file_data, addr as usize, sb.offset_size, sb.length_size)?; diff --git a/crates/clawhdf5-format/src/datatype.rs b/crates/clawhdf5-format/src/datatype.rs index 6bcb810..ba85afa 100644 --- a/crates/clawhdf5-format/src/datatype.rs +++ b/crates/clawhdf5-format/src/datatype.rs @@ -423,34 +423,42 @@ 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(); + // 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; - if ndims > 4 { + // 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, }); } - for i in 0..ndims { - let at = pos + 12 + 4 * i; - legacy_dims.push(LittleEndian::read_u32(&data[at..at + 4])); - } + array_dims = (0..ndims) + .map(|j| { + let at = pos + 12 + 4 * j; + LittleEndian::read_u32(&data[at..at + 4]) + }) + .collect(); pos += 28; } let (mut member_dt, consumed) = Self::parse_with_depth(&data[pos..], depth + 1)?; pos += consumed; - if !legacy_dims.is_empty() { + if !array_dims.is_empty() { member_dt = Datatype::Array { base_type: Box::new(member_dt), - dimensions: legacy_dims, + dimensions: array_dims, }; } members.push(CompoundMember { @@ -1341,66 +1349,6 @@ 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 @@ -1419,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/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index 6d10c3d..bb81939 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -80,6 +80,9 @@ pub enum FormatError { InvalidLocalHeapSignature, /// Invalid local heap version. InvalidLocalHeapVersion(u8), + /// A local heap's free list points outside its data segment (libhdf5: + /// "bad heap free list"). + InvalidLocalHeapFreeList, /// Invalid B-tree v1 signature. InvalidBTreeSignature, /// Invalid B-tree node type. @@ -117,6 +120,14 @@ pub enum FormatError { /// A message is marked shared but was parsed without access to the file, /// so the reference to the real message could not be followed. UnresolvedSharedMessage, + /// A shared-message reference points at an object header that holds no + /// (unshared) message of the referenced type (raw message type id). + SharedMessageTargetMissing(u16), + /// A superblock was parsed at a non-zero offset of the buffer (the file + /// has a user block of this many bytes). HDF5 addresses are relative to + /// the superblock, so the buffer must start there: see + /// `signature::split_user_block`. + UserBlockNotStripped(u64), /// A selection does not fit the dataset it was applied to (wrong rank, or /// it reaches past a dimension's extent). SelectionOutOfBounds(String), @@ -270,6 +281,9 @@ impl fmt::Display for FormatError { FormatError::InvalidLocalHeapSignature => { write!(f, "invalid local heap signature") } + FormatError::InvalidLocalHeapFreeList => { + write!(f, "bad local heap free list") + } FormatError::InvalidLocalHeapVersion(v) => { write!(f, "invalid local heap version: {v}") } @@ -339,6 +353,16 @@ impl fmt::Display for FormatError { FormatError::SelectionOutOfBounds(msg) => { write!(f, "selection out of bounds: {msg}") } + FormatError::UserBlockNotStripped(n) => write!( + f, + "file has a {n}-byte user block: parse the bytes from the superblock on \ + (signature::split_user_block)" + ), + FormatError::SharedMessageTargetMissing(t) => write!( + f, + "shared message reference points at an object header with no message of type \ + {t:#06x}" + ), FormatError::UnresolvedSharedMessage => write!( f, "message is shared but no file data was available to resolve it" diff --git a/crates/clawhdf5-format/src/group_v1.rs b/crates/clawhdf5-format/src/group_v1.rs index 989f826..e55ad16 100644 --- a/crates/clawhdf5-format/src/group_v1.rs +++ b/crates/clawhdf5-format/src/group_v1.rs @@ -45,9 +45,16 @@ pub fn resolve_v1_group_entries( )?; let mut entries = Vec::new(); + let mut heap_checked = false; for snod_addr in snod_addrs { let snod = SymbolTableNode::parse(file_data, snod_addr as usize, offset_size)?; for entry in &snod.entries { + // Like libhdf5, look at the heap's free list only once a name is + // needed: an empty group with a damaged heap still lists. + if !heap_checked { + heap.validate_free_list(file_data, length_size)?; + heap_checked = true; + } let name = heap.read_string(file_data, entry.link_name_offset)?; entries.push(GroupEntry { name, @@ -85,12 +92,17 @@ pub fn find_v1_soft_link( offset_size, length_size, )?; + let mut heap_checked = false; for snod_addr in snod_addrs { let snod = SymbolTableNode::parse(file_data, snod_addr as usize, offset_size)?; for entry in &snod.entries { if entry.cache_type != CACHE_TYPE_SOFT_LINK { continue; } + if !heap_checked { + heap.validate_free_list(file_data, length_size)?; + heap_checked = true; + } if heap.read_string(file_data, entry.link_name_offset)? != name { continue; } diff --git a/crates/clawhdf5-format/src/lib.rs b/crates/clawhdf5-format/src/lib.rs index 4a6c2a1..ffed10f 100644 --- a/crates/clawhdf5-format/src/lib.rs +++ b/crates/clawhdf5-format/src/lib.rs @@ -26,12 +26,13 @@ //! use clawhdf5_format::{signature, superblock, object_header, group_v2, //! datatype, dataspace, data_layout, data_read, message_type::MessageType}; //! -//! let file_data = std::fs::read("output.h5").unwrap(); -//! let sig = signature::find_signature(&file_data).unwrap(); -//! let sb = superblock::Superblock::parse(&file_data, sig).unwrap(); -//! let addr = group_v2::resolve_path_any(&file_data, &sb, "data").unwrap(); +//! let bytes = std::fs::read("output.h5").unwrap(); +//! // Addresses are relative to the superblock: skip any user block. +//! let (_user_block, file_data) = signature::split_user_block(&bytes).unwrap(); +//! let sb = superblock::Superblock::parse(file_data, 0).unwrap(); +//! let addr = group_v2::resolve_path_any(file_data, &sb, "data").unwrap(); //! let hdr = object_header::ObjectHeader::parse( -//! &file_data, addr as usize, sb.offset_size, sb.length_size).unwrap(); +//! file_data, addr as usize, sb.offset_size, sb.length_size).unwrap(); //! ``` //! //! # Features diff --git a/crates/clawhdf5-format/src/local_heap.rs b/crates/clawhdf5-format/src/local_heap.rs index 33e3433..39e9b26 100644 --- a/crates/clawhdf5-format/src/local_heap.rs +++ b/crates/clawhdf5-format/src/local_heap.rs @@ -87,6 +87,57 @@ impl LocalHeap { }) } + /// Walk the free list the way libhdf5 does when it loads a heap's data + /// (`H5HL__fl_deserialize`), rejecting a heap whose free list points + /// outside the data segment. libhdf5 refuses such a heap ("bad heap free + /// list"), and names read from it would be garbage. + /// + /// libhdf5 only loads a heap when it needs a name from it (an empty + /// group's broken heap goes unnoticed), so call this before the first + /// [`Self::read_string`], not on parse. + /// + /// The end of the list is `H5HL_FREE_NULL` (1); an all-ones value (the + /// undefined address) is accepted as "no free list" too. + pub fn validate_free_list(&self, file_data: &[u8], length_size: u8) -> Result<(), FormatError> { + const FREE_NULL: u64 = 1; + let ls = length_size as usize; + let undefined = if ls >= 8 { + u64::MAX + } else { + (1u64 << (8 * ls)) - 1 + }; + let size = self.data_segment_size; + let seg = self.data_segment_address; + let mut next = self.free_list_head_offset; + // Each free block holds two lengths, so a list longer than this + // revisits a block: a cycle. + let max_blocks = size / (2 * ls as u64) + 1; + let mut walked = 0u64; + while next != FREE_NULL && next != undefined { + if next >= size || walked >= max_blocks { + return Err(FormatError::InvalidLocalHeapFreeList); + } + walked += 1; + let at = seg + .checked_add(next) + .and_then(|a| usize::try_from(a).ok()) + .ok_or(FormatError::InvalidLocalHeapFreeList)?; + let block_offset = next; + next = read_offset(file_data, at, length_size)?; + if next == 0 { + return Err(FormatError::InvalidLocalHeapFreeList); + } + let block_size = read_offset(file_data, at + ls, length_size)?; + if block_offset + .checked_add(block_size) + .is_none_or(|end| end > size) + { + return Err(FormatError::InvalidLocalHeapFreeList); + } + } + Ok(()) + } + /// Read a null-terminated string from the heap's data segment at the given byte offset. pub fn read_string(&self, file_data: &[u8], string_offset: u64) -> Result { let seg_addr = self.data_segment_address as usize; @@ -162,8 +213,8 @@ mod tests { // data_segment_size write_val(&mut file, pos, data_seg_size as u64, length_size); pos += length_size as usize; - // free_list_head_offset - write_val(&mut file, pos, 0xFFFFFFFF, length_size); + // free_list_head_offset: H5HL_FREE_NULL (no free space) + write_val(&mut file, pos, 1, length_size); pos += length_size as usize; // data_segment_address write_val(&mut file, pos, data_seg_offset as u64, offset_size); @@ -243,6 +294,50 @@ mod tests { assert_eq!(s, "test"); } + /// Heap with data segment `[a, b, c, 0-padding]` whose free list starts + /// at `head` and has one block `(next, size)` at offset 8. + fn heap_with_free_block(head: u64, next: u64, size: u64) -> Vec { + let mut file = build_heap_file(0, 100, &["abcdefg"], 8, 8); + file.resize(200, 0); + write_val(&mut file, 8, 32, 8); // data segment size + write_val(&mut file, 16, head, 8); + write_val(&mut file, 108, next, 8); + write_val(&mut file, 116, size, 8); + file + } + + #[test] + fn free_list_inside_the_segment_is_accepted() { + let file = heap_with_free_block(8, 1, 24); + let heap = LocalHeap::parse(&file, 0, 8, 8).unwrap(); + heap.validate_free_list(&file, 8).unwrap(); + assert_eq!(heap.read_string(&file, 0).unwrap(), "abcdefg"); + // An all-ones head is "no free list" too. + let file = heap_with_free_block(u64::MAX, 0, 0); + let heap = LocalHeap::parse(&file, 0, 8, 8).unwrap(); + assert!(heap.validate_free_list(&file, 8).is_ok()); + } + + #[test] + fn bad_free_list_is_rejected_like_libhdf5() { + for (head, next, size, why) in [ + (40, 1, 8, "head past the segment"), + (8, 1, 25, "block runs past the segment"), + (8, 0, 8, "next offset of zero"), + (8, 8, 8, "cycle"), + (8, 999, 8, "next past the segment"), + ] { + let file = heap_with_free_block(head, next, size); + // The header itself parses; the free list is checked on use. + let heap = LocalHeap::parse(&file, 0, 8, 8).unwrap(); + assert_eq!( + heap.validate_free_list(&file, 8).unwrap_err(), + FormatError::InvalidLocalHeapFreeList, + "{why}" + ); + } + } + #[test] fn invalid_version() { let mut file = build_heap_file(0, 100, &["x"], 8, 8); diff --git a/crates/clawhdf5-format/src/signature.rs b/crates/clawhdf5-format/src/signature.rs index 27d5e75..600b650 100644 --- a/crates/clawhdf5-format/src/signature.rs +++ b/crates/clawhdf5-format/src/signature.rs @@ -11,6 +11,16 @@ pub const HDF5_SIGNATURE: [u8; 8] = [0x89, b'H', b'D', b'F', b'\r', b'\n', 0x1A, /// (powers of two starting at 512, plus offset 0). /// /// Returns the byte offset where the signature was found. +/// +/// A non-zero offset means the file starts with a *user block*, and every +/// address inside the file is relative to the superblock's position, not to +/// byte 0 (libhdf5 uses the signature's position as the base address even +/// when the stored base-address field disagrees). The parsers in this crate +/// take addresses as indices into `file_data`, so they must be handed the +/// bytes from the signature on — use [`split_user_block`]. [`Superblock::parse`] +/// refuses a non-zero offset for this reason. +/// +/// [`Superblock::parse`]: crate::superblock::Superblock::parse pub fn find_signature(data: &[u8]) -> Result { // Check offset 0 if data.len() >= 8 && data[..8] == HDF5_SIGNATURE { @@ -29,6 +39,17 @@ pub fn find_signature(data: &[u8]) -> Result { Err(FormatError::SignatureNotFound) } +/// Split a file into its user block and its HDF5 bytes. +/// +/// Returns `(user_block, hdf5)`: `user_block` is everything before the +/// superblock signature (empty for most files) and `hdf5` is the rest, in +/// which every HDF5 address is a plain index. Pass `hdf5` as `file_data` to +/// every parser in this crate, and parse the superblock at offset 0 of it. +pub fn split_user_block(data: &[u8]) -> Result<(&[u8], &[u8]), FormatError> { + let offset = find_signature(data)?; + Ok(data.split_at(offset)) +} + #[cfg(test)] mod tests { use super::*; @@ -88,6 +109,21 @@ mod tests { assert_eq!(find_signature(&data), Err(FormatError::SignatureNotFound)); } + #[test] + fn split_user_block_rebases_at_the_signature() { + let mut data = vec![7u8; 1024]; + data[512..520].copy_from_slice(&HDF5_SIGNATURE); + let (ub, hdf5) = split_user_block(&data).unwrap(); + assert_eq!(ub.len(), 512); + assert_eq!(hdf5.len(), 512); + assert_eq!(&hdf5[..8], &HDF5_SIGNATURE); + + data[..8].copy_from_slice(&HDF5_SIGNATURE); + let (ub, hdf5) = split_user_block(&data).unwrap(); + assert!(ub.is_empty()); + assert_eq!(hdf5.len(), 1024); + } + #[test] fn signature_prefers_earliest() { // Signature at both 0 and 512, should return 0 diff --git a/crates/clawhdf5-format/src/superblock.rs b/crates/clawhdf5-format/src/superblock.rs index e971495..d2ec4d6 100644 --- a/crates/clawhdf5-format/src/superblock.rs +++ b/crates/clawhdf5-format/src/superblock.rs @@ -174,8 +174,18 @@ impl Superblock { /// Parse a superblock from `data` starting at `signature_offset`. /// - /// The signature must be present at the given offset. + /// The signature must be present at the given offset, and that offset + /// must be 0: every address in an HDF5 file is relative to the + /// superblock, so when a file has a user block (signature at 512, 1024, + /// …) the caller must pass the bytes from the signature on — see + /// [`crate::signature::split_user_block`] — and use that slice as + /// `file_data` everywhere. A non-zero offset is refused with + /// [`FormatError::UserBlockNotStripped`] because the addresses in the + /// returned superblock would otherwise be applied to the wrong bytes. pub fn parse(data: &[u8], signature_offset: usize) -> Result { + if signature_offset != 0 { + return Err(FormatError::UserBlockNotStripped(signature_offset as u64)); + } let d = data .get(signature_offset..) .ok_or(FormatError::UnexpectedEof { @@ -676,7 +686,16 @@ mod tests { let mut data = vec![0u8; 1024]; let v0 = build_v0_bytes(8); data[512..512 + v0.len()].copy_from_slice(&v0); - let sb = Superblock::parse(&data, 512).unwrap(); + // Addresses are relative to the superblock, so parsing in place + // (where they would be applied to the whole buffer) is refused... + assert_eq!( + Superblock::parse(&data, 512), + Err(FormatError::UserBlockNotStripped(512)) + ); + // ...and the caller parses the bytes from the signature on. + let (ub, hdf5) = crate::signature::split_user_block(&data).unwrap(); + assert_eq!(ub.len(), 512); + let sb = Superblock::parse(hdf5, 0).unwrap(); assert_eq!(sb.version, 0); assert_eq!(sb.root_group_address, 96); } diff --git a/crates/clawhdf5-format/tests/fixtures/tcompound.h5 b/crates/clawhdf5-format/tests/fixtures/tcompound.h5 new file mode 100644 index 0000000..d1ec650 Binary files /dev/null and b/crates/clawhdf5-format/tests/fixtures/tcompound.h5 differ diff --git a/crates/clawhdf5-io/src/async_read.rs b/crates/clawhdf5-io/src/async_read.rs index 94afff0..d9e2ffe 100644 --- a/crates/clawhdf5-io/src/async_read.rs +++ b/crates/clawhdf5-io/src/async_read.rs @@ -268,28 +268,26 @@ impl AsyncHDF5File { /// /// Reads the entire file into memory, then parses the superblock. pub async fn open(reader: &R) -> Result { - let data = reader.read_all().await?; - let sig_offset = find_signature(&data)?; - let superblock = Superblock::parse(&data, sig_offset)?; - Ok(Self { data, superblock }) + Self::from_bytes(reader.read_all().await?) } /// Open an HDF5 file asynchronously from a file path. pub async fn open_path>(path: P) -> Result { - let data = tokio::fs::read(path).await?; - let sig_offset = find_signature(&data)?; - let superblock = Superblock::parse(&data, sig_offset)?; - Ok(Self { data, superblock }) + Self::from_bytes(tokio::fs::read(path).await?) } /// Open an HDF5 file from bytes already in memory. - pub fn from_bytes(data: Vec) -> Result { - let sig_offset = find_signature(&data)?; - let superblock = Superblock::parse(&data, sig_offset)?; + pub fn from_bytes(mut data: Vec) -> Result { + // HDF5 addresses are relative to the superblock: drop any user block + // so they index `data` directly. + let user_block = find_signature(&data)?; + data.drain(..user_block); + let superblock = Superblock::parse(&data, 0)?; Ok(Self { data, superblock }) } - /// Access the raw file bytes. + /// Access the file bytes from the superblock on (any user block is + /// dropped on open). pub fn as_bytes(&self) -> &[u8] { &self.data } diff --git a/crates/clawhdf5-io/src/mpi_vol.rs b/crates/clawhdf5-io/src/mpi_vol.rs index a1ddf72..7dac39e 100644 --- a/crates/clawhdf5-io/src/mpi_vol.rs +++ b/crates/clawhdf5-io/src/mpi_vol.rs @@ -180,7 +180,7 @@ fn mpi_collective_read(vol: &MpiVol, location: &str, path: &str) -> Result Result { reader: R, + /// Offset of the superblock in the file (the user-block size); every + /// HDF5 address is relative to it. + base: usize, superblock: Superblock, root_header: ObjectHeader, /// Cache of parsed object headers, keyed by address. @@ -73,9 +76,9 @@ impl LazyFile { /// /// Parses only the superblock and root group object header. pub fn open(reader: R) -> Result { - let data = reader.as_bytes(); - let sig_offset = signature::find_signature(data)?; - let superblock = Superblock::parse(data, sig_offset)?; + let (user_block, data) = signature::split_user_block(reader.as_bytes())?; + let base = user_block.len(); + let superblock = Superblock::parse(data, 0)?; let root_header = ObjectHeader::parse( data, superblock.root_group_address as usize, @@ -84,15 +87,26 @@ impl LazyFile { )?; Ok(Self { reader, + base, superblock, root_header, header_cache: RefCell::new(HashMap::new()), }) } - /// Returns the raw file bytes. + /// Returns the file's bytes from the superblock on (after any user + /// block), which is the space every HDF5 address in the file indexes. pub fn as_bytes(&self) -> &[u8] { - self.reader.as_bytes() + self.hdf5_bytes() + } + + /// Size of the user block before the superblock (0 for most files). + pub fn user_block_size(&self) -> u64 { + self.base as u64 + } + + fn hdf5_bytes(&self) -> &[u8] { + &self.reader.as_bytes()[self.base..] } /// Returns a reference to the parsed superblock. @@ -110,7 +124,7 @@ impl LazyFile { /// Resolve a path and return a `LazyDataset` handle. pub fn dataset(&self, path: &str) -> Result, Error> { - let data = self.reader.as_bytes(); + let data = self.hdf5_bytes(); let addr = group_v2::resolve_path_any(data, &self.superblock, path)?; let hdr = self.get_or_parse_header(addr)?; if !has_message(&hdr, MessageType::DataLayout) { @@ -124,7 +138,7 @@ impl LazyFile { /// Resolve a path and return a `LazyGroup` handle. pub fn group(&self, path: &str) -> Result, Error> { - let data = self.reader.as_bytes(); + let data = self.hdf5_bytes(); let addr = group_v2::resolve_path_any(data, &self.superblock, path)?; Ok(LazyGroup { file: self, @@ -163,7 +177,7 @@ impl LazyFile { } // Parse and cache - let data = self.reader.as_bytes(); + let data = self.hdf5_bytes(); let hdr = ObjectHeader::parse( data, address as usize, @@ -187,7 +201,7 @@ impl LazyFile { impl std::fmt::Debug for LazyFile { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("LazyFile") - .field("size", &self.reader.as_bytes().len()) + .field("size", &self.hdf5_bytes().len()) .field("superblock_version", &self.superblock.version) .field("cached_headers", &self.header_cache.borrow().len()) .finish() @@ -234,7 +248,7 @@ impl<'f, R: HDF5Read> LazyGroup<'f, R> { /// Read all attributes of this group. pub fn attrs(&self) -> Result, Error> { let hdr = self.file.get_or_parse_header(self.address)?; - let data = self.file.reader.as_bytes(); + let data = self.file.hdf5_bytes(); let attr_msgs = extract_attributes_full(data, &hdr, self.file.offset_size(), self.file.length_size())?; Ok(attrs_to_map( @@ -277,7 +291,7 @@ impl<'f, R: HDF5Read> LazyGroup<'f, R> { fn children(&self) -> Result, Error> { let hdr = self.file.get_or_parse_header(self.address)?; - let data = self.file.reader.as_bytes(); + let data = self.file.hdf5_bytes(); let os = self.file.offset_size(); let ls = self.file.length_size(); resolve_group_entries(data, &hdr, os, ls).map_err(Error::Format) @@ -360,7 +374,7 @@ impl<'f, R: HDF5Read> LazyDataset<'f, R> { let dl = self.data_layout()?; let ds = self.dataspace()?; let dt = self.datatype()?; - let slice = data_read::read_raw_data_zerocopy(self.file.reader.as_bytes(), &dl, &ds, &dt)?; + let slice = data_read::read_raw_data_zerocopy(self.file.hdf5_bytes(), &dl, &ds, &dt)?; Ok(slice) } @@ -401,7 +415,7 @@ impl<'f, R: HDF5Read> LazyDataset<'f, R> { /// Read all attributes of this dataset. pub fn attrs(&self) -> Result, Error> { - let data = self.file.reader.as_bytes(); + let data = self.file.hdf5_bytes(); let attr_msgs = extract_attributes_full( data, &self.header, @@ -479,7 +493,7 @@ impl<'f, R: HDF5Read> LazyDataset<'f, R> { let ds = self.dataspace()?; let dl = self.data_layout()?; let pipeline = self.filter_pipeline()?; - let data = self.file.reader.as_bytes(); + let data = self.file.hdf5_bytes(); // Unallocated storage reads as the dataset's fill value. clawhdf5_format::fill_value::read_full_with_fill( &self.header.messages, diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index 9119cdf..b7251ac 100644 --- a/crates/clawhdf5/src/mmap_file.rs +++ b/crates/clawhdf5/src/mmap_file.rs @@ -34,6 +34,9 @@ use crate::types::{AttrValue, DType, attrs_to_map, classify_datatype}; /// `&[u8]` slice via [`MmapDataset::read_raw_slice`]. pub struct MmapFile { reader: MmapReader, + /// Offset of the superblock in the mapped file (the user-block size); + /// every HDF5 address is relative to it. + base: usize, superblock: Superblock, } @@ -41,10 +44,25 @@ impl MmapFile { /// Open an HDF5 file using memory-mapped I/O. pub fn open>(path: P) -> Result { let reader = MmapReader::open(path).map_err(Error::Io)?; - let data = reader.as_bytes(); - let sig_offset = signature::find_signature(data)?; - let superblock = Superblock::parse(data, sig_offset)?; - Ok(Self { reader, superblock }) + let (user_block, data) = signature::split_user_block(reader.as_bytes())?; + let base = user_block.len(); + let superblock = Superblock::parse(data, 0)?; + Ok(Self { + reader, + base, + superblock, + }) + } + + /// The file's bytes from the superblock on — the space HDF5 addresses + /// index into. + fn hdf5_bytes(&self) -> &[u8] { + &self.reader.as_bytes()[self.base..] + } + + /// Size of the user block before the superblock (0 for most files). + pub fn user_block_size(&self) -> u64 { + self.base as u64 } /// Returns a handle to the root group. @@ -57,7 +75,7 @@ impl MmapFile { /// Resolve a path and return a `MmapDataset` handle. pub fn dataset(&self, path: &str) -> Result, Error> { - let data = self.reader.as_bytes(); + let data = self.hdf5_bytes(); let addr = group_v2::resolve_path_any(data, &self.superblock, path)?; let hdr = self.parse_header(addr)?; if !has_message(&hdr, MessageType::DataLayout) { @@ -71,7 +89,7 @@ impl MmapFile { /// Resolve a path and return a `MmapGroup` handle. pub fn group(&self, path: &str) -> Result, Error> { - let data = self.reader.as_bytes(); + let data = self.hdf5_bytes(); let addr = group_v2::resolve_path_any(data, &self.superblock, path)?; Ok(MmapGroup { file: self, @@ -79,9 +97,11 @@ impl MmapFile { }) } - /// Returns the raw file bytes (zero-copy from mmap). + /// Returns the file's bytes from the superblock on (after any user + /// block), zero-copy from the mmap. Every HDF5 address in the file + /// indexes this slice. pub fn as_bytes(&self) -> &[u8] { - self.reader.as_bytes() + self.hdf5_bytes() } /// Returns a reference to the parsed superblock. @@ -91,7 +111,7 @@ impl MmapFile { fn parse_header(&self, address: u64) -> Result { ObjectHeader::parse( - self.reader.as_bytes(), + self.hdf5_bytes(), address as usize, self.superblock.offset_size, self.superblock.length_size, @@ -155,7 +175,7 @@ impl<'f> MmapGroup<'f> { /// Read all attributes of this group. pub fn attrs(&self) -> Result, Error> { - let data = self.file.reader.as_bytes(); + let data = self.file.hdf5_bytes(); let hdr = self.file.parse_header(self.address)?; let attr_msgs = extract_attributes_full(data, &hdr, self.file.offset_size(), self.file.length_size())?; @@ -198,7 +218,7 @@ impl<'f> MmapGroup<'f> { } fn children(&self) -> Result, Error> { - let data = self.file.reader.as_bytes(); + let data = self.file.hdf5_bytes(); let hdr = self.file.parse_header(self.address)?; let os = self.file.offset_size(); let ls = self.file.length_size(); @@ -326,7 +346,7 @@ impl<'f> MmapDataset<'f> { actual: sz, })); } - let data = self.file.reader.as_bytes(); + let data = self.file.hdf5_bytes(); let a = addr as usize; if a + sz > data.len() { return Err(Error::Format(FormatError::UnexpectedEof { @@ -342,7 +362,7 @@ impl<'f> MmapDataset<'f> { /// Read all attributes of this dataset. pub fn attrs(&self) -> Result, Error> { - let data = self.file.reader.as_bytes(); + let data = self.file.hdf5_bytes(); let attr_msgs = extract_attributes_full( data, &self.header, @@ -423,7 +443,7 @@ impl<'f> MmapDataset<'f> { // Unallocated storage reads as the dataset's fill value. clawhdf5_format::fill_value::read_full_with_fill( &self.header.messages, - self.file.reader.as_bytes(), + self.file.hdf5_bytes(), &dl, &ds, dt.type_size() as usize, @@ -431,7 +451,7 @@ impl<'f> MmapDataset<'f> { self.file.length_size(), || { Ok(data_read::read_raw_data_full( - self.file.reader.as_bytes(), + self.file.hdf5_bytes(), &dl, &ds, &dt, diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index bb6a8fd..b3d7338 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -31,20 +31,43 @@ use crate::types::{AttrValue, DType, attrs_to_map, classify_datatype}; // --------------------------------------------------------------------------- /// Internal storage: either an owned `Vec` or a memory-mapped region. -enum FileData { +enum Backing { Owned(Vec), #[cfg(feature = "mmap")] Mmap(clawhdf5_io::MmapReader), } -impl FileData { - fn as_bytes(&self) -> &[u8] { +impl Backing { + fn whole_file(&self) -> &[u8] { match self { - FileData::Owned(v) => v, + Backing::Owned(v) => v, #[cfg(feature = "mmap")] - FileData::Mmap(r) => r.as_bytes(), + Backing::Mmap(r) => r.as_bytes(), } } +} + +/// The file's bytes, viewed from the superblock on. A file may start with a +/// user block (the superblock at 512, 1024, …); every HDF5 address is +/// relative to the superblock, so all parsing goes through [`Self::as_bytes`]. +struct FileData { + backing: Backing, + /// Offset of the superblock in the file (the user-block size). + base: usize, +} + +impl FileData { + /// Locate the superblock and parse it. + fn new(backing: Backing) -> Result<(Self, Superblock), Error> { + let (user_block, hdf5) = signature::split_user_block(backing.whole_file())?; + let base = user_block.len(); + let superblock = Superblock::parse(hdf5, 0)?; + Ok((Self { backing, base }, superblock)) + } + + fn as_bytes(&self) -> &[u8] { + &self.backing.whole_file()[self.base..] + } fn len(&self) -> usize { self.as_bytes().len() @@ -81,11 +104,9 @@ impl File { #[cfg(feature = "mmap")] { let reader = clawhdf5_io::MmapReader::open(path).map_err(Error::Io)?; - let data_ref = reader.as_bytes(); - let sig_offset = signature::find_signature(data_ref)?; - let superblock = Superblock::parse(data_ref, sig_offset)?; + let (data, superblock) = FileData::new(Backing::Mmap(reader))?; Ok(Self { - data: FileData::Mmap(reader), + data, superblock, chunk_cache: ChunkCache::new(), base_dir, @@ -116,10 +137,9 @@ impl File { /// In-memory files have no directory, so external Virtual Dataset sources /// cannot be resolved automatically (same-file VDS still works). pub fn from_bytes(data: Vec) -> Result { - let sig_offset = signature::find_signature(&data)?; - let superblock = Superblock::parse(&data, sig_offset)?; + let (data, superblock) = FileData::new(Backing::Owned(data))?; Ok(Self { - data: FileData::Owned(data), + data, superblock, chunk_cache: ChunkCache::new(), base_dir: None, @@ -209,11 +229,19 @@ impl File { Ok(results.into_iter().map(|(_, data)| data).collect()) } - /// Returns the raw file bytes. + /// Returns the file's bytes from the superblock on (after any user + /// block). Every HDF5 address in the file indexes this slice, so it is + /// what the `clawhdf5_format` parsers expect as `file_data`. pub fn as_bytes(&self) -> &[u8] { self.data.as_bytes() } + /// Size of the user block before the superblock (0 for most files). + /// Matches h5py's `File.userblock_size`. + pub fn user_block_size(&self) -> u64 { + self.data.base as u64 + } + /// Returns a reference to the parsed superblock. pub fn superblock(&self) -> &Superblock { &self.superblock @@ -221,10 +249,10 @@ impl File { /// Returns `true` when the file is backed by memory-mapped I/O. pub fn is_mmap(&self) -> bool { - match &self.data { - FileData::Owned(_) => false, + match &self.data.backing { + Backing::Owned(_) => false, #[cfg(feature = "mmap")] - FileData::Mmap(_) => true, + Backing::Mmap(_) => true, } } diff --git a/crates/clawhdf5/tests/local_heap_interop.rs b/crates/clawhdf5/tests/local_heap_interop.rs new file mode 100644 index 0000000..02aa6e0 --- /dev/null +++ b/crates/clawhdf5/tests/local_heap_interop.rs @@ -0,0 +1,131 @@ +//! Old-style (symbol-table) groups keep link names in a local heap. libhdf5 +//! validates the heap's free list when it loads the heap and refuses the +//! group ("bad heap free list") when the list points outside the heap; we +//! must refuse too instead of listing names read from a broken heap. Like +//! libhdf5, the check happens when a name is needed, so an empty group with +//! a broken heap still lists. +//! +//! h5py writes the files; skipped when python3 with h5py is unavailable, +//! unless `CLAWHDF5_REQUIRE_INTEROP=1`. + +use std::process::Command; + +use clawhdf5::File; + +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) +} + +macro_rules! skip_if_no_python { + () => { + 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; + } + }; +} + +fn run_python(script: &str) -> String { + let output = Command::new(python()) + .args(["-c", script]) + .output() + .expect("failed to run python"); + assert!( + output.status.success(), + "python failed:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).into_owned() +} + +#[test] +fn local_heap_free_list_checked_like_libhdf5() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let good = dir.path().join("good.h5"); + // Writes `good.h5` (a deleted link leaves a real free block in the root + // group's heap) and two copies whose root heap free list is broken; for + // each prints what h5py lists, or `ERROR`. + let script = format!( + r#" +import h5py, struct +good = "{good}" +with h5py.File(good, "w", libver="earliest") as f: + for name in ("alpha", "beta", "gamma"): + f.create_group(name) + del f["beta"] +data = bytearray(open(good, "rb").read()) +heap = data.find(b"HEAP") # the root group's heap is written first +size, head, seg = struct.unpack_from(" = out.lines().collect(); + assert_eq!( + lines, + [ + "good alpha gamma", + "bad_head ERROR", + "bad_block ERROR", + "bad_empty" + ], + "h5py's view changed" + ); + + let file = File::open(&good).unwrap(); + let mut groups = file.root().groups().unwrap(); + groups.sort(); + assert_eq!(groups, ["alpha", "gamma"]); + + for name in ["bad_head", "bad_block"] { + let file = File::open(dir.path().join(format!("{name}.h5"))).unwrap(); + let listed = file.root().groups(); + assert!( + listed.is_err(), + "{name}: listed {listed:?} from a heap libhdf5 rejects" + ); + } + + let file = File::open(dir.path().join("bad_empty.h5")).unwrap(); + assert_eq!(file.root().groups().unwrap(), Vec::::new()); +} diff --git a/crates/clawhdf5/tests/shared_message_v1.rs b/crates/clawhdf5/tests/shared_message_v1.rs new file mode 100644 index 0000000..0873d03 --- /dev/null +++ b/crates/clawhdf5/tests/shared_message_v1.rs @@ -0,0 +1,121 @@ +//! Version-1 shared messages (HDF5 1.6 era). A dataset that uses a committed +//! datatype stores a *shared* datatype message pointing at the type's object +//! header. In version 1 that pointer is a 1.6 "symbol table entry": after six +//! reserved bytes comes a length-sized heap offset, *then* the address. +//! +//! Fixture: `tcompound.h5` from libhdf5's own tool tests +//! (`tools/test/testfiles/tcompound.h5`, HDF5 source tree, BSD-style +//! licence), 8 KiB. Its datasets use committed compound types through v1 +//! shared messages. The expected types are what h5dump 1.14.6 and h5py 3.16 +//! (HDF5 2.0) report; the h5py cross-check runs when python3 with h5py is +//! available (required with `CLAWHDF5_REQUIRE_INTEROP=1`). + +use std::process::Command; + +use clawhdf5::{DType, File}; + +const FIXTURE: &[u8] = include_bytes!("../../clawhdf5-format/tests/fixtures/tcompound.h5"); + +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) +} + +fn compound(fields: &[(&str, DType)]) -> DType { + DType::Compound( + fields + .iter() + .map(|(n, t)| (n.to_string(), t.clone())) + .collect(), + ) +} + +fn expected() -> Vec<(&'static str, DType)> { + let int_float = |i: &str, f: &str| compound(&[(i, DType::I32), (f, DType::F32)]); + vec![ + ("group1/dset2", int_float("int_name", "float_name")), + ( + "group1/dset3", + compound(&[ + ("int_array", DType::Array(Box::new(DType::I32), vec![4])), + ( + "float_array", + DType::Array(Box::new(DType::F32), vec![5, 6]), + ), + ]), + ), + ("group1/dset4", int_float("int", "float")), + ("group2/dset5", int_float("int", "float")), + ] +} + +#[test] +fn v1_shared_datatype_resolves_to_the_committed_type() { + // Reading the heap offset as the address used to land on the superblock + // and fail with InvalidObjectHeaderVersion. + let file = File::from_bytes(FIXTURE.to_vec()).unwrap(); + for (path, dtype) in expected() { + assert_eq!( + file.dataset(path).unwrap().dtype().unwrap(), + dtype, + "{path}" + ); + } +} + +#[test] +fn v1_shared_datatype_field_names_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; + } + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("tcompound.h5"); + std::fs::write(&path, FIXTURE).unwrap(); + let script = format!( + r#" +import h5py +with h5py.File("{path}", "r") as f: + for p in ("group1/dset2", "group1/dset3", "group1/dset4", "group2/dset5"): + print(p, *f[p].dtype.names) +"#, + path = path.display() + ); + let out = Command::new(python()) + .args(["-c", &script]) + .output() + .unwrap(); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout); + let theirs: Vec<&str> = stdout.lines().collect(); + let ours: Vec = expected() + .into_iter() + .map(|(p, t)| match t { + DType::Compound(fields) => { + let names: Vec = fields.into_iter().map(|(n, _)| n).collect(); + format!("{p} {}", names.join(" ")) + } + other => panic!("{other:?}"), + }) + .collect(); + assert_eq!(theirs, ours); +} diff --git a/crates/clawhdf5/tests/userblock_interop.rs b/crates/clawhdf5/tests/userblock_interop.rs new file mode 100644 index 0000000..c9130b9 --- /dev/null +++ b/crates/clawhdf5/tests/userblock_interop.rs @@ -0,0 +1,314 @@ +//! Files that start with a user block (`h5py.File(..., userblock_size=N)`, +//! `h5jam`): the superblock sits at 512, 1024, ... and every address in the +//! file is relative to it. Each reader (buffered, mmap, `MmapFile`, +//! `LazyFile`) must apply that base, and read the same values h5py does. +//! +//! h5py writes the files; skipped when python3 with h5py is unavailable, +//! unless `CLAWHDF5_REQUIRE_INTEROP=1`. + +use std::collections::HashMap; +use std::path::Path; +use std::process::Command; + +use clawhdf5::{AttrValue, File, LazyFile, MmapFile}; + +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) +} + +macro_rules! skip_if_no_python { + () => { + 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; + } + }; +} + +/// Run `script` and return its stdout as `key -> values` (one +/// `key v1 v2 ...` line per key). +fn run_python(script: &str) -> HashMap> { + let output = Command::new(python()) + .args(["-c", script]) + .output() + .expect("failed to run python"); + assert!( + output.status.success(), + "python failed:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout) + .lines() + .filter_map(|line| { + let mut words = line.split_whitespace().map(str::to_string); + Some((words.next()?, words.collect())) + }) + .collect() +} + +fn parse(values: &[String]) -> Vec +where + T::Err: std::fmt::Debug, +{ + values.iter().map(|v| v.parse().unwrap()).collect() +} + +/// Write a file with a user block of `userblock` bytes holding contiguous, +/// chunked (deflate), compact and committed-type datasets, nested groups, +/// and attributes (compact and, under `latest`, dense). Prints what h5py +/// reads back. +fn write_file(path: &Path, userblock: u32, libver: &str) -> HashMap> { + let script = format!( + r#" +import h5py, numpy as np +path = "{path}" +with h5py.File(path, "w", userblock_size={userblock}, libver={libver}) as f: + f.attrs["title"] = "user block" + f.attrs["answer"] = np.int64(42) + f.create_dataset("contig", data=np.arange(12, dtype=", key: &str) -> String { + match map.get(key) { + Some(AttrValue::I64(v)) => format!("i64 {v}"), + Some(AttrValue::F64(v)) => format!("f64 {v}"), + Some(AttrValue::String(v)) => format!("str {v}"), + other => format!("{other:?}"), + } +} + +fn i64s(v: &[String]) -> Vec { + parse(v) +} + +/// Everything read through the `File` API must match h5py. +fn check_file(file: &File, expected: &HashMap>, label: &str) { + let ub: u64 = expected["userblock"][0].parse().unwrap(); + assert_eq!(file.user_block_size(), ub, "{label}: user block size"); + assert_eq!( + file.dataset("contig").unwrap().read_f64().unwrap(), + parse::(&expected["contig"]), + "{label}: contiguous" + ); + assert_eq!( + file.dataset("chunked") + .unwrap() + .read_i32() + .unwrap() + .iter() + .map(|&v| v as i64) + .collect::>(), + i64s(&expected["chunked"]), + "{label}: chunked" + ); + assert_eq!( + file.dataset("compact").unwrap().read_i64().unwrap(), + i64s(&expected["compact"]), + "{label}: compact" + ); + assert_eq!( + file.dataset("committed").unwrap().read_f32().unwrap(), + parse::(&expected["committed"]), + "{label}: committed datatype" + ); + assert_eq!( + file.dataset("a/b/deep").unwrap().read_i64().unwrap(), + i64s(&expected["deep"]), + "{label}: nested group" + ); + let many: Vec = (0..20) + .map(|i| { + file.dataset(&format!("many/d{i:02}")) + .unwrap() + .read_i32() + .unwrap()[0] as i64 + }) + .collect(); + assert_eq!(many, i64s(&expected["many"]), "{label}: many links"); + + let root = file.root().attrs().unwrap(); + assert_eq!(attr(&root, "title"), "str user block", "{label}"); + assert_eq!(attr(&root, "answer"), "i64 42", "{label}"); + let a = file.group("a").unwrap().attrs().unwrap(); + let k: Vec = (0..12) + .map(|i| match &a[&format!("k{i:02}")] { + AttrValue::I64(v) => *v, + _ => panic!("{label}: k{i:02} is not an i64"), + }) + .collect(); + assert_eq!(k, i64s(&expected["k"]), "{label}: attributes"); + assert_eq!( + attr(&file.group("a/b").unwrap().attrs().unwrap(), "scale"), + "f64 2.5", + "{label}" + ); + assert_eq!( + attr(&file.dataset("contig").unwrap().attrs().unwrap(), "units"), + "str m", + "{label}" + ); +} + +fn check_all_readers(path: &Path, expected: &HashMap>, label: &str) { + check_file( + &File::open(path).unwrap(), + expected, + &format!("{label} File::open"), + ); + check_file( + &File::open_buffered(path).unwrap(), + expected, + &format!("{label} File::open_buffered"), + ); + check_file( + &File::from_bytes(std::fs::read(path).unwrap()).unwrap(), + expected, + &format!("{label} File::from_bytes"), + ); + + let ub: u64 = expected["userblock"][0].parse().unwrap(); + + let mm = MmapFile::open(path).unwrap(); + assert_eq!(mm.user_block_size(), ub, "{label} MmapFile"); + assert_eq!( + mm.dataset("contig").unwrap().read_f64().unwrap(), + parse::(&expected["contig"]), + "{label} MmapFile contiguous" + ); + assert_eq!( + mm.dataset("compact").unwrap().read_i64().unwrap(), + i64s(&expected["compact"]), + "{label} MmapFile compact" + ); + assert_eq!( + mm.dataset("committed").unwrap().read_f32().unwrap(), + parse::(&expected["committed"]), + "{label} MmapFile committed" + ); + assert_eq!( + mm.dataset("a/b/deep").unwrap().read_i64().unwrap(), + i64s(&expected["deep"]), + "{label} MmapFile nested" + ); + assert_eq!( + attr(&mm.root().attrs().unwrap(), "answer"), + "i64 42", + "{label} MmapFile attrs" + ); + + let lazy = LazyFile::open_mmap(path).unwrap(); + assert_eq!(lazy.user_block_size(), ub, "{label} LazyFile"); + assert_eq!( + lazy.dataset("contig").unwrap().read_f64().unwrap(), + parse::(&expected["contig"]), + "{label} LazyFile contiguous" + ); + assert_eq!( + lazy.dataset("chunked") + .unwrap() + .read_i32() + .unwrap() + .iter() + .map(|&v| v as i64) + .collect::>(), + i64s(&expected["chunked"]), + "{label} LazyFile chunked" + ); + assert_eq!( + lazy.dataset("committed").unwrap().read_f32().unwrap(), + parse::(&expected["committed"]), + "{label} LazyFile committed" + ); + assert_eq!( + lazy.dataset("a/b/deep").unwrap().read_i64().unwrap(), + i64s(&expected["deep"]), + "{label} LazyFile nested" + ); + assert_eq!( + attr(&lazy.root().attrs().unwrap(), "answer"), + "i64 42", + "{label} LazyFile attrs" + ); +} + +#[test] +fn user_block_files_read_like_h5py() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + for userblock in [512u32, 4096] { + for libver in ["default", "latest"] { + let label = format!("userblock={userblock} libver={libver}"); + let path = dir.path().join(format!("ub_{userblock}_{libver}.h5")); + let expected = write_file(&path, userblock, libver); + assert_eq!(expected["userblock"], [userblock.to_string()], "{label}"); + check_all_readers(&path, &expected, &label); + } + } +} + +#[test] +fn file_without_user_block_reports_zero() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("no_ub.h5"); + let expected = write_file(&path, 0, "default"); + assert_eq!(expected["userblock"], ["0"]); + check_all_readers(&path, &expected, "userblock=0"); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index 6010210..2948582 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -89,13 +89,23 @@ the VDS item, which is marked. - `%b` printf-style source names are not expanded. - Hyperslab selection versions 1 and 2 are refused. - **Files with a user block:** the base address is not applied. + **Fixed 2026-09-25:** every reader views the file from the superblock on + (`twithub.h5`, `twithub513.h5`, `h5clear_fsm_persist_user_*.h5`; the + `twithub` files still stop at the user-defined link type below). - **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. + **Fixed 2026-09-25:** the address follows the length-sized link-name + offset of the embedded symbol table entry (`tcompound.h5`, `tcompound2.h5`). +- **Array members of version-1 compound datatypes** (found while fixing the + items above) were read as one element — wrong data. **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. - Soft links are left out of `datasets()`. + - **Wrong data (found while fixing user blocks):** an old-style group whose + local-heap free list points outside the heap listed garbage names where + libhdf5 refuses the heap. **Fixed 2026-09-25** + (`InvalidLocalHeapFreeList`, checked when a name is first read, as + libhdf5 does). - **Dense attributes:** a large attribute stored as a fractal-heap "huge" object makes every attribute on the object fail. This affects real NetCDF files (`issue671.nc`).