diff --git a/CHANGELOG.md b/CHANGELOG.md index ac26b3b..6b054b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -273,6 +273,15 @@ - CI keeps zlib-ng building and tested; the arm64 job no longer needs cmake. ### Correctness +- `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. diff --git a/crates/clawhdf5-format/src/attribute.rs b/crates/clawhdf5-format/src/attribute.rs index bb96bd5..06132bb 100644 --- a/crates/clawhdf5-format/src/attribute.rs +++ b/crates/clawhdf5-format/src/attribute.rs @@ -97,7 +97,7 @@ impl AttributeMessage { return Ok(Cow::Borrowed(bytes)); } let (file_data, offset_size) = file.ok_or(FormatError::UnresolvedSharedMessage)?; - let shared_ref = shared_message::parse_shared_ref(bytes, offset_size)?; + let shared_ref = shared_message::parse_shared_ref(bytes, offset_size, length_size)?; shared_message::resolve_shared_message( file_data, &shared_ref, @@ -407,7 +407,8 @@ pub fn extract_attributes_full( if msg.msg_type == MessageType::Attribute { if shared_message::is_shared(msg.flags) { // Shared attribute: resolve the reference to get actual attribute data - let shared_ref = shared_message::parse_shared_ref(&msg.data, offset_size)?; + let shared_ref = + shared_message::parse_shared_ref(&msg.data, offset_size, length_size)?; let resolved_data = shared_message::resolve_shared_message( file_data, &shared_ref, diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index 6d10c3d..7ae57f4 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -117,6 +117,9 @@ 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 selection does not fit the dataset it was applied to (wrong rank, or /// it reaches past a dimension's extent). SelectionOutOfBounds(String), @@ -339,6 +342,11 @@ impl fmt::Display for FormatError { FormatError::SelectionOutOfBounds(msg) => { write!(f, "selection out of bounds: {msg}") } + 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/shared_message.rs b/crates/clawhdf5-format/src/shared_message.rs index 33334fa..a77edc1 100644 --- a/crates/clawhdf5-format/src/shared_message.rs +++ b/crates/clawhdf5-format/src/shared_message.rs @@ -154,13 +154,24 @@ pub fn is_shared(msg_flags: u8) -> bool { /// /// When the shared flag is set on a message, the data contains a reference /// instead of the actual message content. -pub fn parse_shared_ref(data: &[u8], offset_size: u8) -> Result { +/// +/// `length_size` is needed for version 1 references, which embed a +/// symbol-table-entry-shaped pointer whose first field is a length-sized +/// heap offset. +pub fn parse_shared_ref( + data: &[u8], + offset_size: u8, + length_size: u8, +) -> Result { ensure_len(data, 0, 2)?; let version = data[0]; let ref_type = data[1]; // Layouts (HDF5 spec IV.A.2 "Shared Message", and libhdf5's decoder): - // v1: version, type, reserved(6), address — always "committed" + // v1: version, type, reserved(6), then the HDF5 1.6 "symbol table + // entry" encoding of the target: a length-sized local-heap name + // offset (unused, skipped by libhdf5) followed by the object + // header address — always "committed" // v2: version, type, address — always "committed" // v3: version, type, then a fractal-heap ID if type == SOHM, otherwise // an address @@ -177,7 +188,7 @@ pub fn parse_shared_ref(data: &[u8], offset_size: u8) -> Result address_at(2 + 6), + 1 => address_at(2 + 6 + length_size as usize), 2 => address_at(2), 3 if ref_type == SHARE_TYPE_SOHM => { ensure_len(data, 2, FHEAP_ID_LEN)?; @@ -434,7 +445,7 @@ pub fn message_data_with_sohm<'a>( if !is_shared(msg.flags) { return Ok(Cow::Borrowed(&msg.data)); } - let shared_ref = parse_shared_ref(&msg.data, offset_size)?; + let shared_ref = parse_shared_ref(&msg.data, offset_size, length_size)?; let table = if shared_ref.heap_id.is_some() { load_sohm_table(file_data, offset_size, length_size)? } else { @@ -514,7 +525,7 @@ pub fn message_data<'a>( if !is_shared(msg.flags) { return Ok(Cow::Borrowed(&msg.data)); } - let shared_ref = parse_shared_ref(&msg.data, offset_size)?; + let shared_ref = parse_shared_ref(&msg.data, offset_size, length_size)?; resolve_shared_message( file_data, &shared_ref, @@ -571,24 +582,13 @@ pub fn resolve_shared_message_with_sohm( return Ok(msg.data.clone()); } } - // The message at that OH address is the message itself - // In many cases with type 1, the entire OH at that address IS the shared message - // Try returning the first message of any type that isn't Nil - for msg in &target_header.messages { - if msg.msg_type == target_msg_type { - return Ok(msg.data.clone()); - } - } - // Fall back to first non-nil message - for msg in &target_header.messages { - if msg.msg_type != MessageType::Nil { - return Ok(msg.data.clone()); - } - } - Err(FormatError::UnexpectedEof { - expected: 1, - available: 0, - }) + // The referenced header has no (unshared) message of the wanted + // type: the reference is wrong or the file is damaged. Handing + // back some other message's bytes — or another reference's — + // would decode as garbage, so refuse. + Err(FormatError::SharedMessageTargetMissing( + target_msg_type.to_u16(), + )) } (None, Some(heap_id)) => { let table = sohm_table.ok_or(FormatError::InvalidSharedMessageVersion(2))?; @@ -627,7 +627,7 @@ mod tests { data.push(SHARE_TYPE_COMMITTED); // message lives in another object header data.extend_from_slice(&0x1234u64.to_le_bytes()); // address - let shared = parse_shared_ref(&data, 8).unwrap(); + let shared = parse_shared_ref(&data, 8, 8).unwrap(); assert_eq!(shared.version, 3); assert_eq!(shared.ref_type, SHARE_TYPE_COMMITTED); assert_eq!(shared.object_header_address, Some(0x1234)); @@ -641,7 +641,7 @@ mod tests { data.push(SHARE_TYPE_HERE); // stored here but sharable: an address data.extend_from_slice(&0xABCDu64.to_le_bytes()); - let shared = parse_shared_ref(&data, 8).unwrap(); + let shared = parse_shared_ref(&data, 8, 8).unwrap(); assert_eq!(shared.version, 3); assert_eq!(shared.ref_type, 3); assert_eq!(shared.object_header_address, Some(0xABCD)); @@ -653,9 +653,10 @@ mod tests { data.push(1); // version data.push(0); // type data.extend_from_slice(&[0u8; 6]); // reserved + data.extend_from_slice(&0x10u64.to_le_bytes()); // heap name offset data.extend_from_slice(&0x5678u64.to_le_bytes()); - let shared = parse_shared_ref(&data, 8).unwrap(); + let shared = parse_shared_ref(&data, 8, 8).unwrap(); assert_eq!(shared.version, 1); assert_eq!(shared.object_header_address, Some(0x5678)); } @@ -668,7 +669,7 @@ mod tests { data.push(SHARE_TYPE_COMMITTED); data.extend_from_slice(&0x9000u32.to_le_bytes()); - let shared = parse_shared_ref(&data, 4).unwrap(); + let shared = parse_shared_ref(&data, 4, 4).unwrap(); assert_eq!(shared.version, 2); assert_eq!(shared.object_header_address, Some(0x9000)); } @@ -679,7 +680,7 @@ mod tests { // as written by h5py 3.16 / HDF5 2.0 (libver='latest'): header flags // 0x03 (shared), payload `02 02 <8-byte object header address>`. let data = [0x02, 0x02, 0xb3, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; - let shared = parse_shared_ref(&data, 8).unwrap(); + let shared = parse_shared_ref(&data, 8, 8).unwrap(); assert_eq!(shared.object_header_address, Some(0xb3)); assert!(shared.heap_id.is_none()); } @@ -691,7 +692,7 @@ mod tests { data.push(SHARE_TYPE_SOHM); // message lives in the SOHM fractal heap data.extend_from_slice(&[0xAA, 0xBB, 0xCC, 0xDD, 0x11, 0x22, 0x33, 0x44]); - let shared = parse_shared_ref(&data, 8).unwrap(); + let shared = parse_shared_ref(&data, 8, 8).unwrap(); assert_eq!(shared.version, 3); assert_eq!(shared.ref_type, SHARE_TYPE_SOHM); assert_eq!(shared.object_header_address, None); @@ -708,21 +709,21 @@ mod tests { data.push(SHARE_TYPE_SOHM); data.extend_from_slice(&[0xAA, 0xBB]); // only 2 bytes, need 8 - let err = parse_shared_ref(&data, 8).unwrap_err(); + let err = parse_shared_ref(&data, 8, 8).unwrap_err(); assert!(matches!(err, FormatError::UnexpectedEof { .. })); } #[test] fn invalid_version() { let data = vec![99, 0]; - let err = parse_shared_ref(&data, 8).unwrap_err(); + let err = parse_shared_ref(&data, 8, 8).unwrap_err(); assert_eq!(err, FormatError::InvalidSharedMessageVersion(99)); } #[test] fn truncated_data() { let data = vec![3u8]; // too short - let err = parse_shared_ref(&data, 8).unwrap_err(); + let err = parse_shared_ref(&data, 8, 8).unwrap_err(); assert!(matches!(err, FormatError::UnexpectedEof { .. })); } @@ -733,7 +734,7 @@ mod tests { data.push(SHARE_TYPE_COMMITTED); data.extend_from_slice(&0x1000u32.to_le_bytes()); - let shared = parse_shared_ref(&data, 4).unwrap(); + let shared = parse_shared_ref(&data, 4, 4).unwrap(); assert_eq!(shared.object_header_address, Some(0x1000)); } 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/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/docs/known-issues.md b/docs/known-issues.md index 660dab0..0783e91 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -74,6 +74,9 @@ the VDS item, which is marked. - Hyperslab selection versions 1 and 2 are refused. - **Files with a user block:** the base address is not applied. - **Old-style shared messages (version 1)** read the wrong address. + **Fixed 2026-09-25:** the address follows a length-sized heap offset + (`tcompound.h5`, `tcompound2.h5`; their datasets now stop at the layout + v1 gap above). - **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:**