fix(format): refuse a chunk layout whose element size is not the datatype's
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"). clawhdf5 ignored the recorded size and read the chunks anyway, for v3 and v4 layouts. The check runs on every chunked read (read_chunked_data*, read_raw_data_selection) and compares against the stored size: a variable-length element is 4 + offset size + 4 bytes, not Datatype::type_size's 16. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -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<Vec<u8>, 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<usize> = 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<usize> = 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<usize> = dataspace.dimensions.iter().map(|&d| d as usize).collect();
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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="<i4"), chunks=(37,))
|
||||
data = bytearray(open(good, "rb").read())
|
||||
if libver == "earliest":
|
||||
at = data.find(struct.pack("<II", 37, 4)) + 4
|
||||
width = 4
|
||||
else:
|
||||
# v4: version, class, flags, ndims, bytes per dim, then the dims
|
||||
at = data.find(bytes([4, 2, 0, 2, 1, 37, 4])) + 6
|
||||
width = 1
|
||||
assert at > 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!();
|
||||
|
||||
Reference in New Issue
Block a user