fix(format): a chunk that decodes short is an error, not zero-filled

HDF5 stores every chunk at the full chunk size (edge chunks are padded
before filtering, and with "don't filter partial edge chunks" they are
stored raw at full size), so a filter pipeline that decodes to fewer bytes
means a corrupt chunk. Every chunk reader padded it with zeros and
returned it as data. libhdf5 returns the rest uninitialised, or fails when
the filter checks (Blosc with nbytes = 0).

New filters::decompress_chunk_exact decodes and then requires exactly the
chunk size, with the chunk's coordinates in the error
(ChunkedReadError "chunk at [16] decoded to 16 bytes, expected 32"). It
replaces decompress_chunk_masked at every chunk read path: the full read
(sequential and lane-partitioned), the cached read, the sweep read, the
planned-selection read, parallel_read's three decoders and partial_read's
box read. decompress_chunk_masked is unchanged (fractal-heap huge objects
already checked their own size). Blosc also rejects a frame declaring no
data where the chunk size is known.

Tests, each failing with the check disabled: filters and parallel_read
unit tests; h5py_short_decoded_chunk_is_an_error (gzip chunks rewritten
short with write_direct_chunk: 1-D, a 2-D edge chunk, and 40 chunks with
shuffle, read through File full/cached/selection reads, a selection that
avoids the chunk still reads, MmapFile and LazyFile, with and without the
parallel feature); plugin_filters_interop short_decoding_chunks_are_errors
(Blosc nbytes=0 and short, LZF and bzip2 short; the Blosc nbytes=0 case
read as 16 zeros before). The existing don't-filter-partial-edge-chunks
tests still pass. Conformance (tank, 2026-09-26): 573 of 697 ok, and no
file changed class, reader result or first issue against the pre-fix run.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
osobh
2026-09-26 01:22:26 -05:00
co-authored by Claude Opus 5.5
parent 7f52a6f3ba
commit a5bd70216c
7 changed files with 289 additions and 13 deletions
+9 -5
View File
@@ -15,7 +15,7 @@ use crate::datatype::Datatype;
use crate::error::FormatError;
use crate::extensible_array::{ExtensibleArrayHeader, read_extensible_array_chunks};
use crate::filter_pipeline::FilterPipeline;
use crate::filters::{all_filters_skipped, decompress_chunk_masked};
use crate::filters::{all_filters_skipped, decompress_chunk_exact};
use crate::fixed_array::{FixedArrayHeader, read_fixed_array_chunks};
#[cfg(feature = "std")]
use std::sync::Arc;
@@ -65,12 +65,13 @@ fn decompress_all_chunks(
let raw_chunk = &file_data[c_addr..c_addr + size];
let decompressed = if let Some(pl) = pipeline {
decompress_chunk_masked(
decompress_chunk_exact(
raw_chunk,
pl,
chunk_total_bytes,
element_size,
chunk_info.filter_mask,
&chunk_info.offsets,
)?
} else {
raw_chunk.to_vec()
@@ -932,12 +933,13 @@ pub fn read_chunked_data_cached(
let cache_them = total_bytes <= cache.max_bytes();
if let Some(pl) = pipeline {
let decode = |c: &&ChunkInfo| -> Result<Vec<u8>, FormatError> {
decompress_chunk_masked(
decompress_chunk_exact(
raw_bytes(c)?,
pl,
chunk_total_bytes,
elem_size as u32,
c.filter_mask,
&c.offsets,
)
};
for batch in misses.chunks(DECODE_BATCH) {
@@ -1217,12 +1219,13 @@ pub fn read_chunked_data_sweep(
ensure_len(file_data, c_addr, size)?;
let raw_chunk = &file_data[c_addr..c_addr + size];
let dec = if let Some(pl) = pipeline {
decompress_chunk_masked(
decompress_chunk_exact(
raw_chunk,
pl,
chunk_total_bytes,
elem_size as u32,
chunk_info.filter_mask,
&coord,
)?
} else {
raw_chunk.to_vec()
@@ -1346,12 +1349,13 @@ pub fn read_chunked_data_indexed(
ensure_len(file_data, c_addr, size)?;
let raw_chunk = &file_data[c_addr..c_addr + size];
let decompressed = if let Some(pl) = pipeline {
decompress_chunk_masked(
decompress_chunk_exact(
raw_chunk,
pl,
chunk_total_bytes,
elem_size as u32,
*filter_mask,
coord,
)?
} else {
raw_chunk.to_vec()
+62 -1
View File
@@ -4,7 +4,7 @@
extern crate alloc;
#[cfg(not(feature = "std"))]
use alloc::{boxed::Box, vec, vec::Vec};
use alloc::{boxed::Box, format, vec, vec::Vec};
use crate::error::FormatError;
#[cfg(feature = "deflate")]
@@ -120,6 +120,34 @@ pub fn decompress_chunk_masked(
Ok(data)
}
/// Decode one stored chunk of a chunked dataset: [`decompress_chunk_masked`],
/// then require exactly `chunk_size` bytes (when `chunk_size` is known).
///
/// HDF5 stores every chunk at the full chunk size — edge chunks are padded
/// before they are filtered, and an edge chunk left unfiltered is written
/// full-size too — so a pipeline that decodes to fewer bytes means a
/// corrupt chunk. libhdf5 fails such a read (or returns uninitialised
/// memory); it must never read back as zeros. `coords` (the chunk's offset
/// in the dataset) is named in the error.
pub fn decompress_chunk_exact(
compressed: &[u8],
pipeline: &FilterPipeline,
chunk_size: usize,
element_size: u32,
filter_mask: u32,
coords: &[u64],
) -> Result<Vec<u8>, FormatError> {
let data =
decompress_chunk_masked(compressed, pipeline, chunk_size, element_size, filter_mask)?;
if chunk_size != 0 && data.len() != chunk_size {
return Err(FormatError::ChunkedReadError(format!(
"chunk at {coords:?} decoded to {} bytes, expected {chunk_size}",
data.len()
)));
}
Ok(data)
}
/// Apply a filter pipeline to compress a chunk.
/// Filters are applied in FORWARD order for compression.
pub fn compress_chunk(
@@ -1516,6 +1544,39 @@ fn pcodec_decompress(
#[cfg(test)]
mod tests {
/// A chunk whose pipeline decodes to fewer bytes than the chunk holds is
/// an error naming the chunk, never a short buffer the reader pads.
#[test]
fn short_decoded_chunk_is_an_error() {
let pipeline = FilterPipeline {
version: 2,
filters: vec![FilterDescription {
filter_id: FILTER_SHUFFLE,
name: None,
flags: 0,
client_data: vec![4],
}],
};
let full = [7u8; 32];
assert_eq!(
decompress_chunk_exact(&full, &pipeline, 32, 4, 0, &[8]).unwrap(),
full
);
let err = decompress_chunk_exact(&full[..16], &pipeline, 32, 4, 0, &[8, 0]).unwrap_err();
let msg = err.to_string();
assert!(
msg.contains("[8, 0]") && msg.contains("16") && msg.contains("32"),
"{msg}"
);
// Every filter skipped: the stored bytes are the chunk, still checked.
assert!(decompress_chunk_exact(&full[..16], &pipeline, 32, 4, 1, &[0]).is_err());
// Unknown chunk size: not checked.
assert_eq!(
decompress_chunk_exact(&full[..16], &pipeline, 0, 4, 0, &[0]).unwrap(),
&full[..16]
);
}
use super::*;
use crate::filter_pipeline::FilterDescription;
+25 -1
View File
@@ -113,8 +113,15 @@ fn decode_stream(
}
/// Decode a Blosc-filtered chunk: one Blosc 1 frame.
///
/// An HDF5 chunk is never empty, so a frame that decodes to nothing where
/// the chunk size is known is corrupt (libhdf5's filter fails it too).
pub(crate) fn blosc_decode(input: &[u8], ctx: &FilterContext<'_>) -> Result<Vec<u8>, FormatError> {
blosc_decompress(input, ctx.output_limit())
let out = blosc_decompress(input, ctx.output_limit())?;
if out.is_empty() && ctx.max_output != 0 {
return Err(err("empty frame for a non-empty chunk"));
}
Ok(out)
}
/// Decompress a Blosc 1 frame, refusing more than `limit` bytes of output.
@@ -606,6 +613,23 @@ mod tests {
assert!(blosc_encode(&data, &ctx0).is_err());
}
/// A frame that declares no data, for a chunk that has some.
#[test]
fn empty_frame_for_a_non_empty_chunk_is_an_error() {
let mut frame = vec![2u8, 1, 0x20, 4];
for v in [0u32, 64, 16] {
frame.extend_from_slice(&v.to_le_bytes());
}
assert_eq!(blosc_decompress(&frame, 64).unwrap(), b"");
let f = desc(vec![2, 2, 4, 64, 5, 1, 1]);
let ctx = FilterContext {
filter: &f,
element_size: 4,
max_output: 64,
};
assert!(blosc_decode(&frame, &ctx).is_err());
}
/// A frame whose header claims a compressed size smaller than the
/// header itself, not stored raw: an error, not an arithmetic overflow
/// (it panicked in debug builds).
+64 -4
View File
@@ -10,7 +10,7 @@
use crate::chunked_read::ChunkInfo;
use crate::error::FormatError;
use crate::filter_pipeline::FilterPipeline;
use crate::filters::decompress_chunk_masked;
use crate::filters::decompress_chunk_exact;
use crate::lane_partition::{self, LaneStats, PartitionStats};
/// Threshold: only use parallel decompression when chunk count exceeds this.
@@ -84,12 +84,13 @@ pub fn decompress_chunks_lane_partitioned(
}
let raw_chunk = &file_data[c_addr..c_addr + size];
let decompressed = decompress_chunk_masked(
let decompressed = decompress_chunk_exact(
raw_chunk,
pipeline,
chunk_total_bytes,
element_size,
chunk_info.filter_mask,
&chunk_info.offsets,
)?;
stats.chunks_processed += 1;
@@ -160,12 +161,13 @@ pub fn decompress_chunks_parallel(
}
let raw_chunk = &file_data[c_addr..c_addr + size];
let decompressed = decompress_chunk_masked(
let decompressed = decompress_chunk_exact(
raw_chunk,
pipeline,
chunk_total_bytes,
element_size,
chunk_info.filter_mask,
&chunk_info.offsets,
)?;
Ok(DecompressedChunk {
@@ -204,12 +206,13 @@ pub fn decompress_chunks_sequential(
let raw_chunk = &file_data[c_addr..c_addr + size];
let decompressed = if let Some(pl) = pipeline {
decompress_chunk_masked(
decompress_chunk_exact(
raw_chunk,
pl,
chunk_total_bytes,
element_size,
chunk_info.filter_mask,
&chunk_info.offsets,
)?
} else {
raw_chunk.to_vec()
@@ -218,3 +221,60 @@ pub fn decompress_chunks_sequential(
}
Ok(result)
}
#[cfg(test)]
mod tests {
use super::*;
use crate::filter_pipeline::{FILTER_SHUFFLE, FilterDescription};
/// Eight shuffled 32-byte chunks; chunk 5 is stored short when `short`.
fn chunks(short: bool) -> (Vec<u8>, Vec<ChunkInfo>) {
let mut file = Vec::new();
let mut infos = Vec::new();
for i in 0..8u64 {
let len = if short && i == 5 { 16 } else { 32 };
infos.push(ChunkInfo {
chunk_size: len as u32,
filter_mask: 0,
offsets: vec![i * 8],
address: file.len() as u64,
});
file.extend(core::iter::repeat_n(i as u8, len));
}
(file, infos)
}
/// Every parallel decoder refuses a chunk that decodes short, naming it.
#[test]
fn short_decoded_chunk_is_an_error() {
let pipeline = FilterPipeline {
version: 2,
filters: vec![FilterDescription {
filter_id: FILTER_SHUFFLE,
name: None,
flags: 0,
client_data: vec![4],
}],
};
let (file, good) = chunks(false);
assert_eq!(
decompress_chunks_parallel(&file, &good, &pipeline, 32, 4).unwrap()[5],
[5u8; 32]
);
let (file, bad) = chunks(true);
let errs = [
decompress_chunks_lane_partitioned(&file, &bad, &pipeline, 32, 4, 1, Some(3))
.map(|_| ())
.unwrap_err(),
decompress_chunks_parallel(&file, &bad, &pipeline, 32, 4)
.map(|_| ())
.unwrap_err(),
decompress_chunks_sequential(&file, &bad, Some(&pipeline), 32, 4)
.map(|_| ())
.unwrap_err(),
];
for e in errs {
assert!(e.to_string().contains("[40]"), "{e}");
}
}
}
+3 -2
View File
@@ -22,7 +22,7 @@ use crate::data_read::extract_selection_from_buffer;
use crate::dataspace::Dataspace;
use crate::error::FormatError;
use crate::filter_pipeline::FilterPipeline;
use crate::filters::{all_filters_skipped, decompress_chunk_masked};
use crate::filters::{all_filters_skipped, decompress_chunk_exact};
use crate::selection::Selection;
/// The smallest axis-aligned box containing every selected element, as
@@ -330,12 +330,13 @@ pub fn read_selection(
let decoded;
let data: &[u8] = match pipeline {
Some(pl) if !all_filters_skipped(pl, chunk.filter_mask) => {
decoded = decompress_chunk_masked(
decoded = decompress_chunk_exact(
raw,
pl,
chunk_bytes,
elem_size as u32,
chunk.filter_mask,
&chunk.offsets[..rank],
)?;
&decoded
}