From e38c8133bcda52c0d4b6f670d674b5d501269e40 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 19 Sep 2026 06:39:01 -0700 Subject: [PATCH] feat(format): follow soft links; explicit errors for external links and external raw data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Path resolution follows soft links in both old-style (symbol table, cache type 2) and new-style (compact and dense Link message) groups: absolute and relative targets, links to groups, links through links, with a depth limit so a link cycle is NestingDepthExceeded rather than a hang. A dangling link reports the target it could not find. Previously every soft link was PathNotFound. - An external link is FormatError::ExternalLinkUnsupported { filename, object_path } instead of a misleading PathNotFound. - Message 0x0007 (External Data Files) is now a known MessageType, and a dataset carrying it is FormatError::ExternalDataFilesUnsupported. Such a dataset has no data address in this file, so it would otherwise be read as "never written" and answered with fill values — wrong data, no error. - Dense link iteration is shared between hard-link listing and the new symbolic-link lookup; entry listing behaviour is unchanged. - h5py interop test for both libver settings. Co-Authored-By: Claude Fable 5.1 --- crates/clawhdf5-format/src/error.rs | 23 +++ crates/clawhdf5-format/src/fill_value.rs | 9 ++ crates/clawhdf5-format/src/group_v1.rs | 48 ++++++ crates/clawhdf5-format/src/group_v2.rs | 153 +++++++++++++++++--- crates/clawhdf5-format/src/message_type.rs | 17 ++- crates/clawhdf5/tests/h5py_interop_tests.rs | 85 +++++++++++ 6 files changed, 314 insertions(+), 21 deletions(-) diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index 43a1cae..a8fa0ec 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -117,6 +117,17 @@ 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, + /// The dataset's raw data is stored in external files (External Data + /// Files message), which this reader does not follow. + ExternalDataFilesUnsupported, + /// The path goes through an external link (a link into another file), + /// which this reader does not follow. + ExternalLinkUnsupported { + /// The file the link points into. + filename: String, + /// The object path within that file. + object_path: String, + }, /// Invalid SOHM table version. InvalidSohmTableVersion(u8), /// Invalid SOHM table signature (expected "SMTB"). @@ -310,6 +321,18 @@ impl fmt::Display for FormatError { FormatError::InvalidSharedMessageVersion(v) => { write!(f, "invalid shared message version: {v}") } + FormatError::ExternalLinkUnsupported { + filename, + object_path, + } => write!( + f, + "path goes through an external link to {object_path} in {filename}, which is \ + not supported" + ), + FormatError::ExternalDataFilesUnsupported => write!( + f, + "dataset raw data is stored in external file(s), which is not supported" + ), FormatError::UnresolvedSharedMessage => write!( f, "message is shared but no file data was available to resolve it" diff --git a/crates/clawhdf5-format/src/fill_value.rs b/crates/clawhdf5-format/src/fill_value.rs index f1b0c3b..d10ba47 100644 --- a/crates/clawhdf5-format/src/fill_value.rs +++ b/crates/clawhdf5-format/src/fill_value.rs @@ -165,6 +165,15 @@ pub fn read_full_with_fill>( length_size: u8, read: impl FnOnce() -> Result, E>, ) -> Result, E> { + // A dataset with external raw data also has no data address in this + // file. It is NOT unallocated — its values live elsewhere — so it must + // never be answered with the fill value. + if messages + .iter() + .any(|m| m.msg_type == MessageType::ExternalDataFiles) + { + return Err(FormatError::ExternalDataFilesUnsupported.into()); + } let fill = dataset_fill_value(messages)?; if !has_storage(layout) { return Ok(filled_dataset(dataspace, elem_size, fill.as_deref())?); diff --git a/crates/clawhdf5-format/src/group_v1.rs b/crates/clawhdf5-format/src/group_v1.rs index f295039..989f826 100644 --- a/crates/clawhdf5-format/src/group_v1.rs +++ b/crates/clawhdf5-format/src/group_v1.rs @@ -60,6 +60,54 @@ pub fn resolve_v1_group_entries( Ok(entries) } +/// Symbol table cache type for a soft link: the scratch pad's first four bytes +/// are the local-heap offset of the link's target path, and the entry's object +/// header address is undefined. +const CACHE_TYPE_SOFT_LINK: u32 = 2; + +/// The target path of the soft link called `name` in a v1 group, if any. +pub fn find_v1_soft_link( + file_data: &[u8], + sym_table_msg: &SymbolTableMessage, + name: &str, + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + let heap = LocalHeap::parse( + file_data, + sym_table_msg.local_heap_address as usize, + offset_size, + length_size, + )?; + let snod_addrs = collect_symbol_table_nodes( + file_data, + sym_table_msg.btree_address, + offset_size, + length_size, + )?; + 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.read_string(file_data, entry.link_name_offset)? != name { + continue; + } + let value_offset = u32::from_le_bytes([ + entry.scratch_pad[0], + entry.scratch_pad[1], + entry.scratch_pad[2], + entry.scratch_pad[3], + ]); + return heap + .read_string(file_data, u64::from(value_offset)) + .map(Some); + } + } + Ok(None) +} + /// Extract the SymbolTableMessage from an object header's messages. fn find_symbol_table_message( obj_header: &ObjectHeader, diff --git a/crates/clawhdf5-format/src/group_v2.rs b/crates/clawhdf5-format/src/group_v2.rs index 87f1d65..903c7d6 100644 --- a/crates/clawhdf5-format/src/group_v2.rs +++ b/crates/clawhdf5-format/src/group_v2.rs @@ -63,14 +63,15 @@ fn resolve_compact_entries( Ok(entries) } -/// Resolve entries from dense storage (fractal heap + B-tree v2). -fn resolve_dense_entries( +/// Visit every link in dense storage (fractal heap + B-tree v2 name index). +fn for_each_dense_link( file_data: &[u8], link_info: &LinkInfoMessage, fh_addr: u64, offset_size: u8, length_size: u8, -) -> Result, FormatError> { + mut visit: impl FnMut(LinkMessage), +) -> Result<(), FormatError> { // Parse fractal heap let fh = FractalHeapHeader::parse(file_data, fh_addr as usize, offset_size, length_size)?; @@ -81,7 +82,6 @@ fn resolve_dense_entries( let btree_hdr = BTreeV2Header::parse(file_data, btree_addr as usize, offset_size, length_size)?; let records = collect_btree_v2_records(file_data, &btree_hdr, offset_size, length_size)?; - let mut entries = Vec::new(); for record in &records { // For type 5 (name index): hash(4) + heap_id(heap_id_length) // For type 6 (creation order): creation_order(8) + heap_id(heap_id_length) @@ -98,22 +98,94 @@ fn resolve_dense_entries( // Read managed object from fractal heap let link_data = fh.read_managed_object(file_data, id_bytes, offset_size)?; + visit(LinkMessage::parse(&link_data, offset_size)?); + } + Ok(()) +} - // Parse as Link message - let link = LinkMessage::parse(&link_data, offset_size)?; - if let LinkTarget::Hard { - object_header_address, - } = link.link_target - { - entries.push(GroupEntry { - name: link.name, +/// Resolve entries from dense storage (fractal heap + B-tree v2). +fn resolve_dense_entries( + file_data: &[u8], + link_info: &LinkInfoMessage, + fh_addr: u64, + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + let mut entries = Vec::new(); + for_each_dense_link( + file_data, + link_info, + fh_addr, + offset_size, + length_size, + |link| { + if let LinkTarget::Hard { object_header_address, - cache_type: 0, - }); + } = link.link_target + { + entries.push(GroupEntry { + name: link.name, + object_header_address, + cache_type: 0, + }); + } + }, + )?; + Ok(entries) +} + +/// The soft or external link called `name` in this group, if there is one. +/// Hard links are what `resolve_group_entries` returns; this is consulted only +/// when a path component isn't among them. +fn find_symbolic_link( + file_data: &[u8], + object_header: &ObjectHeader, + name: &str, + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + if is_v1_group(object_header) { + let Some(sym_msg) = object_header + .messages + .iter() + .find(|m| m.msg_type == MessageType::SymbolTable) + else { + return Ok(None); + }; + let stm = SymbolTableMessage::parse(&sym_msg.data, offset_size)?; + return group_v1::find_v1_soft_link(file_data, &stm, name, offset_size, length_size) + .map(|target| target.map(|target_path| LinkTarget::Soft { target_path })); + } + if !is_v2_group(object_header) { + return Ok(None); + } + let is_symbolic = |t: &LinkTarget| !matches!(t, LinkTarget::Hard { .. }); + let link_info = find_link_info(object_header, offset_size)?; + let mut found = None; + if let Some(fh_addr) = link_info.fractal_heap_address { + for_each_dense_link( + file_data, + &link_info, + fh_addr, + offset_size, + length_size, + |link| { + if link.name == name && is_symbolic(&link.link_target) { + found = Some(link.link_target); + } + }, + )?; + } else { + for msg in &object_header.messages { + if msg.msg_type == MessageType::Link { + let link = LinkMessage::parse(&msg.data, offset_size)?; + if link.name == name && is_symbolic(&link.link_target) { + found = Some(link.link_target); + } + } } } - - Ok(entries) + Ok(found) } /// Find and parse the Link Info message from an object header. @@ -158,6 +230,19 @@ pub fn resolve_path_any( file_data: &[u8], superblock: &Superblock, path: &str, +) -> Result { + resolve_path_following_links(file_data, superblock, path, 0) +} + +/// Soft links followed while resolving one path. Guards against link cycles +/// (`a -> b -> a`), which are legal to create. +const MAX_SOFT_LINK_DEPTH: u8 = 16; + +fn resolve_path_following_links( + file_data: &[u8], + superblock: &Superblock, + path: &str, + depth: u8, ) -> Result { let components: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect(); if components.is_empty() { @@ -176,7 +261,9 @@ pub fn resolve_path_any( for (i, component) in components.iter().enumerate() { let entries = resolve_group_entries(file_data, ¤t_header, os, ls)?; - let found = entries.iter().find(|e| e.name == *component); + let found = entries + .iter() + .find(|e| e.name == *component && e.object_header_address != u64::MAX); match found { Some(entry) => { if i == components.len() - 1 { @@ -186,7 +273,37 @@ pub fn resolve_path_any( current_header = ObjectHeader::parse(file_data, current_addr as usize, os, ls)?; } None => { - return Err(FormatError::PathNotFound(String::from(*component))); + return match find_symbolic_link(file_data, ¤t_header, component, os, ls)? { + Some(LinkTarget::Soft { target_path }) => { + if depth >= MAX_SOFT_LINK_DEPTH { + return Err(FormatError::NestingDepthExceeded); + } + // A relative target is relative to the group holding + // the link; then the rest of the original path. + let mut full = String::new(); + if !target_path.starts_with('/') { + for parent in &components[..i] { + full.push('/'); + full.push_str(parent); + } + } + full.push('/'); + full.push_str(&target_path); + for rest in &components[i + 1..] { + full.push('/'); + full.push_str(rest); + } + resolve_path_following_links(file_data, superblock, &full, depth + 1) + } + Some(LinkTarget::External { + filename, + object_path, + }) => Err(FormatError::ExternalLinkUnsupported { + filename, + object_path, + }), + _ => Err(FormatError::PathNotFound(String::from(*component))), + }; } } } diff --git a/crates/clawhdf5-format/src/message_type.rs b/crates/clawhdf5-format/src/message_type.rs index 6b66a9a..dce4d24 100644 --- a/crates/clawhdf5-format/src/message_type.rs +++ b/crates/clawhdf5-format/src/message_type.rs @@ -9,6 +9,9 @@ pub enum MessageType { Datatype, FillValueOld, FillValue, + /// External Data Files (0x0007): the dataset's raw data lives in other + /// files, listed by this message. + ExternalDataFiles, Link, DataLayout, GroupInfo, @@ -36,6 +39,7 @@ impl MessageType { 0x0004 => MessageType::FillValueOld, 0x0005 => MessageType::FillValue, 0x0006 => MessageType::Link, + 0x0007 => MessageType::ExternalDataFiles, 0x0008 => MessageType::DataLayout, 0x000A => MessageType::GroupInfo, 0x000B => MessageType::FilterPipeline, @@ -60,6 +64,7 @@ impl MessageType { MessageType::Datatype => 0x0003, MessageType::FillValueOld => 0x0004, MessageType::FillValue => 0x0005, + MessageType::ExternalDataFiles => 0x0007, MessageType::Link => 0x0006, MessageType::DataLayout => 0x0008, MessageType::GroupInfo => 0x000A, @@ -90,6 +95,7 @@ mod tests { (0x0003, MessageType::Datatype), (0x0004, MessageType::FillValueOld), (0x0005, MessageType::FillValue), + (0x0007, MessageType::ExternalDataFiles), (0x0006, MessageType::Link), (0x0008, MessageType::DataLayout), (0x000A, MessageType::GroupInfo), @@ -119,8 +125,13 @@ mod tests { #[test] fn unknown_type_zero_gap() { - // 0x0007 is not a defined type - let mt = MessageType::from_u16(0x0007); - assert_eq!(mt, MessageType::Unknown(0x0007)); + // 0x0009 is reserved for the library's own testing; no file uses it. + let mt = MessageType::from_u16(0x0009); + assert_eq!(mt, MessageType::Unknown(0x0009)); + // 0x0007 used to be treated as unknown: it is External Data Files. + assert_eq!( + MessageType::from_u16(0x0007), + MessageType::ExternalDataFiles + ); } } diff --git a/crates/clawhdf5/tests/h5py_interop_tests.rs b/crates/clawhdf5/tests/h5py_interop_tests.rs index ef6d01a..83becfb 100644 --- a/crates/clawhdf5/tests/h5py_interop_tests.rs +++ b/crates/clawhdf5/tests/h5py_interop_tests.rs @@ -699,3 +699,88 @@ with h5py.File("{path_str}", "r") as f: ); } } + +// --------------------------------------------------------------------------- +// h5py writes soft / external links and external raw data -> clawhdf5 +// --------------------------------------------------------------------------- + +/// Soft links are followed (absolute, relative, through groups, with a cycle +/// guard). Things this reader does not follow — external links, and datasets +/// whose raw data lives in another file — are explicit errors. They used to +/// surface as a misleading `PathNotFound`, and external raw data could read +/// back as fill values. +#[test] +fn h5py_links_clawhdf5_resolves_or_refuses() { + use clawhdf5::Error; + use clawhdf5_format::error::FormatError; + + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let dir_str = dir.path().display().to_string(); + for (tag, kwargs) in [("default", ""), ("latest", ", libver='latest'")] { + let script = format!( + r#" +import h5py, numpy as np, os +os.chdir("{dir_str}") +with h5py.File("other_{tag}.h5", "w"{kwargs}) as o: + o.create_dataset("remote", data=np.arange(3, dtype="