From c7092722aacbfa66b1266a428280051bea7e06b2 Mon Sep 17 00:00:00 2001 From: osobh Date: Fri, 25 Sep 2026 21:54:20 -0500 Subject: [PATCH] fix(format): locate the address in version-1 shared messages A version-1 shared message reference is version, type, six reserved bytes and then an old-style symbol table entry: link-name offset (length size), object header address, cache type, reserved, scratch. We read the address straight after the reserved bytes, i.e. the link-name offset, and the committed datatype lookup failed with InvalidObjectHeaderVersion (the bytes checked in tcompound.h5: name offset 0x10, then 0x590 = /type1). Datasets of 1.4/1.6-era files that use a committed datatype were unreadable. Skip the name offset. parse_shared_ref has no length size, so add parse_shared_ref_sized and use it in every internal caller; parse_shared_ref keeps its signature and assumes length size == offset size. The old parse_v1_ref unit test encoded the wrong layout and now uses the real bytes. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 8 ++ crates/clawhdf5-format/src/attribute.rs | 5 +- crates/clawhdf5-format/src/shared_message.rs | 49 +++++++++--- .../tests/fixtures/legacy/README.md | 1 + .../tests/fixtures/legacy/tcompound.h5 | Bin 0 -> 8192 bytes .../clawhdf5/tests/legacy_format_interop.rs | 75 ++++++++++++++++++ docs/known-issues.md | 2 + 7 files changed, 127 insertions(+), 13 deletions(-) create mode 100644 crates/clawhdf5-format/tests/fixtures/legacy/tcompound.h5 diff --git a/CHANGELOG.md b/CHANGELOG.md index bb55b44..2229e45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -305,6 +305,14 @@ the member's offset; they are now array members, as in libhdf5 (`tarrold.h5`, `tcompound.h5`). Only reachable once layout versions 1/2 were readable, since the files that use it are that old. +- `clawhdf5-format` reader — errors on valid files: a version-1 shared + message (a committed datatype in HDF5 1.4/1.6-era files) was read as if the + object header address followed the reserved bytes; it follows a link-name + offset (the reference is an old-style symbol table entry), so the reader + followed the name offset and failed with `InvalidObjectHeaderVersion` + (`tcompound.h5`). New `shared_message::parse_shared_ref_sized` takes the + superblock's length size; `parse_shared_ref` assumes it equals the offset + size. - `clawhdf5-format` reader — errors on valid files: enum and bool datasets through the numeric readers; the "don't filter partial edge chunks" layout flag; Fletcher32 ahead of deflate (NetCDF-4's order). Unknown-message flags diff --git a/crates/clawhdf5-format/src/attribute.rs b/crates/clawhdf5-format/src/attribute.rs index bb96bd5..36ded53 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_sized(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_sized(&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/shared_message.rs b/crates/clawhdf5-format/src/shared_message.rs index 33334fa..9ab5d0f 100644 --- a/crates/clawhdf5-format/src/shared_message.rs +++ b/crates/clawhdf5-format/src/shared_message.rs @@ -154,13 +154,29 @@ 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. +/// +/// Assumes the file's length size equals its offset size, which only matters +/// for version-1 references; use [`parse_shared_ref_sized`] when the +/// superblock's length size is known. pub fn parse_shared_ref(data: &[u8], offset_size: u8) -> Result { + parse_shared_ref_sized(data, offset_size, offset_size) +} + +/// [`parse_shared_ref`] with the superblock's length size, which locates the +/// object header address in a version-1 reference. +pub fn parse_shared_ref_sized( + 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 an old-style symbol table + // entry: link-name offset(length_size), object header address, + // cache type(4), reserved(4), scratch(16) — always "committed" // v2: version, type, address — always "committed" // v3: version, type, then a fractal-heap ID if type == SOHM, otherwise // an address @@ -177,7 +193,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 +450,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_sized(&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 +530,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_sized(&msg.data, offset_size, length_size)?; resolve_shared_message( file_data, &shared_ref, @@ -649,15 +665,26 @@ mod tests { #[test] fn parse_v1_ref() { - let mut data = Vec::new(); - data.push(1); // version - data.push(0); // type - data.extend_from_slice(&[0u8; 6]); // reserved - data.extend_from_slice(&0x5678u64.to_le_bytes()); + // Datatype message of `/group1/dset2` in HDF5's `tcompound.h5` + // (written in 2000): version 1, six reserved bytes, then an old-style + // symbol table entry — link-name offset 0x10, object header address + // 0x590 (the committed datatype `/type1`), cache type, reserved and + // scratch. + let mut data = vec![1, 0, 0, 0, 0, 0, 0, 0]; + data.extend_from_slice(&0x10u64.to_le_bytes()); + data.extend_from_slice(&0x590u64.to_le_bytes()); + data.extend_from_slice(&[0; 24]); - let shared = parse_shared_ref(&data, 8).unwrap(); + let shared = parse_shared_ref_sized(&data, 8, 8).unwrap(); assert_eq!(shared.version, 1); - assert_eq!(shared.object_header_address, Some(0x5678)); + assert_eq!(shared.object_header_address, Some(0x590)); + + // The name offset is a length: 4 bytes here, then an 8-byte address. + let mut data = vec![1, 0, 0, 0, 0, 0, 0, 0]; + data.extend_from_slice(&0x10u32.to_le_bytes()); + data.extend_from_slice(&0x590u64.to_le_bytes()); + let shared = parse_shared_ref_sized(&data, 8, 4).unwrap(); + assert_eq!(shared.object_header_address, Some(0x590)); } #[test] diff --git a/crates/clawhdf5-format/tests/fixtures/legacy/README.md b/crates/clawhdf5-format/tests/fixtures/legacy/README.md index a574759..0eda271 100644 --- a/crates/clawhdf5-format/tests/fixtures/legacy/README.md +++ b/crates/clawhdf5-format/tests/fixtures/legacy/README.md @@ -10,3 +10,4 @@ write these structures, so they are kept as files. | `deflate.h5` | `test/testfiles/deflate.h5` | Data Layout message v1, chunked + deflate (v1 B-tree index) | | `h5ex_g_iterate.h5` | `HDF5Examples/C/H5G/h5ex_g_iterate.h5` | Data Layout message v2, contiguous; an unallocated dataset | | `tarrold.h5` | `test/testfiles/tarrold.h5` | Compound datatype v1 members with legacy array dimensions | +| `tcompound.h5` | `tools/test/testfiles/tcompound.h5` | Version-1 shared messages (committed datatypes); compound v1 array members with data | diff --git a/crates/clawhdf5-format/tests/fixtures/legacy/tcompound.h5 b/crates/clawhdf5-format/tests/fixtures/legacy/tcompound.h5 new file mode 100644 index 0000000000000000000000000000000000000000..d1ec6504cafee27eeda2e6ea97f95bd9bfc8c97d GIT binary patch literal 8192 zcmeHMJ8u&~5T5g0f(Z{f1xa`x9i>SEazlJ45tIT!#G}Rr!b5^23PFMjmmpE31giXu zl(a}eN`pk{G9@J)d~-8%an84P3`B{*Bkj)Y?CkB_?9A-m-rJcgSC0&x7$SyZkpe1_ zpERWUsX$?-tuku`B^+pGI-cdOn)a6!ubooDfo|WNo+k3h<~MBOGl5W{G5YwwvVcbg zcn6tV(lGp%+wav1HN}QJ8ch17BKY`PLXN=MOAxBxov%NeGif(29VEmELrC{@jJl$8 z(D1pl>6pKf|~<^&kLfp<3gC+`%!7v6!RJ~tmrstwb!AtHYMY=3+yq+gJ-ho zBGtpzc~;kXCDkuXsZK;T|C&9U`aIXzZuxf=all~fD6M||zgWPPf3xvx_Tb!*#I6Rg zPvzvCVe!1v`1Lfuc{@K6@@3NqlO`Yn%>)U#+R?iKc`@7BLy@o!PkzYfK(eevs=!!jX{=do{e{;iXD%GgbUmEkw>`79BEh*0r4 zB3ecHghD8&w(*+zyqIMhD61%+P?|){<8w$JFAmYk9}TJla1f!|&H1u=#Ub=7W6~BK zKo8{U@H_A4ny=M1aVR>(5oYG2t{ho8+d(t zK>&|)4xqqi8i3C^1rTtqwEGvNwFJygvuBmzK;7 Vec<(i32, f32)> { + file.dataset(name) + .unwrap() + .read_selection(&Selection::All) + .unwrap() + .as_chunks::<8>() + .0 + .iter() + .map(|b| { + ( + i32::from_be_bytes(b[..4].try_into().unwrap()), + f32::from_be_bytes(b[4..].try_into().unwrap()), + ) + }) + .collect() + }; + assert_eq!( + be_pairs("group1/dset2"), + [(0, 0.0), (1, 1.1), (2, 2.2), (3, 3.3), (4, 4.4)] + ); + assert_eq!( + be_pairs("group2/dset5"), + [(0, 0.0), (1, 0.1), (2, 0.2), (3, 0.3), (4, 0.4)] + ); + + // `/type2`: { int_array: i32[4], float_array: f32[5][6] }, whose array + // members are compound v1 legacy dimensions. + let dset3 = file.dataset("group1/dset3").unwrap(); + assert_eq!( + dset3.dtype().unwrap(), + DType::Compound(vec![ + ( + "int_array".into(), + DType::Array(Box::new(DType::I32), vec![4]) + ), + ( + "float_array".into(), + DType::Array(Box::new(DType::F32), vec![5, 6]) + ), + ]) + ); + let raw = dset3.read_selection(&Selection::All).unwrap(); + assert_eq!(raw.len(), 3 * 6 * (16 + 120)); + assert_eq!( + &raw[..16], + &[0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 2, 0, 0, 0, 3] + ); + let first: Vec = raw[16..16 + 120] + .as_chunks::<4>() + .0 + .iter() + .map(|b| f32::from_be_bytes(*b)) + .collect(); + let expected: Vec = (0..5) + .flat_map(|i| (0..6).map(move |j| (1 + i + j) as f32)) + .collect(); + assert_eq!(first, expected); +} + /// Every dataset in every fixture, byte for byte against h5py. #[test] fn legacy_fixtures_match_h5py() { @@ -111,6 +176,16 @@ fn legacy_fixtures_match_h5py() { ("deflate.h5", &["Dataset1"][..]), ("h5ex_g_iterate.h5", &["DS1", "G1/DS2"][..]), ("tarrold.h5", &["Dataset1", "Dataset2"][..]), + ( + "tcompound.h5", + &[ + "dset1", + "group1/dset2", + "group1/dset3", + "group1/dset4", + "group2/dset5", + ][..], + ), ] { let path = format!("{FIXTURES}/{name}"); let script = format!( diff --git a/docs/known-issues.md b/docs/known-issues.md index da70ab8..e0a1cae 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -80,6 +80,8 @@ the VDS item, which is marked. - Hyperslab selection versions 1 and 2 are refused. - **Files with a user block:** the base address is not applied. - **Old-style shared messages (version 1)** read the wrong address. + **Fixed 2026-09-25:** the address follows the link-name offset of the + embedded symbol table entry. - **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.