diff --git a/crates/clawhdf5-format/src/chunk_cache.rs b/crates/clawhdf5-format/src/chunk_cache.rs index aedb602..6e3fee8 100644 --- a/crates/clawhdf5-format/src/chunk_cache.rs +++ b/crates/clawhdf5-format/src/chunk_cache.rs @@ -262,6 +262,11 @@ struct CachedChunk { struct DatasetEntry { /// Chunk coordinate -> ChunkInfo (offset + size in file). index: Option>>, + /// The same chunks in the order the chunk index lists them: what + /// [`ChunkCache::chunks_for`] returns, so a cached read walks (and, on a + /// damaged file, fails at) the chunks in the same order as an uncached + /// one, rather than in hash-map order. + ordered: Option>>, /// Pre-built chunk index for O(1) coordinate lookups. chunk_index: Option>, /// Pre-computed chunk layout for fast assembly. @@ -274,6 +279,7 @@ struct DatasetEntry { impl DatasetEntry { fn weight(&self) -> usize { self.index.as_ref().map_or(0, |m| m.len()) + + self.ordered.as_ref().map_or(0, |o| o.len()) + self.chunk_index.as_ref().map_or(0, |c| c.num_chunks()) } } @@ -562,11 +568,22 @@ impl ChunkCache { rank: usize, build: impl FnOnce() -> Result, E>, ) -> Result, E> { - Ok(self - .index_for(addr, rank, build)? - .values() - .cloned() - .collect()) + if let Some(ordered) = self.lock().touch(addr).ordered.clone() { + return Ok(ordered.as_ref().clone()); + } + let chunks = build()?; + let map: HashMap = chunks + .iter() + .map(|ci| (ci.offsets.iter().take(rank).copied().collect(), ci.clone())) + .collect(); + let mut inner = self.lock(); + let entry = inner.touch(addr); + // Another thread may have built this dataset's index meanwhile: keep + // the first one, so every reader sees the same order. + let ordered = Arc::clone(entry.ordered.get_or_insert_with(|| Arc::new(chunks))); + entry.index.get_or_insert_with(|| Arc::new(map)); + inner.trim_datasets(addr); + Ok(ordered.as_ref().clone()) } fn index_for( @@ -580,11 +597,14 @@ impl ChunkCache { } let chunks = build()?; let map: HashMap = chunks - .into_iter() - .map(|ci| (ci.offsets.iter().take(rank).copied().collect(), ci)) + .iter() + .map(|ci| (ci.offsets.iter().take(rank).copied().collect(), ci.clone())) .collect(); let mut inner = self.lock(); let entry = inner.touch(addr); + if entry.index.is_none() { + entry.ordered = Some(Arc::new(chunks)); + } let index = Arc::clone(entry.index.get_or_insert_with(|| Arc::new(map))); inner.trim_datasets(addr); Ok(index) diff --git a/crates/clawhdf5/tests/h5py_chunked_read_tests.rs b/crates/clawhdf5/tests/h5py_chunked_read_tests.rs index 467931e..393a22c 100644 --- a/crates/clawhdf5/tests/h5py_chunked_read_tests.rs +++ b/crates/clawhdf5/tests/h5py_chunked_read_tests.rs @@ -455,3 +455,70 @@ with h5py.File("{p}", "w") as f: assert!(lazy.dataset("line").unwrap().read_i32().is_err()); assert!(lazy.dataset("grid").unwrap().read_f64().is_err()); } + +// --------------------------------------------------------------------------- +// Several damaged chunks: which one is reported +// --------------------------------------------------------------------------- + +/// With more than one damaged chunk, the error names the first damaged chunk +/// in the chunk index's order, every time and on every read path. The cached +/// reader used to walk the chunks in hash-map order, so two opens of the same +/// file could report different chunks (`cve-2025-2310.h5`). +#[test] +fn several_damaged_chunks_report_the_same_chunk_every_time() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("damaged.h5"); + let p = path.display().to_string(); + run_python(&format!( + r#" +import h5py, numpy as np, zlib +with h5py.File("{p}", "w") as f: + ds = f.create_dataset("d", shape=(512,), chunks=(8,), dtype="