From bdf584abb2c3307382f0550b0a6985bcb3c0a3ba Mon Sep 17 00:00:00 2001 From: osobh Date: Sun, 27 Sep 2026 07:40:11 -0500 Subject: [PATCH] format: chunk grids with a zero extent have no chunks (no division by zero) A Fixed or Extensible Array index whose maximum extent (the current one when no maximum is recorded) is 0 along a dimension has a zero stride for every dimension before it; ChunkGrid::offsets divided by it. The unfixed editor made such files by resizing a clawhdf5-written dataset to a zero extent: `h5rs check` panicked and the next resize raised an internal error (12 of the reviewer's random-edit seeds 10..39). Such an index has no slot for any chunk of the dataset; offsets now returns None. Tests: chunk_grid::zero_extent_has_no_chunks; edit_interop's zero_extent_resizes_without_a_recorded_maximum on a file the unfixed editor left (fixture) and on a 2.7.0-written file taken through zero extents with `h5rs check --data` and h5dump at every step; test_edit.py random edits on seeds 10..39 of a clawhdf5-written file. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 16 +++++ crates/clawhdf5-format/src/chunk_grid.rs | 28 ++++++++ crates/clawhdf5-py/tests/test_edit.py | 7 ++ crates/clawhdf5-tools/tests/edit_interop.rs | 65 ++++++++++++++++++ .../fixtures/chunk_zero_extent_no_maxshape.h5 | Bin 0 -> 5158 bytes 5 files changed, 116 insertions(+) create mode 100644 crates/clawhdf5/tests/fixtures/chunk_zero_extent_no_maxshape.h5 diff --git a/CHANGELOG.md b/CHANGELOG.md index b34d1db..5c0e719 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ ## Unreleased +### Correctness: zero extents in Fixed/Extensible Array chunk indexes (2026-09-27) +- **A chunked dataset whose maximum (or, with none recorded, current) + extent is 0 along a dimension made the reader divide by zero** (fixed + 2026-09-27): `h5rs check` panicked ("attempt to divide by zero", + `chunk_grid.rs`) and the next `FileEditor::resize` failed with an + internal error. The unfixed editor produced such files by resizing a + clawhdf5-written dataset to a zero extent (12 of 30 extra random-edit + seeds on clawhdf5-written files). Such an index has no slot for any + chunk of the dataset, and `ChunkGrid::offsets` now says so instead of + dividing by the zero stride. Tests: `chunk_grid`'s + `zero_extent_has_no_chunks`, `edit_interop.rs`'s + `zero_extent_resizes_without_a_recorded_maximum` (a file the unfixed + editor left checks clean and resizes on; a 2.7.0-written file through + zero extents checks clean at each step), and `test_edit.py`'s random + edits on seeds 10 to 39 of a clawhdf5-written file. + ### Correctness: resizing chunked datasets with no recorded maximum (2026-09-27) - **`FileEditor::resize` scrambled the values of a chunked dataset whose dataspace records no maximum dimensions when it shrank it** (fixed diff --git a/crates/clawhdf5-format/src/chunk_grid.rs b/crates/clawhdf5-format/src/chunk_grid.rs index 9e03b9d..8957512 100644 --- a/crates/clawhdf5-format/src/chunk_grid.rs +++ b/crates/clawhdf5-format/src/chunk_grid.rs @@ -135,6 +135,12 @@ impl ChunkGrid { let mut rem = index; for p in 0..rank { let d = self.order[p]; + // A zero stride: a later dimension has no chunks (its maximum, + // or with none recorded its current extent, is 0), so no slot of + // the index is a chunk of the dataset. + if self.down[p] == 0 { + return None; + } let scaled = rem / self.down[p]; rem %= self.down[p]; if scaled >= self.cur_chunks[d] { @@ -193,6 +199,28 @@ mod tests { assert_eq!(g.offsets(11), Some(vec![2, 3])); } + #[test] + fn zero_extent_has_no_chunks() { + // No maximum recorded and a zero current dimension: every stride + // before it is 0 (this divided by zero). + let g = ChunkGrid::fixed_array(&[1, 0], None, &[6, 6]).unwrap(); + for i in 0..16 { + assert_eq!(g.offsets(i), None); + } + let g = ChunkGrid::fixed_array(&[0, 0, 3], Some(&[4, 0, 3]), &[2, 2, 3]).unwrap(); + for i in 0..16 { + assert_eq!(g.offsets(i), None); + } + let g = ChunkGrid::extensible_array(&[0, 5], Some(&[u64::MAX, 0]), &[2, 2]).unwrap(); + for i in 0..16 { + assert_eq!(g.offsets(i), None); + } + // A zero last dimension leaves the other strides alone. + let g = ChunkGrid::fixed_array(&[4, 0], Some(&[4, 6]), &[2, 3]).unwrap(); + assert_eq!(g.offsets(0), None); + assert_eq!(g.linear_index(&[1, 1]), 3); + } + #[test] fn rejects_two_unlimited_dims_after_the_first() { assert!(ChunkGrid::fixed_array(&[4, 6], Some(&[u64::MAX, u64::MAX]), &[2, 3]).is_err()); diff --git a/crates/clawhdf5-py/tests/test_edit.py b/crates/clawhdf5-py/tests/test_edit.py index a2fd83d..690bd0f 100644 --- a/crates/clawhdf5-py/tests/test_edit.py +++ b/crates/clawhdf5-py/tests/test_edit.py @@ -445,6 +445,13 @@ def test_random_edits_match_h5py(h5py, tmp_path, source, seed): h5dump_reads(ours_path, base) +@pytest.mark.parametrize("seed", range(10, 40)) +def test_random_edits_on_clawhdf5_files(h5py, tmp_path, seed): + """More random sequences on a clawhdf5-written file, whose resizes to + zero extents once left files `h5rs check` could not read.""" + test_random_edits_match_h5py(h5py, tmp_path, "clawhdf5", seed) + + def resized_model(before, shape, fill): """`before` resized to `shape` as HDF5 resizes: elements inside both extents keep their values, the others read as the fill value.""" diff --git a/crates/clawhdf5-tools/tests/edit_interop.rs b/crates/clawhdf5-tools/tests/edit_interop.rs index 3a37e83..fad815c 100644 --- a/crates/clawhdf5-tools/tests/edit_interop.rs +++ b/crates/clawhdf5-tools/tests/edit_interop.rs @@ -1509,3 +1509,68 @@ fn unencodable_filters_are_unsupported() { "a refused edit changed the file" ); } + +fn fixture(dir: &Path, name: &str) -> std::path::PathBuf { + let path = dir.join(name); + std::fs::copy( + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../clawhdf5/tests/fixtures") + .join(name), + &path, + ) + .unwrap(); + path +} + +/// Zero extents on chunked datasets with no recorded maximum. The unfixed +/// editor left `chunk_zero_extent_no_maxshape.h5` (a 2.7.0-written file +/// resized to 1x0): a Fixed Array whose maximum, taken from the current +/// dimensions, has no chunks along one dimension, so every stride before it +/// is 0 — `h5rs check` panicked dividing by it and the next resize failed +/// with an internal error. Such a file must check clean and resize on; a +/// 2.7.0-written file taken through zero extents by the fixed editor must +/// check clean at every step and read the fill value where it grew. +#[test] +fn zero_extent_resizes_without_a_recorded_maximum() { + if !tools_ok() { + return; + } + let dir = tmpdir(); + let path = fixture(dir.path(), "chunk_zero_extent_no_maxshape.h5"); + check_tools(&path, true); + let mut ed = FileEditor::open(&path).unwrap(); + ed.resize("d", &[0, 0]).unwrap(); + ed.resize("z", &[0, 0]).unwrap(); + // Their maximum is now what the index was laid out by (1 x 0). + ed.resize("d", &[1, 0]).unwrap(); + assert!(matches!( + ed.resize("d", &[1, 1]), + Err(Error::InvalidArgument(_)) + )); + drop(ed); + check_tools(&path, true); + + let path = fixture(dir.path(), "chunked_no_maxshape_v2_7_0.h5"); + for shape in [[15, 15], [3, 2], [1, 1], [1, 0], [0, 0], [0, 20], [20, 20]] { + let mut ed = FileEditor::open(&path).unwrap(); + ed.resize("d", &shape).unwrap(); + ed.resize("z", &shape).unwrap(); + drop(ed); + check_tools(&path, true); + } + let f = File::open(&path).unwrap(); + for name in ["d", "z"] { + let d = f.dataset(name).unwrap(); + assert_eq!(d.shape().unwrap(), [20, 20]); + assert!(d.read_f32().unwrap().iter().all(|&v| v == 0.0), "{name}"); + } + assert_eq!( + py(&format!( + "import h5py\n\ + with h5py.File({:?}) as f:\n\ + \x20 print(int(abs(f['d'][()]).sum() + abs(f['z'][()]).sum()), f['d'].maxshape)", + path.to_str().unwrap() + )), + "0 (20, 20)" + ); +} diff --git a/crates/clawhdf5/tests/fixtures/chunk_zero_extent_no_maxshape.h5 b/crates/clawhdf5/tests/fixtures/chunk_zero_extent_no_maxshape.h5 new file mode 100644 index 0000000000000000000000000000000000000000..fa1e0efe7d8dee313fb482a5db333a66a857c9b2 GIT binary patch literal 5158 zcmdtm2~ZPf6bJB^i#yyX9%w{CJW!@$2?|Oi#i#_Wg2e-k0)jW7BA!+8K&t|R*9aan zYPF*bEhwmC!UC*Csg#5Pn>(e};!=I!^g@7tF+tjR7{DgC4- z%}qo`M#RNmY&hF86*u;U`De{~3{)ux3u&a#T36#v5YE*R!89 zOKu%LTEG<8*QaH$>vr3l&0sF$FR~?pm8><1YtEEWQzL5nEsh14OeiD)+re&3BtcDN zVuazuLJ|oK48$UnH&W(hk4N(*%(mnHjctFZ`2>heo9IT-y>UU!n7WaeqqTX#JDCeA zMP?+h#0Sj14{m;La0z?Bn_noz{JA#Fi#|aEIx*Y%{?m3Mb{pn0wI7ES`*DcGV!;D; zH?M-;TiLV!!>PcBNDHJOvIH5p!8y1JTBw97s0Mi-A}{a(C8)q37Q<3VfEBP3R>L01 zfK2!avSB}5g3E9fuER~JfX0RoW*P(?2Voru!Mu7B)@vY@SI@=zAms7t zTCDHDJzl-u3?Hya!rUA9v!l-*`?l1xcO+_)^wiT(+pMR49JL}n^{1#k*Hiz1k?WmO zg*{%vukZ@0;ZHF0C6aPApf?1-Ko|sH z!(bQ!V<8m6ARNX)1T;3p=#hW{uh$;yX3(5h?~Ju8bm7%~uvS1HUOf=&AQ;N4M`Ar5 zCh+Qwo!*Uhd987K)EC>a?( zSXI9)1dy65laREZI5j&6xw22W=MlDU|qL!TomPe&Bs< z&SQUM&NXJP^r8DrZW3D-IyyKgwx`(ivZh@tjPKXB)H`72udaG@DyQ|4xVS&6C1N|$?t8#?yx9UJTPf`VpU@eSvDyM*^!u>HRzE=*oNOcksQ^dG4X@f|YYM(OpU%Via3 zd&}Hh^;+2dM zIrHleCPI=5({2%G7Vkk-RaS+>Q(@tX{B@+OaXRYXK#JGz%E z4dEJix#s#pcbTH4=G3~qBXTC+$eH@rJCR}3?r1+@?j@q~MDts`p1R9A{jXm+^o)J( z6<2j`+p%TQTz|zQ3qjom-f^A&D;c*A{r=Q5=3eFb(37>-uk^KW;`P*Ba{bBjf{J|E z%j)Qx9qsMY(yiKcAsQ}~*wsB(-*L^|N?=}lw<=;Ez*KBE*j;nC5^#@XxVs9o%l~in z;Kv)MxO+GENnK)}>nN)CIWqTd?zz#5o+tcuXX*ZJ@pnlg&^;j1J##VuL(tupb+=>_ IFa>`64FI8F{Qv*} literal 0 HcmV?d00001