Fix silent wrong data and libhdf5 interop found by the HDF5 audit #11

Merged
osobh merged 41 commits from fix/phase0-correctness into main 2026-09-26 02:42:54 +00:00
5 changed files with 167 additions and 1 deletions
Showing only changes of commit d074385944 - Show all commits
+26 -1
View File
@@ -519,6 +519,7 @@ pub fn list_chunks(
addr_opt, addr_opt,
single_filtered_size, single_filtered_size,
single_filter_mask, single_filter_mask,
unfiltered_edges,
) = match layout { ) = match layout {
DataLayout::Chunked { DataLayout::Chunked {
chunk_dimensions, chunk_dimensions,
@@ -527,6 +528,7 @@ pub fn list_chunks(
chunk_index_type, chunk_index_type,
single_chunk_filtered_size, single_chunk_filtered_size,
single_chunk_filter_mask, single_chunk_filter_mask,
dont_filter_partial_edge_chunks,
} => ( } => (
chunk_dimensions, chunk_dimensions,
*version, *version,
@@ -534,6 +536,7 @@ pub fn list_chunks(
*btree_address, *btree_address,
*single_chunk_filtered_size, *single_chunk_filtered_size,
*single_chunk_filter_mask, *single_chunk_filter_mask,
*dont_filter_partial_edge_chunks,
), ),
_ => { _ => {
return Err(FormatError::ChunkedReadError( return Err(FormatError::ChunkedReadError(
@@ -566,7 +569,7 @@ pub fn list_chunks(
} }
// Collect chunks based on version and index type // Collect chunks based on version and index type
let chunks = match (version, chunk_index_type) { let mut chunks = match (version, chunk_index_type) {
(3, _) => { (3, _) => {
let ndims = chunk_dimensions.len(); // rank+1 let ndims = chunk_dimensions.len(); // rank+1
collect_chunk_info(file_data, addr, ndims, offset_size, length_size)? collect_chunk_info(file_data, addr, ndims, offset_size, length_size)?
@@ -645,6 +648,23 @@ pub fn list_chunks(
} }
}; };
// With "don't filter partial edge chunks", a chunk that extends past the
// dataset's extent is stored raw while its filter mask still reads 0.
// Mark every filter skipped so all read paths copy it as-is.
if unfiltered_edges {
for chunk in &mut chunks {
let partial = chunk
.offsets
.iter()
.zip(&chunk_dims)
.zip(&ds_dims)
.any(|((&off, &cd), &dd)| off.saturating_add(cd as u64) > dd as u64);
if partial {
chunk.filter_mask = u32::MAX;
}
}
}
Ok((chunks, chunk_dims)) Ok((chunks, chunk_dims))
} }
@@ -1829,6 +1849,7 @@ mod tests {
chunk_index_type: None, chunk_index_type: None,
single_chunk_filtered_size: None, single_chunk_filtered_size: None,
single_chunk_filter_mask: None, single_chunk_filter_mask: None,
dont_filter_partial_edge_chunks: false,
}; };
let dataspace = Dataspace { let dataspace = Dataspace {
@@ -1852,6 +1873,7 @@ mod tests {
chunk_index_type: None, chunk_index_type: None,
single_chunk_filtered_size: None, single_chunk_filtered_size: None,
single_chunk_filter_mask: None, single_chunk_filter_mask: None,
dont_filter_partial_edge_chunks: false,
}; };
let dataspace = Dataspace { let dataspace = Dataspace {
space_type: DataspaceType::Simple, space_type: DataspaceType::Simple,
@@ -2009,6 +2031,7 @@ mod tests {
chunk_index_type: None, chunk_index_type: None,
single_chunk_filtered_size: None, single_chunk_filtered_size: None,
single_chunk_filter_mask: None, single_chunk_filter_mask: None,
dont_filter_partial_edge_chunks: false,
}; };
let dataspace = Dataspace { let dataspace = Dataspace {
space_type: DataspaceType::Simple, space_type: DataspaceType::Simple,
@@ -2091,6 +2114,7 @@ mod tests {
chunk_index_type: None, chunk_index_type: None,
single_chunk_filtered_size: None, single_chunk_filtered_size: None,
single_chunk_filter_mask: None, single_chunk_filter_mask: None,
dont_filter_partial_edge_chunks: false,
}; };
let dataspace = Dataspace { let dataspace = Dataspace {
space_type: DataspaceType::Simple, space_type: DataspaceType::Simple,
@@ -2253,6 +2277,7 @@ mod tests {
chunk_index_type: Some(1), chunk_index_type: Some(1),
single_chunk_filtered_size: None, single_chunk_filtered_size: None,
single_chunk_filter_mask: None, single_chunk_filter_mask: None,
dont_filter_partial_edge_chunks: false,
}; };
let dataspace = Dataspace { let dataspace = Dataspace {
space_type: DataspaceType::Simple, space_type: DataspaceType::Simple,
@@ -1314,6 +1314,7 @@ mod tests {
chunk_index_type, chunk_index_type,
single_chunk_filtered_size, single_chunk_filtered_size,
single_chunk_filter_mask, single_chunk_filter_mask,
..
} => { } => {
assert_eq!(version, 4); assert_eq!(version, 4);
assert_eq!(chunk_index_type, Some(1)); assert_eq!(chunk_index_type, Some(1));
+34
View File
@@ -53,6 +53,11 @@ pub enum DataLayout {
single_chunk_filtered_size: Option<u64>, single_chunk_filtered_size: Option<u64>,
/// Filter mask for v4 single chunk with filters. /// Filter mask for v4 single chunk with filters.
single_chunk_filter_mask: Option<u32>, single_chunk_filter_mask: Option<u32>,
/// Layout v4 flag bit 0 (`H5D_CHUNK_DONT_FILTER_PARTIAL_CHUNKS`):
/// partial edge chunks — those extending past the dataset's current
/// extent in some dimension — are stored without the filter pipeline,
/// even though their filter mask is 0. Always `false` for v3.
dont_filter_partial_edge_chunks: bool,
}, },
/// Virtual dataset layout (v4 only). /// Virtual dataset layout (v4 only).
Virtual { Virtual {
@@ -322,6 +327,7 @@ impl DataLayout {
chunk_index_type: None, chunk_index_type: None,
single_chunk_filtered_size: None, single_chunk_filtered_size: None,
single_chunk_filter_mask: None, single_chunk_filter_mask: None,
dont_filter_partial_edge_chunks: false,
}) })
} }
_ => Err(FormatError::InvalidLayoutClass(layout_class)), _ => Err(FormatError::InvalidLayoutClass(layout_class)),
@@ -505,6 +511,7 @@ impl DataLayout {
chunk_index_type: Some(chunk_index_type), chunk_index_type: Some(chunk_index_type),
single_chunk_filtered_size, single_chunk_filtered_size,
single_chunk_filter_mask, single_chunk_filter_mask,
dont_filter_partial_edge_chunks: flags & 0x01 != 0,
}) })
} }
3 => { 3 => {
@@ -602,6 +609,7 @@ mod tests {
chunk_index_type: None, chunk_index_type: None,
single_chunk_filtered_size: None, single_chunk_filtered_size: None,
single_chunk_filter_mask: None, single_chunk_filter_mask: None,
dont_filter_partial_edge_chunks: false,
} }
); );
} }
@@ -679,10 +687,35 @@ mod tests {
chunk_index_type: Some(1), chunk_index_type: Some(1),
single_chunk_filtered_size: None, single_chunk_filtered_size: None,
single_chunk_filter_mask: None, single_chunk_filter_mask: None,
dont_filter_partial_edge_chunks: false,
} }
); );
} }
#[test]
fn v4_chunked_dont_filter_partial_edge_chunks_flag() {
let mut buf = vec![4u8, 2]; // version=4, class=2
buf.push(0x01); // flags bit 0 = don't filter partial edge chunks
buf.push(2); // dimensionality=2
buf.push(4); // dim_size_encoded_length=4
buf.extend_from_slice(&5u32.to_le_bytes());
buf.extend_from_slice(&4u32.to_le_bytes());
buf.push(3); // Fixed Array
buf.push(10); // max_dblk_page_nelmts_bits
buf.extend_from_slice(&0x3000u64.to_le_bytes());
match DataLayout::parse(&buf, 8, 8).unwrap() {
DataLayout::Chunked {
dont_filter_partial_edge_chunks,
btree_address,
..
} => {
assert!(dont_filter_partial_edge_chunks);
assert_eq!(btree_address, Some(0x3000));
}
other => panic!("expected Chunked, got {other:?}"),
}
}
#[test] #[test]
fn v4_chunked_single_chunk_with_filters() { fn v4_chunked_single_chunk_with_filters() {
let mut buf = vec![4u8, 2]; // version=4, class=2 let mut buf = vec![4u8, 2]; // version=4, class=2
@@ -705,6 +738,7 @@ mod tests {
chunk_index_type: Some(1), chunk_index_type: Some(1),
single_chunk_filtered_size: Some(1024), single_chunk_filtered_size: Some(1024),
single_chunk_filter_mask: Some(0), single_chunk_filter_mask: Some(0),
dont_filter_partial_edge_chunks: false,
} }
); );
} }
Binary file not shown.
@@ -237,3 +237,109 @@ with h5py.File("{p}", "w") as f:
grid grid
); );
} }
// ---------------------------------------------------------------------------
// "Don't filter partial edge chunks"
// ---------------------------------------------------------------------------
/// libhdf5's own test file for `H5Pset_chunk_opts(H5D_CHUNK_DONT_FILTER_PARTIAL_CHUNKS)`:
/// a 12x6 f32 dataset in 5x5 gzip chunks whose edge chunks are stored raw
/// with a filter mask of 0. Reading it tried to inflate the raw edge chunks
/// ("deflate: ... unknown compression method").
#[test]
fn libhdf5_edge_chunk_fixture_reads() {
let path = concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/fixtures/h5fc_edge_v3.h5"
);
let file = File::open(path).unwrap();
let ds = file.dataset("DSET_EDGE").unwrap();
assert_eq!(ds.shape().unwrap(), vec![12, 6]);
assert_eq!(ds.read_f32().unwrap(), vec![100.0f32; 72]);
}
/// The same layout flag on datasets with varied contents, set through the
/// libhdf5 that h5py ships (h5py has no binding for `H5Pset_chunk_opts`).
#[test]
fn h5py_unfiltered_partial_edge_chunks_read() {
skip_if_no_python!();
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("edge.h5");
let p = path.display().to_string();
let script = format!(
r#"
import ctypes, glob, os, sys
import h5py, numpy as np
here = os.path.dirname(h5py.__file__)
libs = glob.glob(os.path.join(here, "..", "h5py.libs", "libhdf5-*.so*"))
libs += glob.glob(os.path.join(here, ".dylibs", "libhdf5*.dylib"))
if not libs:
print("NO_LIBHDF5")
sys.exit(0)
lib = ctypes.CDLL(libs[0])
lib.H5Pset_chunk_opts.argtypes = [ctypes.c_int64, ctypes.c_uint]
def make(f, name, data, chunk, maxshape, shuffle):
dcpl = h5py.h5p.create(h5py.h5p.DATASET_CREATE)
dcpl.set_chunk(chunk)
if shuffle:
dcpl.set_shuffle()
dcpl.set_deflate(6)
assert lib.H5Pset_chunk_opts(dcpl.id, 0x0002) >= 0
space = h5py.h5s.create_simple(data.shape, maxshape)
d = h5py.h5d.create(f.id, name.encode(), h5py.h5t.py_create(data.dtype), space, dcpl=dcpl)
d.write(h5py.h5s.ALL, h5py.h5s.ALL, np.ascontiguousarray(data))
with h5py.File("{p}", "w") as f:
line = np.sin(np.arange(1000) / 7.0)
grid = np.arange(37 * 53, dtype="<f8").reshape(37, 53) * 0.5
make(f, "fixed_1d", line, (64,), None, False)
make(f, "ea_1d", line, (64,), (h5py.h5s.UNLIMITED,), True)
make(f, "bt2_2d", grid, (8, 8), (h5py.h5s.UNLIMITED,) * 2, True)
make(f, "fixed_2d", grid, (8, 8), None, True)
print("OK")
"#
);
let out = Command::new(python())
.args(["-c", &script])
.output()
.expect("failed to run python3");
assert!(
out.status.success(),
"{}",
String::from_utf8_lossy(&out.stderr)
);
if String::from_utf8_lossy(&out.stdout).contains("NO_LIBHDF5") {
eprintln!("SKIP: h5py's bundled libhdf5 not found (needed for H5Pset_chunk_opts)");
return;
}
let file = File::open(&path).unwrap();
let line: Vec<f64> = (0..1000).map(|i| (f64::from(i) / 7.0).sin()).collect();
for name in ["fixed_1d", "ea_1d"] {
let values = file.dataset(name).unwrap().read_f64().unwrap();
assert_eq!(values.len(), line.len(), "{name}");
for (i, (v, w)) in values.iter().zip(&line).enumerate() {
assert!((v - w).abs() < 1e-12, "{name}[{i}]: {v} vs {w}");
}
}
let grid: Vec<f64> = (0..37 * 53).map(|i| f64::from(i) * 0.5).collect();
for name in ["bt2_2d", "fixed_2d"] {
assert_eq!(
file.dataset(name).unwrap().read_f64().unwrap(),
grid,
"{name}"
);
}
// A selection touching only the last (partial, unfiltered) chunk.
let tail = file
.dataset("fixed_1d")
.unwrap()
.read_selection(&Selection::Hyperslab {
start: vec![990],
stride: vec![1],
count: vec![1],
block: vec![10],
})
.unwrap();
let want: Vec<u8> = line[990..].iter().flat_map(|v| v.to_le_bytes()).collect();
assert_eq!(tail, want);
}