diff --git a/CHANGELOG.md b/CHANGELOG.md index 75de94f..a658a30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,32 @@ ## Unreleased +### Correctness +- `clawhdf5-format`: **datasets indexed by an Extensible Array returned wrong + data beyond their first few dozen chunks.** One unlimited dimension gives a + dataset an Extensible Array chunk index, whose first elements (4 by default) + sit inline in the index block and whose rest live in data blocks sized by a + formula the reader got wrong. In the default layout everything through the + 36th chunk happened to line up and the 37th onwards did not: a 400-chunk + dataset silently returned wrong values from chunk 37, and datasets past + about a thousand chunks failed outright with "invalid Extensible Array data + block signature". **Reads were wrong, not + merely refused** — the caller got plausible numbers from the wrong chunks. + Four separate layout errors, each checked against files written by HDF5 2.0 + and against the library source: + - the number of data blocks in super block `u` is `2^(u/2)`, not `2^u`; + - each holds `2^((u+1)/2) * data_blk_min_elmts` elements, which doubles + every *other* level rather than every level; + - a super block carries a block-offset field before its data block + addresses, which was not skipped; + - the page-init bitmap belongs to the super block, one bit per page packed + across all its data blocks (MSB first), and was being read from inside the + data block instead; a paged data block also ends its prefix with a + checksum before the first page. + Covered now by interop tests at 4, 37, 400, 5 000 and 200 000 chunks (the + last large enough for paged data blocks), plus sparse, gzip-filtered and + 2-D cases. Writing is unaffected; this is a read-path bug. + ### Security - `clawhdf5-format`: **a crafted file could crash any reader through B-tree v2 traversal.** Recursion was bounded only by the depth the file claimed (a diff --git a/crates/clawhdf5-format/src/extensible_array.rs b/crates/clawhdf5-format/src/extensible_array.rs index 84add79..c3665d2 100644 --- a/crates/clawhdf5-format/src/extensible_array.rs +++ b/crates/clawhdf5-format/src/extensible_array.rs @@ -270,6 +270,40 @@ fn index_to_chunk_offsets( /// Collect elements from a data block at the given offset. #[allow(clippy::too_many_arguments)] +/// Layout of super block `u`, per the HDF5 spec: the number of data blocks it +/// owns and how many elements each of them holds. +/// +/// `ndblks` and `dblk_nelmts` each double every *other* level, a half-step +/// apart, so the blocks grow as 1x16, 1x32, 2x32, 2x64, 4x64 ... for a +/// 16-element minimum. Treating either as doubling every level (the previous +/// implementation) puts every element after the first data block at the wrong +/// index. +fn sblk_info(u: usize, data_blk_min_elmts: usize) -> Option<(usize, usize)> { + let ndblks = 1usize.checked_shl((u / 2) as u32)?; + let dblk_nelmts = 1usize + .checked_shl(u.div_ceil(2) as u32)? + .checked_mul(data_blk_min_elmts)?; + Some((ndblks, dblk_nelmts)) +} + +/// Width of the "offset of the block in the array" field carried by super and +/// data blocks (`hdr->arr_off_size`). +fn arr_off_size(header: &ExtensibleArrayHeader) -> usize { + (header.max_nelmts_bits as usize).div_ceil(8) +} + +/// Elements per data block page, once a data block is large enough to be paged. +fn page_nelmts(header: &ExtensibleArrayHeader) -> Option { + 1usize.checked_shl(u32::from(header.max_dblk_nelmts_bits)) +} + +/// Read the elements of one data block (EADB). +/// +/// `page_init` is the owning super block's page-init bitmap and `first_page` +/// this block's first bit in it; both are only consulted when the block is +/// paged. The bitmap lives in the super block, not here — a paged data block +/// stores only its prefix, then one slot per page. +#[allow(clippy::too_many_arguments)] fn read_data_block_elements( file_data: &[u8], db_offset: usize, @@ -280,117 +314,86 @@ fn read_data_block_elements( start_index: usize, num_chunks_per_dim: &[u64], chunk_dimensions: &[u32], + page_init: &[u8], + first_page: usize, ) -> Result, FormatError> { - // AEDB: signature(4) + version(1) + client_id(1) + header_address(offset_size) - let db_header_size = 4 + 1 + 1 + offset_size as usize; + // EADB: signature(4) + version(1) + client_id(1) + header_address(offset_size) + // + block offset(arr_off_size) + let db_header_size = 4 + 1 + 1 + offset_size as usize + arr_off_size(header); ensure_len(file_data, db_offset, db_header_size)?; - let d = &file_data[db_offset..]; - if &d[0..4] != b"EADB" { + if &file_data[db_offset..db_offset + 4] != b"EADB" { return Err(FormatError::ChunkedReadError( "invalid Extensible Array data block signature".into(), )); } - // Skip version(1) + client_id(1) + header_address(offset_size) + block_offset - // Block offset is encoded in ceil(max_nelmts_bits/8) bytes - let blk_off_size = (header.max_nelmts_bits as usize).div_ceil(8); - let mut pos = db_offset + db_header_size + blk_off_size; - // Check if paged - if header.max_nelmts_bits >= usize::BITS as u8 { - return Err(FormatError::Overflow( - "max_nelmts_bits exceeds usize bit width".into(), - )); - } - let page_nelmts = 1usize << header.max_nelmts_bits; - let is_paged = nelmts > page_nelmts; + let mut pos = db_offset + db_header_size; + let page = page_nelmts(header).ok_or_else(|| { + FormatError::Overflow("Extensible Array page element count overflows usize".into()) + })?; let mut chunks = Vec::new(); - - if !is_paged { - for i in 0..nelmts { + let read_run = |from: usize, + count: usize, + first_index: usize, + chunks: &mut Vec| + -> Result { + let mut p = from; + for i in 0..count { let (info, consumed) = read_element( file_data, - pos, + p, header.client_id, header.element_size, offset_size, chunk_byte_size, - start_index + i, + first_index + i, num_chunks_per_dim, chunk_dimensions, )?; if let Some(ci) = info { chunks.push(ci); } - pos += consumed; + p += consumed; } + Ok(p) + }; + + if nelmts <= page { + read_run(pos, nelmts, start_index, &mut chunks)?; + return Ok(chunks); + } + + // Paged: the prefix ends with its own checksum, then one slot per page, + // each holding `page` elements followed by a checksum. Pages whose bit is + // clear were never written; their slot still occupies the file, so stride + // over it rather than reading zeros as addresses. + pos += 4; + let elem_bytes = if header.client_id == 0 { + offset_size as usize } else { - // Paged: elements are split into pages of page_nelmts. - // After the data block header comes a page bitmap, then each page - // has page_nelmts elements followed by a 4-byte checksum. - let npages = nelmts.div_ceil(page_nelmts); - // Page bitmap: ceil(npages / 8) bytes - let bitmap_size = npages.div_ceil(8); - // Read bitmap - if pos + bitmap_size > file_data.len() { - return Err(FormatError::UnexpectedEof { - expected: pos + bitmap_size, - available: file_data.len(), - }); - } - let bitmap = &file_data[pos..pos + bitmap_size]; - pos += bitmap_size; - - let elem_bytes = if header.client_id == 0 { - offset_size as usize - } else { - header.element_size as usize - }; - - let mut global_idx = start_index; - for page_idx in 0..npages { - let byte_idx = page_idx / 8; - let bit_idx = page_idx % 8; - let page_has_data = (bitmap[byte_idx] >> bit_idx) & 1 != 0; - - let elems_this_page = if page_idx == npages - 1 { - let remainder = nelmts % page_nelmts; - if remainder == 0 { - page_nelmts - } else { - remainder - } - } else { - page_nelmts - }; - - if page_has_data { - for i in 0..elems_this_page { - let (info, consumed) = read_element( - file_data, - pos, - header.client_id, - header.element_size, - offset_size, - chunk_byte_size, - global_idx + i, - num_chunks_per_dim, - chunk_dimensions, - )?; - if let Some(ci) = info { - chunks.push(ci); - } - pos += consumed; - } - // Skip page checksum (4 bytes) - pos += 4; - } else { - // Empty page: skip all elements + checksum - pos += elems_this_page * elem_bytes + 4; - } - global_idx += elems_this_page; + header.element_size as usize + }; + let page_stride = page + .checked_mul(elem_bytes) + .and_then(|b| b.checked_add(4)) + .ok_or_else(|| FormatError::Overflow("Extensible Array page stride".into()))?; + let npages = nelmts.div_ceil(page); + for p in 0..npages { + // One bit per page across the whole super block, packed contiguously + // and MSB-first within each byte, as H5VM_bit_get reads it. + let bit = first_page + p; + let initialised = page_init + .get(bit / 8) + .is_some_and(|byte| byte & (0x80 >> (bit % 8)) != 0); + if initialised { + let count = core::cmp::min(page, nelmts - p * page); + read_run(pos, count, start_index + p * page, &mut chunks)?; } + pos = pos + .checked_add(page_stride) + .ok_or_else(|| FormatError::Overflow("Extensible Array page offset".into()))?; } Ok(chunks) @@ -427,30 +430,26 @@ pub fn read_extensible_array_chunks( let chunk_byte_size: u64 = chunk_dimensions.iter().map(|&d| d as u64).product::() * element_size as u64; - // Parse index block (AEIB) + // Parse index block (EAIB): signature(4) + version(1) + client_id(1) + // + header address(offset_size), then the inline elements, then the + // direct data block addresses, then the super block addresses. let ib_offset = header.index_block_address as usize; - let ib_header_size = 4 + 1 + 1 + offset_size as usize; // sig + ver + client + hdr_addr + let ib_header_size = 4 + 1 + 1 + os; ensure_len(file_data, ib_offset, ib_header_size)?; - let ib = &file_data[ib_offset..]; - if &ib[0..4] != b"EAIB" { + if &file_data[ib_offset..ib_offset + 4] != b"EAIB" { return Err(FormatError::ChunkedReadError( "invalid Extensible Array index block signature".into(), )); } - // Skip version(1) + client_id(1) + header_address(offset_size) let mut pos = ib_offset + ib_header_size; let mut chunks = Vec::new(); - let mut global_index = 0usize; let total_elements = header.num_elements as usize; - // 1. Read inline elements in index block - let n_inline = header.idx_blk_elmts as usize; + // 1. Elements stored inline in the index block. + let n_inline = (header.idx_blk_elmts as usize).min(total_elements); for i in 0..n_inline { - if global_index + i >= total_elements { - break; - } let (info, consumed) = read_element( file_data, pos, @@ -458,7 +457,7 @@ pub fn read_extensible_array_chunks( header.element_size, offset_size, chunk_byte_size, - global_index + i, + i, &num_chunks_per_dim, chunk_dimensions, )?; @@ -467,154 +466,128 @@ pub fn read_extensible_array_chunks( } pos += consumed; } - global_index += n_inline.min(total_elements); - - // If all elements were inline, we're done + let mut global_index = n_inline; if global_index >= total_elements { return Ok(chunks); } - // Compute data block and super block counts - let min_dblk = header.min_dblk_nelmts as usize; - let sblk_min = header.super_blk_min_nelmts as usize; + let dmin = header.min_dblk_nelmts as usize; + if dmin == 0 || !dmin.is_power_of_two() { + return Err(FormatError::ChunkedReadError( + "Extensible Array data block minimum is not a power of two".into(), + )); + } + // nsblks = 1 + (max_nelmts_bits - log2(data_blk_min_elmts)), and the index + // block holds 2 * (sup_blk_min_data_ptrs - 1) data block addresses. + let log2_dmin = dmin.trailing_zeros() as usize; + let nsblks = 1 + (header.max_nelmts_bits as usize).saturating_sub(log2_dmin); + let ndblk_addrs = 2 * (header.super_blk_min_nelmts as usize).saturating_sub(1); - // The first sblk_min super block levels have their data blocks listed directly - // in the index block. Compute their sizes. - let mut n_direct_dblks = 0usize; - let mut dblk_sizes: Vec = Vec::new(); - { - let mut nelmts = min_dblk; - for sb_level in 0..sblk_min { - if sb_level >= usize::BITS as usize { - return Err(FormatError::Overflow( - "sb_level exceeds usize bit width".into(), + // 2. Data blocks listed directly in the index block: the first + // `ndblk_addrs` data blocks in super-block order, each sized by the + // level it belongs to. + let mut direct: Vec = Vec::with_capacity(ndblk_addrs); + let mut level = 0usize; + while direct.len() < ndblk_addrs { + let (ndblks, dblk_nelmts) = sblk_info(level, dmin).ok_or_else(|| { + FormatError::Overflow("Extensible Array super block layout overflows usize".into()) + })?; + if level >= nsblks { + return Err(FormatError::ChunkedReadError( + "Extensible Array index block claims more data blocks than the array has".into(), + )); + } + for _ in 0..ndblks { + direct.push(dblk_nelmts); + } + level += 1; + } + if direct.len() != ndblk_addrs { + // A partial level in the index block is not a layout HDF5 produces, + // and guessing where the super blocks resume would misplace elements. + return Err(FormatError::ChunkedReadError( + "Extensible Array index block ends mid super block".into(), + )); + } + + for &dblk_nelmts in &direct { + if global_index >= total_elements { + return Ok(chunks); + } + ensure_len(file_data, pos, os)?; + let addr = read_offset(file_data, pos, offset_size)?; + pos += os; + if !is_undefined_addr(addr, offset_size) { + if dblk_nelmts > page_nelmts(header).unwrap_or(usize::MAX) { + // Would need a page-init bitmap, which only a super block + // carries. HDF5 never pages these small early blocks. + return Err(FormatError::ChunkedReadError( + "Extensible Array index block references a paged data block".into(), )); } - let ndblks = 1usize << sb_level; - for _ in 0..ndblks { - dblk_sizes.push(nelmts); - n_direct_dblks += 1; - } - if sb_level > 0 { - nelmts *= 2; - } + chunks.extend(read_data_block_elements( + file_data, + addr as usize, + dblk_nelmts, + header, + offset_size, + chunk_byte_size, + global_index, + &num_chunks_per_dim, + chunk_dimensions, + &[], + 0, + )?); } + global_index += dblk_nelmts; } - // Read direct data block addresses from index block - let mut dblk_addrs: Vec = Vec::with_capacity(n_direct_dblks); - for _ in 0..n_direct_dblks { - if pos + os > file_data.len() { + // 3. Everything else lives in super blocks, one address per remaining + // level, starting at the level after the direct data blocks. + for u in level..nsblks { + if global_index >= total_elements { break; } - let addr = read_offset(file_data, pos, offset_size)?; - dblk_addrs.push(addr); + ensure_len(file_data, pos, os)?; + let sb_addr = read_offset(file_data, pos, offset_size)?; pos += os; - } - - // Read elements from direct data blocks - for (i, &addr) in dblk_addrs.iter().enumerate() { - if i >= dblk_sizes.len() { - break; + let (ndblks, dblk_nelmts) = sblk_info(u, dmin).ok_or_else(|| { + FormatError::Overflow("Extensible Array super block layout overflows usize".into()) + })?; + if !is_undefined_addr(sb_addr, offset_size) { + chunks.extend(read_super_block( + file_data, + sb_addr as usize, + ndblks, + dblk_nelmts, + header, + offset_size, + chunk_byte_size, + global_index, + &num_chunks_per_dim, + chunk_dimensions, + )?); } - let nelmts = dblk_sizes[i]; - if is_undefined_addr(addr, offset_size) { - global_index += nelmts; - continue; - } - let block_chunks = read_data_block_elements( - file_data, - addr as usize, - nelmts, - header, - offset_size, - chunk_byte_size, - global_index, - &num_chunks_per_dim, - chunk_dimensions, - )?; - chunks.extend(block_chunks); - global_index += nelmts; - } - - // Remaining elements are in super blocks - let total_in_ib_and_direct: usize = n_inline + dblk_sizes.iter().sum::(); - if total_elements <= total_in_ib_and_direct { - return Ok(chunks); - } - let remaining_elements = total_elements - total_in_ib_and_direct; - - // Compute super block layout - let mut sb_addrs: Vec = Vec::new(); - let mut sb_infos: Vec<(usize, usize)> = Vec::new(); - { - let mut covered = 0usize; - let mut sb_level = sblk_min; - let mut nelmts_per_dblk = min_dblk; - for lev in 0..sblk_min { - if lev > 0 { - nelmts_per_dblk *= 2; - } - } - - while covered < remaining_elements { - if sb_level >= usize::BITS as usize { - return Err(FormatError::Overflow( - "sb_level exceeds usize bit width".into(), - )); - } - let ndblks = 1usize << sb_level; - nelmts_per_dblk *= 2; - let total_in_sb = ndblks * nelmts_per_dblk; - sb_infos.push((ndblks, nelmts_per_dblk)); - covered += total_in_sb; - sb_level += 1; - } - } - - // Read super block addresses from index block - for _ in 0..sb_infos.len() { - if pos + os > file_data.len() { - break; - } - let addr = read_offset(file_data, pos, offset_size)?; - sb_addrs.push(addr); - pos += os; - } - - // Process each super block - for (sb_idx, &sb_addr) in sb_addrs.iter().enumerate() { - let (ndblks, nelmts_per_dblk) = sb_infos[sb_idx]; - if is_undefined_addr(sb_addr, offset_size) { - global_index += ndblks * nelmts_per_dblk; - continue; - } - let sb_chunks = read_super_block( - file_data, - sb_addr as usize, - ndblks, - nelmts_per_dblk, - header, - offset_size, - chunk_byte_size, - global_index, - &num_chunks_per_dim, - chunk_dimensions, - )?; - chunks.extend(sb_chunks); - global_index += ndblks * nelmts_per_dblk; + global_index = + global_index.saturating_add(ndblks.checked_mul(dblk_nelmts).ok_or_else(|| { + FormatError::Overflow("Extensible Array super block span".into()) + })?); } Ok(chunks) } -/// Read a super block (AESB) and its data blocks. +/// Read a super block (EASB) and the data blocks it owns. +/// +/// On disk: signature(4) + version(1) + client_id(1) + header address +/// + block offset + the page-init bitmap for every data block it owns +/// + one address per data block + checksum. #[allow(clippy::too_many_arguments)] fn read_super_block( file_data: &[u8], sb_offset: usize, ndblks: usize, - nelmts_per_dblk: usize, + dblk_nelmts: usize, header: &ExtensibleArrayHeader, offset_size: u8, chunk_byte_size: u64, @@ -623,9 +596,7 @@ fn read_super_block( chunk_dimensions: &[u32], ) -> Result, FormatError> { let os = offset_size as usize; - - // AESB: signature(4) + version(1) + client_id(1) + header_address(offset_size) - let sb_header_size = 4 + 1 + 1 + os; + let sb_header_size = 4 + 1 + 1 + os + arr_off_size(header); ensure_len(file_data, sb_offset, sb_header_size)?; if &file_data[sb_offset..sb_offset + 4] != b"EASB" { @@ -634,43 +605,50 @@ fn read_super_block( )); } - let mut pos = sb_offset + sb_header_size; - - // Read data block addresses - let mut dblk_addrs: Vec = Vec::with_capacity(ndblks); - for _ in 0..ndblks { - if pos + os > file_data.len() { - return Err(FormatError::UnexpectedEof { - expected: pos + os, - available: file_data.len(), - }); - } - let addr = read_offset(file_data, pos, offset_size)?; - dblk_addrs.push(addr); - pos += os; - } + // Page-init bitmap: one bit per page, `npages` bits per data block, packed + // contiguously. HDF5 sizes the buffer `ndblks * ceil(npages / 8)`, which + // is bigger than the bits need when `npages` is not a multiple of eight. + // Zero-sized unless this level's data blocks are paged. + let page = page_nelmts(header).ok_or_else(|| { + FormatError::Overflow("Extensible Array page element count overflows usize".into()) + })?; + let npages = if dblk_nelmts > page { + dblk_nelmts / page + } else { + 0 + }; + let per_dblk_bitmap = npages.div_ceil(8); + let bitmap_bytes = per_dblk_bitmap + .checked_mul(ndblks) + .ok_or_else(|| FormatError::Overflow("Extensible Array page bitmap size".into()))?; + let bitmap_start = sb_offset + sb_header_size; + ensure_len(file_data, bitmap_start, bitmap_bytes)?; + let bitmap = &file_data[bitmap_start..bitmap_start + bitmap_bytes]; + let mut pos = bitmap_start + bitmap_bytes; let mut chunks = Vec::new(); let mut global_idx = start_index; - for &addr in &dblk_addrs { - if is_undefined_addr(addr, offset_size) { - global_idx += nelmts_per_dblk; - continue; + for i in 0..ndblks { + ensure_len(file_data, pos, os)?; + let addr = read_offset(file_data, pos, offset_size)?; + pos += os; + if !is_undefined_addr(addr, offset_size) { + chunks.extend(read_data_block_elements( + file_data, + addr as usize, + dblk_nelmts, + header, + offset_size, + chunk_byte_size, + global_idx, + num_chunks_per_dim, + chunk_dimensions, + bitmap, + i * npages, + )?); } - let block_chunks = read_data_block_elements( - file_data, - addr as usize, - nelmts_per_dblk, - header, - offset_size, - chunk_byte_size, - global_idx, - num_chunks_per_dim, - chunk_dimensions, - )?; - chunks.extend(block_chunks); - global_idx += nelmts_per_dblk; + global_idx += dblk_nelmts; } Ok(chunks) diff --git a/crates/clawhdf5/tests/h5py_interop_tests.rs b/crates/clawhdf5/tests/h5py_interop_tests.rs index ede336b..ca458ab 100644 --- a/crates/clawhdf5/tests/h5py_interop_tests.rs +++ b/crates/clawhdf5/tests/h5py_interop_tests.rs @@ -1091,3 +1091,153 @@ with h5py.File("{path_str}", "w", libver="latest") as f: assert_eq!(v, i as i32, "element {i}"); } } + +#[test] +fn h5py_extensible_array_chunk_index_clawhdf5_reads() { + // One unlimited dimension means an Extensible Array chunk index. Only its + // first few elements live inline in the index block (4 by default), and + // every other fixture here is small enough to stop there — which is how + // the data block and super block layouts came to be wrong without a test + // noticing. The counts below step over each boundary in turn: + // 4 inline elements only + // 37 past the first direct data block + // 400 into the first super block + // 5000 several super block levels + // 200000 data blocks large enough to be paged + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + + for n in [4usize, 37, 400, 5_000, 200_000] { + let path = dir.path().join(format!("ea_{n}.h5")); + let path_str = path.display().to_string(); + run_python(&format!( + r#" +import h5py, numpy as np +with h5py.File("{path_str}", "w", libver="latest") as f: + d = f.create_dataset("x", shape=({n},), maxshape=(None,), chunks=(1,), dtype="i4") + d[...] = np.arange({n}, dtype="i4") +"# + )); + + let bytes = std::fs::read(&path).unwrap(); + assert!( + bytes.windows(4).any(|w| w == b"EAHD"), + "n={n}: fixture is not indexed by an Extensible Array" + ); + + let file = File::open(&path).unwrap(); + let values = file.dataset("x").unwrap().read_i32().unwrap(); + assert_eq!(values.len(), n, "n={n}"); + let wrong = values + .iter() + .enumerate() + .filter(|&(i, &v)| v != i as i32) + .count(); + assert_eq!(wrong, 0, "n={n}: {wrong} of {n} elements read back wrong"); + } +} + +#[test] +fn h5py_sparse_extensible_array_leaves_pages_uninitialised() { + // Writing a scattered subset leaves whole pages of a paged data block + // never initialised. Those pages still occupy their slot on disk, so the + // reader has to skip them by stride and take the fill value instead — + // driven by the page-init bitmap, which is packed one bit per page across + // the whole super block, MSB first. + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("ea_sparse.h5"); + let path_str = path.display().to_string(); + let n = 200_000usize; + let step = 997usize; + + run_python(&format!( + r#" +import h5py +with h5py.File("{path_str}", "w", libver="latest") as f: + d = f.create_dataset("x", shape=({n},), maxshape=(None,), chunks=(1,), + dtype="i4", fillvalue=-1) + for i in list(range(0, {n}, {step})) + list(range(0, 40)): + d[i] = i +"# + )); + + let file = File::open(&path).unwrap(); + let values = file.dataset("x").unwrap().read_i32().unwrap(); + assert_eq!(values.len(), n); + let wrong = values + .iter() + .enumerate() + .filter(|&(i, &v)| { + let expected = if i % step == 0 || i < 40 { + i as i32 + } else { + -1 + }; + v != expected + }) + .count(); + assert_eq!(wrong, 0, "{wrong} of {n} elements read back wrong"); +} + +#[test] +fn h5py_filtered_and_2d_extensible_array_clawhdf5_reads() { + // Filtered elements carry a size and filter mask beside the address, and + // a second (fixed) dimension changes how a linear index maps back to + // chunk offsets. Both run through the same traversal. + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + + let gz = dir.path().join("ea_gzip.h5"); + let gz_str = gz.display().to_string(); + run_python(&format!( + r#" +import h5py, numpy as np +with h5py.File("{gz_str}", "w", libver="latest") as f: + d = f.create_dataset("x", shape=(5000,), maxshape=(None,), chunks=(1,), + dtype="i4", compression="gzip", compression_opts=4) + d[...] = np.arange(5000, dtype="i4") +"# + )); + let values = File::open(&gz) + .unwrap() + .dataset("x") + .unwrap() + .read_i32() + .unwrap(); + assert_eq!(values.len(), 5000); + assert_eq!( + values + .iter() + .enumerate() + .filter(|&(i, &v)| v != i as i32) + .count(), + 0 + ); + + let two_d = dir.path().join("ea_2d.h5"); + let two_d_str = two_d.display().to_string(); + run_python(&format!( + r#" +import h5py, numpy as np +with h5py.File("{two_d_str}", "w", libver="latest") as f: + d = f.create_dataset("x", shape=(3000, 4), maxshape=(None, 4), chunks=(1, 4), dtype="i4") + d[...] = np.arange(12000, dtype="i4").reshape(3000, 4) +"# + )); + let values = File::open(&two_d) + .unwrap() + .dataset("x") + .unwrap() + .read_i32() + .unwrap(); + assert_eq!(values.len(), 12_000); + assert_eq!( + values + .iter() + .enumerate() + .filter(|&(i, &v)| v != i as i32) + .count(), + 0 + ); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index d81eef1..57aa18e 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -193,3 +193,37 @@ B-tree v2 backs dense attribute storage, v2 groups, shared object header messages and chunk indexes, so opening an object that uses any of them is enough. Both are now errors: depth is capped at 64, and traversal stops once it has produced more records than the file could physically hold. + +--- + +## Extensible Array chunk indexes read back wrong data past the inline elements + +**Status:** fixed on `main` (2026-09-20), after v2.6.0. **Every release up to +and including v2.6.0 is affected.** + +A dataset created with exactly one unlimited dimension (`maxshape=(None, ...)`, +the usual append-only/resizable case) is indexed by an Extensible Array. Its +index block holds the first `idx_blk_elmts` chunk entries inline — 4 by +default — and everything after that lives in data blocks and super blocks whose +layout `clawhdf5-format` computed incorrectly. + +Consequences, by dataset size (1 chunk per element): + +| chunks | result before the fix | +|---|---| +| <= 36 | correct (inline, plus two data blocks that happened to line up) | +| 37 | 1 element wrong | +| 400 | 364 elements wrong | +| >= ~1000 | `invalid Extensible Array data block signature` | + +The dangerous case is the middle one: values were returned from the wrong +chunks rather than an error being raised. Any reader that accepted the data at +face value saw plausible but incorrect numbers. + +The root causes were the super block sizing formulas (`ndblks` and +`dblk_nelmts` each double every *other* level, a half-step apart), a missing +block-offset field in the super block, and a page-init bitmap read from the +wrong structure. All four are fixed and covered by interop tests against +HDF5 2.0 at sizes that cross each boundary, including paged data blocks. + +Files written by this crate are unaffected — this was purely a read-path bug.