From bc820fbd8c8a5c4704aa7098292f088bec332fb0 Mon Sep 17 00:00:00 2001 From: osobh Date: Fri, 25 Sep 2026 21:13:08 -0500 Subject: [PATCH] fix(format): refuse path-like group and dataset names FileWriter writes the root group plus one level of groups; it has no way to create intermediate groups. create_group("a/b") therefore stored a single link literally named "a/b", which no HDF5 reader can resolve (h5py: "component not found"). Nesting would mean restructuring the writer's layout around a group tree, so for now finish() rejects any group, dataset or external-link name that is empty, "." or contains '/'. Attribute names may still contain '/'. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/file_writer.rs | 26 +++++++++++++ .../tests/writer_meta_tests.rs | 39 +++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index d432f5e..2294425 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -63,6 +63,19 @@ fn build_paged_superblock_extension(page_size: u32) -> Result, FormatErr w.serialize() } +/// A group or dataset name must be one path component: not empty, not ".", +/// and without '/'. `FileWriter` writes a root group plus one level of +/// groups, and cannot create intermediate groups for a path. +fn check_link_name(name: &str) -> Result<(), FormatError> { + if name.is_empty() || name == "." || name.contains('/') { + return Err(FormatError::SerializationError(format!( + "invalid object name {name:?}: names must be a single path component \ + (FileWriter does not create nested groups)" + ))); + } + Ok(()) +} + /// Threshold for switching from compact (inline) to dense attribute storage. const DENSE_ATTR_THRESHOLD: usize = 8; @@ -1137,6 +1150,19 @@ impl FileWriter { }) }; + // Every name becomes a single link in its parent group. The writer + // has no nested groups, so a path like "a/b" would be stored as one + // link literally named "a/b" — which no HDF5 reader can resolve. + let root_names = self.root_datasets.iter().map(|d| d.name.as_str()); + let group_names = self.groups.iter().flat_map(|g| { + core::iter::once(g.name.as_str()) + .chain(g.datasets.iter().map(|d| d.name.as_str())) + .chain(g.external_links.iter().map(|l| l.0.as_str())) + }); + for name in root_names.chain(group_names) { + check_link_name(name)?; + } + let mut all_ds: Vec = Vec::new(); let mut groups: Vec = Vec::new(); let mut root_ds_indices: Vec = Vec::new(); diff --git a/crates/clawhdf5-format/tests/writer_meta_tests.rs b/crates/clawhdf5-format/tests/writer_meta_tests.rs index 4e78bbd..587f65f 100644 --- a/crates/clawhdf5-format/tests/writer_meta_tests.rs +++ b/crates/clawhdf5-format/tests/writer_meta_tests.rs @@ -524,3 +524,42 @@ fn h5py_reads_all_attributes_next_to_an_empty_string() { assert_eq!(out, r#"["", "héllo", ["", ""], 3]"#); h5dump_ok(&path); } + +// ---- 6. path-like names ---- + +#[test] +fn slash_in_a_group_or_dataset_name_is_an_error() { + // Measured: create_group("a/b") wrote one link literally named "a/b", + // which h5py cannot reach ("component not found"). The writer has no + // nested groups, so such names are refused. + let mut fw = FileWriter::new(); + let mut g = fw.create_group("a/b"); + g.create_dataset("c").with_f64_data(&[1.0]); + fw.add_group(g.finish()); + assert!(fw.finish().is_err()); + + let mut fw = FileWriter::new(); + fw.create_dataset("x/y").with_f64_data(&[1.0]); + assert!(fw.finish().is_err()); + + let mut fw = FileWriter::new(); + let mut g = fw.create_group("g"); + g.create_dataset("x/y").with_f64_data(&[1.0]); + fw.add_group(g.finish()); + assert!(fw.finish().is_err()); + + for bad in ["", "."] { + let mut fw = FileWriter::new(); + fw.create_dataset(bad).with_f64_data(&[1.0]); + assert!(fw.finish().is_err(), "{bad:?}"); + } + + // One level of groups still works, and '/' stays legal in attribute names. + let mut fw = FileWriter::new(); + let mut g = fw.create_group("g"); + g.create_dataset("c").with_f64_data(&[1.0]); + g.set_attr("m/s", AttrValue::I64(1)); + fw.add_group(g.finish()); + let bytes = fw.finish().unwrap(); + header_at(&bytes, "g/c"); +}