From a6e90f3ee30b8b7a39000814b4a54688280ca058 Mon Sep 17 00:00:00 2001 From: osobh Date: Fri, 25 Sep 2026 22:02:47 -0500 Subject: [PATCH] fix(format): apply the base address of files with a user block A file may start with a user block (h5py userblock_size, h5jam), putting the superblock at 512, 1024, ...; every address in the file is then relative to the superblock. The signature search found it, but every reader passed the whole file to the parsers, so addresses landed userblock bytes early and the root group failed with InvalidObjectHeaderVersion (twithub.h5, twithub513.h5, h5clear_fsm_persist_user_*.h5). Readers now view the file from the superblock on, taking the signature's position as the base address as libhdf5 does: File (mmap, buffered, from_bytes), MmapFile, LazyFile, AsyncHDF5File, the VOL and MPI VOL readers, the HNSW loader and external VDS source files. File, MmapFile and LazyFile gain user_block_size(). The new signature::split_user_block returns the two parts, and Superblock::parse refuses a non-zero offset (UserBlockNotStripped) so a format-level caller cannot silently apply superblock-relative addresses to the whole file. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 13 + crates/clawhdf5-ann/src/hnsw.rs | 7 +- crates/clawhdf5-format/src/data_read.rs | 8 +- crates/clawhdf5-format/src/error.rs | 10 + crates/clawhdf5-format/src/lib.rs | 11 +- crates/clawhdf5-format/src/shared_message.rs | 4 +- crates/clawhdf5-format/src/signature.rs | 36 +++ crates/clawhdf5-format/src/superblock.rs | 23 +- crates/clawhdf5-io/src/async_read.rs | 22 +- crates/clawhdf5-io/src/mpi_vol.rs | 9 +- crates/clawhdf5-io/src/vol.rs | 7 +- crates/clawhdf5/src/lazy.rs | 42 ++- crates/clawhdf5/src/mmap_file.rs | 50 ++- crates/clawhdf5/src/reader.rs | 60 +++- crates/clawhdf5/tests/userblock_interop.rs | 314 +++++++++++++++++++ docs/known-issues.md | 3 + 16 files changed, 540 insertions(+), 79 deletions(-) create mode 100644 crates/clawhdf5/tests/userblock_interop.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b054b1..5a5283b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -273,6 +273,19 @@ - CI keeps zlib-ng building and tested; the arm64 job no longer needs cmake. ### Correctness +- **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 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/error.rs b/crates/clawhdf5-format/src/error.rs index 7ae57f4..a54ca72 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -120,6 +120,11 @@ pub enum FormatError { /// 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), @@ -342,6 +347,11 @@ 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 \ 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/shared_message.rs b/crates/clawhdf5-format/src/shared_message.rs index a77edc1..f760021 100644 --- a/crates/clawhdf5-format/src/shared_message.rs +++ b/crates/clawhdf5-format/src/shared_message.rs @@ -408,8 +408,8 @@ pub fn load_sohm_table( offset_size: u8, length_size: u8, ) -> Result, FormatError> { - let sig = crate::signature::find_signature(file_data)?; - let sb = crate::superblock::Superblock::parse(file_data, sig)?; + // `file_data` starts at the superblock (see `signature::split_user_block`). + let sb = crate::superblock::Superblock::parse(file_data, 0)?; let Some(ext_addr) = sb .superblock_extension_address .filter(|&a| !is_undefined(a, offset_size)) 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-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/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 0783e91..6f84c6a 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -73,6 +73,9 @@ 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 a length-sized heap offset (`tcompound.h5`, `tcompound2.h5`; their datasets now stop at the layout