From 751edeb7e6fa9edcbcea6e23c0906459176aec28 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 08:57:52 -0500 Subject: [PATCH] fix(format): refuse a dense link or attribute too big for the heap A message in dense storage is a fractal heap object, and an object must fit one direct block: 65 515 bytes here, since the writer has no huge-object path. A bigger one (a soft link with a 80 000-byte target in a group of more than 8 links) was written without error, cut off at the end of its block, and libhdf5 could not list the group ("object overruns end of direct block"). finish() now fails with an error that names the limit, for links and for dense attributes; a 65 001-byte soft link target still works and h5py reads it back. The heap packer also skips a child indirect block whose blocks are all too small for the next object instead of walking it. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/file_writer.rs | 30 +++++++++++-- .../clawhdf5/tests/writer_groups_interop.rs | 42 +++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 1574dc1..65181a0 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -316,6 +316,18 @@ pub(crate) fn build_single_block_fractal_heap( // Direct block layout: sig(4) + ver(1) + heap_addr(os) + block_offset(bo_bytes) // + checksum(4) [when flags bit 1 set] + data... let dblock_header_size = 4 + 1 + os + block_offset_bytes + 4; // +4 for checksum + + // An object must fit one direct block: the writer has no huge-object + // path, and libhdf5 cannot read an object that overruns its block. + let max_managed = max_direct_block_size as usize - dblock_header_size; + if let Some(big) = serialized.iter().find(|s| s.len() > max_managed) { + return Err(FormatError::SerializationError(format!( + "a {}-byte message cannot go in dense storage: a fractal heap \ + object holds at most {max_managed} bytes (huge heap objects are \ + not written)", + big.len() + ))); + } let total_data_size: usize = serialized.iter().map(|s| s.len()).sum(); let dblock_content_size = dblock_header_size + total_data_size; let starting_block_size = dblock_content_size.next_power_of_two().max(512) as u64; @@ -373,8 +385,7 @@ pub(crate) fn build_single_block_fractal_heap( frhp.extend_from_slice(&heap_id_length.to_le_bytes()); frhp.extend_from_slice(&0u16.to_le_bytes()); // io_filter_encoded_length frhp.push(0x02); // flags: bit 1 = checksum direct blocks - let max_managed = max_direct_block_size as u32 - dblock_header_size as u32; - frhp.extend_from_slice(&max_managed.to_le_bytes()); + frhp.extend_from_slice(&(max_managed as u32).to_le_bytes()); write_length(&mut frhp, 0, LENGTH_SIZE); // next_huge_object_id write_undef_offset(&mut frhp, OFFSET_SIZE); // btree_huge_objects_address write_length(&mut frhp, free_space as u64, LENGTH_SIZE); // free_space_managed_blocks @@ -455,7 +466,8 @@ pub(crate) fn build_single_block_fractal_heap( /// in turn hold indirect blocks. Objects are packed into direct blocks in /// heap-offset order and never span blocks; a block too small for the next /// object is left unallocated (an undefined address), as libhdf5 skips rows -/// when it needs a bigger block. There is no huge-object path. +/// when it needs a bigger block. The caller has checked that every object +/// fits a maximum-size direct block (there is no huge-object path). fn build_multiblock_fractal_heap( serialized: &[Vec], base_address: u64, @@ -730,7 +742,17 @@ impl HeapPacker<'_> { } else if row < geom.max_direct_rows() { slots.push(self.fill_direct(off, size)); } else { - let child = self.fill(off, Some(geom.rows_for_size(size)))?; + let child_rows = geom.rows_for_size(size); + // A child whose biggest direct block cannot hold the + // next object is skipped whole, not walked. + let biggest = geom.row_size(child_rows.min(geom.max_direct_rows()) - 1); + if self.objects[self.next].len() > (biggest as usize - geom.dblock_header_size) + { + slots.push(HeapSlot::Empty); + off += size; + continue; + } + let child = self.fill(off, Some(child_rows))?; let used = child.slots.iter().any(|s| !matches!(s, HeapSlot::Empty)); slots.push(if used { HeapSlot::Indirect(child) diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index 3b8727d..770b835 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -765,3 +765,45 @@ fn dense_attributes_past_the_direct_blocks_of_the_root() { } } } + +#[test] +fn a_link_too_big_for_dense_storage_is_an_error() { + // A link message must fit one fractal heap direct block (64 KiB less + // its header); the writer has no huge-object path. It used to be + // written anyway, cut off, and libhdf5 could not list the group. + let mut b = FileBuilder::new(); + for i in 0..10 { + b.create_dataset(&format!("d{i}")).with_i32_data(&[i]); + } + b.add_soft_link("s", &"/y".repeat(40_000)); + let err = b.finish().unwrap_err().to_string(); + assert!(err.contains("fractal heap object holds at most"), "{err}"); + // The same for a dense attribute. + let mut b = FileBuilder::new(); + let x = b.create_dataset("x"); + x.with_i32_data(&[1]); + for i in 0..9 { + x.set_attr(&format!("a{i}"), AttrValue::I64(i)); + } + x.set_attr("big", AttrValue::F64Array(vec![0.5; 9_000])); + let err = b.finish().unwrap_err().to_string(); + assert!(err.contains("fractal heap object holds at most"), "{err}"); + + // Just under the limit is fine, and libhdf5 reads it back. + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + for i in 0..10 { + b.create_dataset(&format!("d{i}")).with_i32_data(&[i]); + } + let target = format!("/{}", "y".repeat(65_000)); + b.add_soft_link("s", &target); + let path = write(&dir, "long_soft.h5", b); + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 print(json.dumps([len(f), len(f.get('s', getlink=True).path)]))", + ); + assert_eq!(out, "[11, 65001]"); + h5dump_ok(&path); +}