diff --git a/CHANGELOG.md b/CHANGELOG.md index 93cf91e..7fdde48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -323,7 +323,9 @@ VAX byte order; libhdf5 ignores it before version 3, and so does this. - chunked layouts (`FormatError::InvalidChunkDimensions`): a zero chunk dimension, a chunk rank that does not match the dataspace, a chunk of - 4 GiB or more (0x80000000-sized chunks hung the reader), and v1 B-tree + 4 GiB or more indexed by a v1 B-tree (layout version 3 or earlier; + 0x80000000-sized chunks hung the reader — layout versions 4 and 5 allow + larger chunks, and HDF5 2.0 writes them), and v1 B-tree chunk keys whose offsets are not multiples of the chunk dimensions, including the keys that only bound a node (`chunked_read::collect_chunk_info_checked`). diff --git a/crates/clawhdf5-format/src/chunked_read.rs b/crates/clawhdf5-format/src/chunked_read.rs index beac35f..7abf021 100644 --- a/crates/clawhdf5-format/src/chunked_read.rs +++ b/crates/clawhdf5-format/src/chunked_read.rs @@ -152,12 +152,15 @@ pub(crate) fn checked_byte_len(elements: u64, elem_size: usize) -> Result Result<(usize, Vec), FormatError> { @@ -180,9 +183,9 @@ pub(crate) fn chunk_geometry( let bytes = spatial .iter() .fold(elem_size as u128, |acc, &c| acc * u128::from(c)); - if bytes > u128::from(u32::MAX) { + if layout_version < 4 && bytes > u128::from(u32::MAX) { return Err(FormatError::InvalidChunkDimensions(format!( - "chunk size must be < 4GB (chunk {spatial:?} of {elem_size}-byte elements)" + "chunk size must be < 4GB with v1 b-tree index (chunk {spatial:?} of {elem_size}-byte elements)" ))); } Ok((rank, spatial.iter().map(|&c| c as usize).collect())) @@ -658,7 +661,7 @@ pub fn list_chunks( .ok_or_else(|| FormatError::ChunkedReadError("no address for chunked layout".into()))?; // Both v3 and v4 include element size as last dim (rank+1) - let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, dataspace, elem_size)?; + let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, version, dataspace, elem_size)?; let ds_dims: Vec = dataspace.dimensions.iter().map(|&d| d as usize).collect(); // Collect chunks based on version and index type @@ -894,12 +897,13 @@ pub fn read_chunked_data_cached( length_size: u8, cache: &ChunkCache, ) -> Result, FormatError> { - let (chunk_dimensions, addr_opt) = match layout { + let (chunk_dimensions, version, addr_opt) = match layout { DataLayout::Chunked { chunk_dimensions, + version, btree_address, .. - } => (chunk_dimensions, *btree_address), + } => (chunk_dimensions, *version, *btree_address), _ => { return Err(FormatError::ChunkedReadError( "expected chunked layout".into(), @@ -911,7 +915,7 @@ pub fn read_chunked_data_cached( .ok_or_else(|| FormatError::ChunkedReadError("no address for chunked layout".into()))?; let elem_size = datatype.type_size() as usize; - let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, dataspace, elem_size)?; + let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, version, dataspace, elem_size)?; let ds_dims: Vec = dataspace.dimensions.iter().map(|&d| d as usize).collect(); // The per-file cache is shared across datasets (and threads); every @@ -1199,12 +1203,13 @@ pub fn read_chunked_data_sweep( cache: &ChunkCache, sweep: &mut SweepContext, ) -> Result, FormatError> { - let (chunk_dimensions, addr_opt) = match layout { + let (chunk_dimensions, version, addr_opt) = match layout { DataLayout::Chunked { chunk_dimensions, + version, btree_address, .. - } => (chunk_dimensions, *btree_address), + } => (chunk_dimensions, *version, *btree_address), _ => { return Err(FormatError::ChunkedReadError( "expected chunked layout".into(), @@ -1216,7 +1221,7 @@ pub fn read_chunked_data_sweep( .ok_or_else(|| FormatError::ChunkedReadError("no address for chunked layout".into()))?; let elem_size = datatype.type_size() as usize; - let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, dataspace, elem_size)?; + let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, version, dataspace, elem_size)?; let ds_dims: Vec = dataspace.dimensions.iter().map(|&d| d as usize).collect(); // The per-file cache is shared across datasets (and threads); every @@ -1335,12 +1340,13 @@ pub fn read_chunked_data_indexed( length_size: u8, cache: &ChunkCache, ) -> Result, FormatError> { - let (chunk_dimensions, addr_opt) = match layout { + let (chunk_dimensions, version, addr_opt) = match layout { DataLayout::Chunked { chunk_dimensions, + version, btree_address, .. - } => (chunk_dimensions, *btree_address), + } => (chunk_dimensions, *version, *btree_address), _ => { return Err(FormatError::ChunkedReadError( "expected chunked layout".into(), @@ -1352,7 +1358,7 @@ pub fn read_chunked_data_indexed( .ok_or_else(|| FormatError::ChunkedReadError("no address for chunked layout".into()))?; let elem_size = datatype.type_size() as usize; - let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, dataspace, elem_size)?; + let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, version, dataspace, elem_size)?; let ds_dims: Vec = dataspace.dimensions.iter().map(|&d| d as usize).collect(); // Chunk index and assembly plan for this dataset, built on first access @@ -1845,31 +1851,41 @@ mod tests { dimensions: dims.to_vec(), max_dimensions: None, }; + for v in [3, 4] { + assert_eq!( + chunk_geometry(&[4, 5, 8], v, &space(&[10, 10]), 8).unwrap(), + (2, vec![4, 5]) + ); + // Rank mismatch. + assert!(matches!( + chunk_geometry(&[4, 8], v, &space(&[10, 10]), 8), + Err(FormatError::InvalidChunkDimensions(m)) if m.contains("doesn't match") + )); + // Zero dimension (a layout built in memory, bypassing the parser). + assert!(matches!( + chunk_geometry(&[4, 0, 8], v, &space(&[10, 10]), 8), + Err(FormatError::InvalidChunkDimensions(m)) if m.contains("must be > 0") + )); + assert!(chunk_geometry(&[0xFFFF_FFFF, 1], v, &space(&[10]), 1).is_ok()); + } + // With a v1 B-tree index (layout version 3) the largest chunk is + // 4 GiB - 1 bytes: 0x80000000 x 4-byte elements (8 GiB) is refused. + // These dims used to hang the reader. + assert!(matches!( + chunk_geometry(&[0x8000_0000, 4], 3, &space(&[10]), 4), + Err(FormatError::InvalidChunkDimensions(m)) if m.contains("4GB with v1 b-tree") + )); + assert!(matches!( + chunk_geometry(&[0xFFFF_FFFF, 0xFFFF_FFFF, 1], 3, &space(&[10, 10]), 1), + Err(FormatError::InvalidChunkDimensions(m)) if m.contains("4GB with v1 b-tree") + )); + // The other chunk indexes (layout version 4, and 5, which is read as + // 4) allow chunks of 4 GiB and more; HDF5 2.0 writes them. assert_eq!( - chunk_geometry(&[4, 5, 8], &space(&[10, 10]), 8).unwrap(), - (2, vec![4, 5]) + chunk_geometry(&[0x2000_0001, 8], 4, &space(&[10]), 8).unwrap(), + (1, vec![0x2000_0001]) ); - // Rank mismatch. - assert!(matches!( - chunk_geometry(&[4, 8], &space(&[10, 10]), 8), - Err(FormatError::InvalidChunkDimensions(m)) if m.contains("doesn't match") - )); - // Zero dimension (a layout built in memory, bypassing the parser). - assert!(matches!( - chunk_geometry(&[4, 0, 8], &space(&[10, 10]), 8), - Err(FormatError::InvalidChunkDimensions(m)) if m.contains("must be > 0") - )); - // 0x80000000 x 4-byte elements is 8 GiB; the largest allowed chunk - // is 4 GiB - 1 bytes. These dims used to hang the reader. - assert!(matches!( - chunk_geometry(&[0x8000_0000, 4], &space(&[10]), 4), - Err(FormatError::InvalidChunkDimensions(m)) if m.contains("4GB") - )); - assert!(matches!( - chunk_geometry(&[0xFFFF_FFFF, 0xFFFF_FFFF, 1], &space(&[10, 10]), 1), - Err(FormatError::InvalidChunkDimensions(m)) if m.contains("4GB") - )); - assert!(chunk_geometry(&[0xFFFF_FFFF, 1], &space(&[10]), 1).is_ok()); + assert!(chunk_geometry(&[0xFFFF_FFFF, 0xFFFF_FFFF, 1], 4, &space(&[10, 10]), 1).is_ok()); } fn make_f64_type() -> Datatype { diff --git a/crates/clawhdf5-format/src/data_read.rs b/crates/clawhdf5-format/src/data_read.rs index 91b739a..8583473 100644 --- a/crates/clawhdf5-format/src/data_read.rs +++ b/crates/clawhdf5-format/src/data_read.rs @@ -360,7 +360,7 @@ pub fn read_raw_data_selection( chunk_index_type, .. } => { - crate::chunked_read::chunk_geometry(chunk_dimensions, dataspace, elem_size)?; + crate::chunked_read::chunk_geometry(chunk_dimensions, *version, dataspace, elem_size)?; // For chunked data, only read chunks that intersect the selection let chunk_dims: Vec = chunk_dimensions.iter().map(|&d| d as u64).collect(); let rank = dims.len(); diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index b3cfd65..755a998 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -212,7 +212,7 @@ pub enum FormatError { InvalidDatatype(String), /// A chunked layout whose chunk dimensions libhdf5 refuses: a zero /// dimension, a rank that does not match the dataspace, or a chunk of - /// 4 GiB or more. + /// 4 GiB or more indexed by a version-1 B-tree. InvalidChunkDimensions(String), /// The superblock's end-of-file address lies past the end of the file: /// the file was truncated (libhdf5 refuses to open it). diff --git a/crates/clawhdf5/tests/header_validation_interop.rs b/crates/clawhdf5/tests/header_validation_interop.rs index 14dc9d7..daa3e19 100644 --- a/crates/clawhdf5/tests/header_validation_interop.rs +++ b/crates/clawhdf5/tests/header_validation_interop.rs @@ -269,3 +269,47 @@ save("float_overlap", bad) ], ); } + +/// Runs a Python script (with `h5py`, `numpy as np` and `struct` imported, +/// `d` the output directory) and fails the test if it fails. +fn run_python(dir: &Path, body: &str) { + let script = format!( + "import h5py, numpy as np, struct, os\nd = \"{}\"\n{body}", + dir.display() + ); + let out = Command::new(python()) + .args(["-c", &script]) + .output() + .expect("failed to run python"); + assert!( + out.status.success(), + "python failed:\n{}", + String::from_utf8_lossy(&out.stderr) + ); +} + +/// libhdf5 limits a chunk to under 4 GiB only when a version-1 B-tree +/// indexes it; HDF5 2.0 writes larger chunks with layout version 5 (libver +/// v200), and h5py reads them. These were refused as "chunk size must be < +/// 4GB". Ignored by default: h5py writes a 4 GiB chunk and both libraries +/// hold it in memory (about 9 GiB in all). +#[test] +#[ignore = "writes and reads a 4 GiB chunk (about 9 GiB of memory)"] +fn chunks_of_4_gib_and_more_read_with_layout_v5() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + run_python( + dir.path(), + r#" +with h5py.File(os.path.join(d, "big.h5"), "w", libver=("v200", "v200")) as f: + ds = f.create_dataset("d", shape=(10,), maxshape=(None,), chunks=(2**29 + 1,), + dtype=">()); +}