From 2c6c6c176e0e504ed7a91a11a98ee704ad4bb89f Mon Sep 17 00:00:00 2001 From: osobh Date: Fri, 25 Sep 2026 21:59:15 -0500 Subject: [PATCH] fix(format): decode the version-1 VDS mapping list HDF5 2.0 writes With a 2.0 low version bound, libhdf5 stores the VDS mapping list as heap block version 1: every entry starts with a flags byte (0x04 same file, no file name; 0x01/0x02 file/dataset name shared with an earlier entry, whose index is stored in place of the name). The parser treated only a leading 0x04 byte as special, so a 0x00 flags byte read as an empty (same-file) name and shared names were read as garbage. Decode it as H5D__virtual_load_layout does, refusing unknown flags, forward references and block versions above 1. Test: vds_interop::vds_mapping_block_version1_shared_names (h5py libver=("v200","v200") with repeated long names; failed before with "unknown dataspace selection type") plus the exact heap block as a unit test. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 4 + crates/clawhdf5-format/src/data_layout.rs | 132 ++++++++++++++++++++-- crates/clawhdf5/tests/vds_interop.rs | 36 ++++++ docs/known-issues.md | 2 + 4 files changed, 162 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a349fba..fd8624f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -282,6 +282,10 @@ selection. Both are decoded now, as are irregular hyperslabs (a union of blocks, read in row-major order as libhdf5 iterates them). `SerializedSelection` exposes the raw form, including unlimited counts. + - The version-1 mapping list HDF5 2.0 writes (low version bound 2.0) was + misparsed: each entry's flags byte was read as the start of the source + file name, and names shared with an earlier entry (stored as that entry's + index) were not followed. Now decoded as `H5D__virtual_load_layout` does. - `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-format/src/data_layout.rs b/crates/clawhdf5-format/src/data_layout.rs index 8429c5d..b58843c 100644 --- a/crates/clawhdf5-format/src/data_layout.rs +++ b/crates/clawhdf5-format/src/data_layout.rs @@ -72,21 +72,33 @@ pub enum DataLayout { }, } +/// Version-1 VDS mapping flag: the source file name is stored by an earlier +/// entry, whose index follows in place of the name. +const VDS_SOURCE_FILE_SHARED: u8 = 0x01; +/// Version-1 VDS mapping flag: likewise for the source dataset name. +const VDS_SOURCE_DSET_SHARED: u8 = 0x02; +/// Version-1 VDS mapping flag: the source is in the virtual file itself +/// (`"."`); no file name is stored. +const VDS_SOURCE_SAME_FILE: u8 = 0x04; +const VDS_ALL_FLAGS: u8 = VDS_SOURCE_FILE_SHARED | VDS_SOURCE_DSET_SHARED | VDS_SOURCE_SAME_FILE; + /// Parse VDS mappings from global-heap object data. /// /// The global-heap block holding a VDS mapping list is laid out as -/// (reverse-engineered and validated against HDF5 2.0): +/// (`H5D__virtual_store_layout` / `H5D__virtual_load_layout` in libhdf5): /// /// ```text /// version(1) · nused(length_size, LE) · entry[nused] · checksum(4) /// ``` /// /// Each entry is: -/// - source file name — a null-terminated string in **block version 0**; in -/// **block version 1** a same-file reference is encoded as a single `0x04` -/// marker byte (the source file is the virtual file itself) in place of the -/// name; -/// - source dataset name (null-terminated string); +/// - **block version 1 only:** a flags byte. `0x04`: the source is in the +/// virtual file itself and no file name is stored; `0x01`/`0x02`: the +/// source file/dataset name is that of an earlier entry, whose index +/// (`length_size` bytes) is stored instead of the name. libhdf5 2.0 writes +/// version 1 when the file's low version bound is 2.0 and it saves space; +/// - source file name (null-terminated string, unless flagged above); +/// - source dataset name (null-terminated string, unless flagged above); /// - source selection (serialized `H5S` dataspace selection — self-describing /// in length); /// - virtual selection (serialized `H5S` dataspace selection). @@ -112,7 +124,7 @@ pub fn parse_vds_mappings( // `nused` is untrusted; don't pre-allocate from it. Each entry consumes at // least a few bytes, so the loop is naturally bounded by the heap data and // a bogus `nused` simply errors out on the first short read. - let mut mappings = Vec::new(); + let mut mappings: Vec = Vec::new(); // Reads one self-describing selection at `pos`, returning its raw bytes and // advancing past it — bounds-checked so a corrupt selection can't overrun. let read_selection = |heap_data: &[u8], pos: &mut usize| -> Result, FormatError> { @@ -132,17 +144,57 @@ pub fn parse_vds_mappings( Ok(bytes) }; - for _ in 0..nused { - // Source file name (with the version-1 same-file marker handled). - let source_file = if version >= 1 && heap_data.get(pos) == Some(&0x04) { + if version > 1 { + return Err(FormatError::ChunkedReadError( + "unsupported VDS mapping block version".into(), + )); + } + for i in 0..nused { + // Version 1 prefixes each entry with a flags byte; a name may then be + // omitted (same file) or replaced by the index of an earlier entry + // holding the same name (`H5D__virtual_load_layout`). + let flags = if version >= 1 { + let f = *heap_data.get(pos).ok_or(FormatError::UnexpectedEof { + expected: pos + 1, + available: heap_data.len(), + })?; pos += 1; + if f & !VDS_ALL_FLAGS != 0 { + return Err(FormatError::ChunkedReadError( + "unknown VDS mapping flags".into(), + )); + } + f + } else { + 0 + }; + // Index of an earlier entry, for a shared name. + let earlier = |pos: &mut usize| -> Result { + let idx = read_length(heap_data, *pos, length_size)?; + *pos += ls; + if idx >= i { + return Err(FormatError::ChunkedReadError( + "VDS mapping shares a name with a later entry".into(), + )); + } + Ok(idx as usize) + }; + + let source_file = if flags & VDS_SOURCE_SAME_FILE != 0 { String::from(".") + } else if flags & VDS_SOURCE_FILE_SHARED != 0 { + let idx = earlier(&mut pos)?; + mappings[idx].source_file.clone() } else { read_null_terminated_string(heap_data, &mut pos)? }; - // Source dataset name. - let source_dataset = read_null_terminated_string(heap_data, &mut pos)?; + let source_dataset = if flags & VDS_SOURCE_DSET_SHARED != 0 { + let idx = earlier(&mut pos)?; + mappings[idx].source_dataset.clone() + } else { + read_null_terminated_string(heap_data, &mut pos)? + }; // Source selection, then virtual selection (both self-describing length). let source_selection = read_selection(heap_data, &mut pos)?; @@ -849,6 +901,62 @@ mod tests { assert_eq!(v1.iter_linear_1d(8).unwrap(), vec![4, 5, 6, 7]); } + #[test] + fn parse_vds_mappings_v1_shared_names() { + // Written by HDF5 2.0 (h5py, libver=("v200", "v200")) for three + // mappings from `a_rather_long_source_file.h5:a_rather_long_dataset_name` + // and one from the same file: the entries carry flags 0x00, 0x03, 0x03 + // and 0x06, so names after the first are stored as entry indices. + let blob: &[u8] = &[ + 0x01, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x61, 0x5f, 0x72, 0x61, + 0x74, 0x68, 0x65, 0x72, 0x5f, 0x6c, 0x6f, 0x6e, 0x67, 0x5f, 0x73, 0x6f, 0x75, 0x72, + 0x63, 0x65, 0x5f, 0x66, 0x69, 0x6c, 0x65, 0x2e, 0x68, 0x35, 0x00, 0x61, 0x5f, 0x72, + 0x61, 0x74, 0x68, 0x65, 0x72, 0x5f, 0x6c, 0x6f, 0x6e, 0x67, 0x5f, 0x64, 0x61, 0x74, + 0x61, 0x73, 0x65, 0x74, 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x00, 0x02, 0x00, 0x00, 0x00, + 0x03, 0x00, 0x00, 0x00, 0x01, 0x02, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, + 0x01, 0x00, 0x04, 0x00, 0x02, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x02, + 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x01, 0x00, 0x04, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x03, + 0x00, 0x00, 0x00, 0x01, 0x02, 0x01, 0x00, 0x00, 0x00, 0x04, 0x00, 0x01, 0x00, 0x01, + 0x00, 0x04, 0x00, 0x02, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x02, 0x02, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x01, 0x00, 0x04, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x03, 0x00, + 0x00, 0x00, 0x01, 0x02, 0x01, 0x00, 0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x01, 0x00, + 0x04, 0x00, 0x02, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x02, 0x02, 0x00, + 0x00, 0x00, 0x02, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, + 0x01, 0x00, 0x04, 0x00, 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, + 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x02, 0x01, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x01, 0x00, 0x01, 0x00, 0x04, 0x00, 0x02, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, + 0x00, 0x01, 0x02, 0x02, 0x00, 0x00, 0x00, 0x03, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x04, 0x00, 0x8e, 0xa7, 0xea, 0x7a, + ]; + let mappings = parse_vds_mappings(blob, 8).unwrap(); + let names: Vec<(&str, &str)> = mappings + .iter() + .map(|m| (m.source_file.as_str(), m.source_dataset.as_str())) + .collect(); + let (file, dset) = ("a_rather_long_source_file.h5", "a_rather_long_dataset_name"); + assert_eq!( + names, + vec![(file, dset), (file, dset), (file, dset), (".", dset)] + ); + } + + #[test] + fn parse_vds_mappings_v1_forward_reference_is_error() { + // Entry 0 claiming to share entry 0's file name must not index past + // the entries decoded so far. + let mut blob = vec![0x01u8, 1, 0, 0, 0, 0, 0, 0, 0, 0x01]; + blob.extend_from_slice(&[0u8; 8]); + blob.extend_from_slice(b"d\0"); + assert!(parse_vds_mappings(&blob, 8).is_err()); + // Unknown flag bits are refused. + let blob = [0x01u8, 1, 0, 0, 0, 0, 0, 0, 0, 0x08, b'd', 0]; + assert!(parse_vds_mappings(&blob, 8).is_err()); + } + #[test] fn parse_vds_mappings_external_v0() { // Block version 0 with an explicit (external) source file name. diff --git a/crates/clawhdf5/tests/vds_interop.rs b/crates/clawhdf5/tests/vds_interop.rs index 43eaee4..f3ce7e3 100644 --- a/crates/clawhdf5/tests/vds_interop.rs +++ b/crates/clawhdf5/tests/vds_interop.rs @@ -154,3 +154,39 @@ expect("v1.h5", "grid", "grid") assert_matches_libhdf5(dir.path(), "v1.h5", "strided", "strided"); assert_matches_libhdf5(dir.path(), "v1.h5", "grid", "grid"); } + +// --------------------------------------------------------------------------- +// Mapping list encoding +// --------------------------------------------------------------------------- + +/// With a 2.0 low version bound libhdf5 writes the mapping list as block +/// version 1: a flags byte per entry, and repeated names stored as the index +/// of the entry that first spelled them out. The flags byte was mistaken for +/// an empty (same-file) name. +#[test] +fn vds_mapping_block_version1_shared_names() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + generate( + dir.path(), + r#" +name = "a_rather_long_dataset_name" +with h5py.File("a_rather_long_source_file.h5", "w") as s: + s.create_dataset(name, data=np.arange(12.0) + 100) +with h5py.File("shared.h5", "w", libver=("v200", "v200")) as f: + f.create_dataset(name, data=np.arange(12.0) * -1) + lay = h5py.VirtualLayout(shape=(4, 4), dtype="f8") + for i in range(3): + src = h5py.VirtualSource("a_rather_long_source_file.h5", name, shape=(12,)) + lay[i] = src[4 * i:4 * i + 4] + lay[3] = h5py.VirtualSource(".", name, shape=(12,))[0:4] + f.create_virtual_dataset("v", lay) +# the heap block must really be version 1 for this test to mean anything +raw = open("shared.h5", "rb").read() +gcol = raw.index(b"GCOL") +assert raw[gcol + 32] == 1, "expected a version-1 VDS mapping block" +expect("shared.h5", "v", "shared") +"#, + ); + assert_matches_libhdf5(dir.path(), "shared.h5", "v", "shared"); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index c15e824..eb7d80f 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -73,6 +73,8 @@ the VDS item, which is marked. - `%b` printf-style source names are not expanded. - ~~Hyperslab selection versions 1 and 2 are refused.~~ Fixed 2026-09-25: versions 1-3 and irregular hyperslabs are decoded. + - ~~The version-1 mapping list written with a 2.0 low bound (flags byte, + shared names) was misparsed.~~ Found and fixed 2026-09-25. - **Files with a user block:** the base address is not applied. - **Old-style shared messages (version 1)** read the wrong address. - **Groups and links:**