diff --git a/CHANGELOG.md b/CHANGELOG.md index c20f408..97bfb97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -335,7 +335,9 @@ dimension, a chunk rank that does not match the dataspace, a chunk of 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 + larger chunks, and HDF5 2.0 writes them), an element size in the + layout that differs from the datatype's stored size (the chunks were + laid out with the wrong element size), 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 7abf021..afff80d 100644 --- a/crates/clawhdf5-format/src/chunked_read.rs +++ b/crates/clawhdf5-format/src/chunked_read.rs @@ -191,6 +191,53 @@ pub(crate) fn chunk_geometry( Ok((rank, spatial.iter().map(|&c| c as usize).collect())) } +/// The size of one element of `dt` as stored in the file: a +/// variable-length element is its length (4), a global heap address +/// (`offset_size`) and an index (4), not the 16 of [`Datatype::type_size`]. +fn stored_element_size(dt: &Datatype, offset_size: u8) -> u64 { + match dt { + Datatype::VariableLength { .. } => 8 + u64::from(offset_size), + Datatype::Array { + base_type, + dimensions, + } => dimensions + .iter() + .fold(stored_element_size(base_type, offset_size), |acc, &d| { + acc.saturating_mul(u64::from(d)) + }), + _ => u64::from(dt.type_size()), + } +} + +/// A chunked layout records the element size as its last dimension, and +/// libhdf5 refuses a dataset whose datatype has another size +/// (`H5D__chunk_set_sizes`: "stored datatype size in chunk layout does not +/// match datatype description"). Reading it anyway laid the chunks out with +/// the wrong element size. +pub(crate) fn check_chunk_element_size( + layout: &DataLayout, + datatype: &Datatype, + offset_size: u8, +) -> Result<(), FormatError> { + let DataLayout::Chunked { + chunk_dimensions, .. + } = layout + else { + return Ok(()); + }; + let Some(&stored) = chunk_dimensions.last() else { + return Ok(()); + }; + let expected = stored_element_size(datatype, offset_size); + if u64::from(stored) != expected { + return Err(FormatError::InvalidChunkDimensions(format!( + "stored datatype size in chunk layout does not match datatype description \ + (layout {stored} bytes, datatype {expected})" + ))); + } + Ok(()) +} + /// Product of chunk dimensions times the element size, overflow-checked. pub(crate) fn checked_chunk_byte_len( chunk_dims: &[usize], @@ -774,6 +821,7 @@ pub fn read_chunked_data( offset_size: u8, length_size: u8, ) -> Result, FormatError> { + check_chunk_element_size(layout, datatype, offset_size)?; let elem_size = datatype.type_size() as usize; let (chunks, chunk_dims) = list_chunks( file_data, @@ -914,6 +962,7 @@ pub fn read_chunked_data_cached( let addr = addr_opt .ok_or_else(|| FormatError::ChunkedReadError("no address for chunked layout".into()))?; + check_chunk_element_size(layout, datatype, offset_size)?; let elem_size = datatype.type_size() as usize; 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(); @@ -1220,6 +1269,7 @@ pub fn read_chunked_data_sweep( let addr = addr_opt .ok_or_else(|| FormatError::ChunkedReadError("no address for chunked layout".into()))?; + check_chunk_element_size(layout, datatype, offset_size)?; let elem_size = datatype.type_size() as usize; 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(); @@ -1357,6 +1407,7 @@ pub fn read_chunked_data_indexed( let addr = addr_opt .ok_or_else(|| FormatError::ChunkedReadError("no address for chunked layout".into()))?; + check_chunk_element_size(layout, datatype, offset_size)?; let elem_size = datatype.type_size() as usize; 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(); diff --git a/crates/clawhdf5-format/src/data_read.rs b/crates/clawhdf5-format/src/data_read.rs index 8583473..b8f2ffe 100644 --- a/crates/clawhdf5-format/src/data_read.rs +++ b/crates/clawhdf5-format/src/data_read.rs @@ -303,6 +303,7 @@ pub fn read_raw_data_selection( use crate::selection::Selection; crate::partial_read::validate(selection, &dataspace.dimensions)?; + crate::chunked_read::check_chunk_element_size(layout, datatype, offset_size)?; // Read only what the selection's bounding box touches when that is // possible; everything below is the decode-everything-then-pick path, diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index 755a998..a9a0c2b 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -211,8 +211,9 @@ pub enum FormatError { /// an empty enum name, a compound member outside its compound, … 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 indexed by a version-1 B-tree. + /// dimension, a rank that does not match the dataspace, an element size + /// that is not the datatype's, or a chunk of 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 a2cebf5..7cdd4b4 100644 --- a/crates/clawhdf5/tests/header_validation_interop.rs +++ b/crates/clawhdf5/tests/header_validation_interop.rs @@ -156,6 +156,102 @@ for libver in ("earliest", "latest"): ); } +/// Python: `fix_ohdr(buf, off)` recomputes the Jenkins lookup3 checksum of +/// the version-2 object header chunk 0 at `off`, so a field can be changed +/// in a `libver="latest"` file without the checksum failing first. +const FIX_OHDR_PY: &str = r#" +def _rot(x, k): + return ((x << k) | (x >> (32 - k))) & 0xFFFFFFFF +def lookup3(data): + M = 0xFFFFFFFF + n = len(data); a = b = c = (0xDEADBEEF + n) & M; i = 0 + w = lambda j: int.from_bytes(data[j:j + 4], "little") + while n > 12: + a = (a + w(i)) & M; b = (b + w(i + 4)) & M; c = (c + w(i + 8)) & M + a = (a - c) & M; a ^= _rot(c, 4); c = (c + b) & M + b = (b - a) & M; b ^= _rot(a, 6); a = (a + c) & M + c = (c - b) & M; c ^= _rot(b, 8); b = (b + a) & M + a = (a - c) & M; a ^= _rot(c, 16); c = (c + b) & M + b = (b - a) & M; b ^= _rot(a, 19); a = (a + c) & M + c = (c - b) & M; c ^= _rot(b, 4); b = (b + a) & M + n -= 12; i += 12 + if n == 0: + return c + t = bytes(data[i:]) + bytes(12) + w = lambda j: int.from_bytes(t[j:j + 4], "little") + a = (a + w(0)) & M; b = (b + w(4)) & M; c = (c + w(8)) & M + c ^= b; c = (c - _rot(b, 14)) & M + a ^= c; a = (a - _rot(c, 11)) & M + b ^= a; b = (b - _rot(a, 25)) & M + c ^= b; c = (c - _rot(b, 16)) & M + a ^= c; a = (a - _rot(c, 4)) & M + b ^= a; b = (b - _rot(a, 14)) & M + c ^= b; c = (c - _rot(b, 24)) & M + return c +def fix_ohdr(buf, off): + assert buf[off:off + 4] == b"OHDR" + flags = buf[off + 5]; p = off + 6 + if flags & 0x20: p += 16 + if flags & 0x10: p += 4 + width = 1 << (flags & 3) + end = p + width + int.from_bytes(buf[p:p + width], "little") + buf[end:end + 4] = lookup3(bytes(buf[off:end])).to_bytes(4, "little") +"#; + +/// A chunked layout records the element size as its last dimension; libhdf5 +/// refuses a dataset whose datatype has another size ("stored datatype size +/// in chunk layout does not match datatype description"). This read the +/// chunks laid out with the wrong element size. +#[test] +fn chunk_layout_element_size_must_match_the_datatype() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let body = format!( + "{FIX_OHDR_PY}{}", + r#" +for libver in ("earliest", "latest"): + good = os.path.join(d, f"{libver}_good.h5") + with h5py.File(good, "w", libver=libver) as f: + f.create_dataset("d", data=np.arange(100, dtype=" 6 + for size in (2, 8): + bad = bytearray(data) + bad[at:at + width] = size.to_bytes(width, "little") + if libver == "latest": + fix_ohdr(bad, bad.rfind(b"OHDR", 0, at)) + open(os.path.join(d, f"{libver}_size{size}.h5"), "wb").write(bad) +"# + ); + let verdicts = h5py_verdicts(dir.path(), &body); + assert_agrees_with_h5py( + dir.path(), + &verdicts, + &[ + "earliest_good ok", + "earliest_size2 ERROR", + "earliest_size8 ERROR", + "latest_good ok", + "latest_size2 ERROR", + "latest_size8 ERROR", + ], + ); + for name in ["earliest_size2", "latest_size8"] { + let err = clawhdf5_reads(&dir.path().join(format!("{name}.h5"))).unwrap_err(); + assert!( + err.contains("stored datatype size in chunk layout"), + "{name}: {err}" + ); + } +} + #[test] fn truncated_files_are_refused_and_nothing_past_the_end_of_file_is_read() { skip_if_no_python!();