From 3938f7f8a2feb385340e021d25756dadd0bafa4b Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 01:30:56 -0500 Subject: [PATCH] fix(io): refuse truncated files in the VOL, async and MPI readers The truncated-file check and the end-of-file clamp reached File, LazyFile and MmapFile but not clawhdf5-io's readers, which still opened truncated files and read past the recorded end of file. NativeVol (open, and read_dataset for from_bytes), AsyncHDF5File::from_bytes and MpiVol's collective read now view the file through the new vol::hdf5_view: from the superblock to Superblock::data_end, refusing a file shorter than that. MpiVol's read is compiled only with the mpi-io feature, which needs an MPI installation; it was not built here. The edit there only swaps its two-line superblock setup for hdf5_view. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 7 +++- crates/clawhdf5-io/src/async_read.rs | 27 +++++++++++++ crates/clawhdf5-io/src/mpi_vol.rs | 9 ++--- crates/clawhdf5-io/src/vol.rs | 57 +++++++++++++++++++++++++--- 4 files changed, 88 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97bfb97..75b1ba3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -343,8 +343,11 @@ (`chunked_read::collect_chunk_info_checked`). - truncated files (`FormatError::TruncatedFile`, `Superblock::data_end`): a file shorter than the end of file its superblock records is refused - ("truncated file"), and nothing past that end is read. `File`, - `LazyFile` and `MmapFile` do this. + ("truncated file"), and nothing past that end is read. Every reader + does this: `File`, `LazyFile` and `MmapFile`, and in `clawhdf5-io` + `NativeVol` (at `open`, and on read for `from_bytes`), + `AsyncHDF5File` and `MpiVol` (the MPI path is not built in CI: it + needs an MPI installation). - the writer: `FileWriter::finish()` / `FileBuilder::finish()` refuse a datatype the reader would refuse (`FormatError::SerializationError`, "datatype cannot be written: ..."), such as a compound with a repeated diff --git a/crates/clawhdf5-io/src/async_read.rs b/crates/clawhdf5-io/src/async_read.rs index ddfce7b..30969fa 100644 --- a/crates/clawhdf5-io/src/async_read.rs +++ b/crates/clawhdf5-io/src/async_read.rs @@ -281,8 +281,13 @@ impl AsyncHDF5File { // HDF5 addresses are relative to the superblock: drop any user block // so they index `data` directly. let user_block = find_signature(&data)?; + let whole_len = data.len() as u64; data.drain(..user_block); let superblock = Superblock::parse(&data, 0)?; + // Refuse a truncated file, and keep nothing past the end of file the + // superblock records, as libhdf5 does. + let end = superblock.data_end(user_block as u64, whole_len)?; + data.truncate(end as usize); Ok(Self { data, superblock }) } @@ -605,6 +610,28 @@ mod tests { tokio::fs::remove_file(&path).await.ok(); } + /// As libhdf5 does: a truncated file is refused, and bytes past the end + /// of file the superblock records are dropped. + #[tokio::test] + async fn async_refuses_truncated_files_and_drops_trailing_bytes() { + let bytes = make_test_hdf5_f64("v", &[1.0, 2.0]); + let truncated = bytes[..bytes.len() - 8].to_vec(); + let err = AsyncHDF5File::from_bytes(truncated).err().unwrap(); + assert!( + matches!( + err, + AsyncHDF5Error::Format(FormatError::TruncatedFile { .. }) + ), + "{err}" + ); + + let mut appended = bytes.clone(); + appended.extend_from_slice(&[0xAB; 64]); + let file = AsyncHDF5File::from_bytes(appended).unwrap(); + assert_eq!(file.as_bytes().len(), bytes.len()); + assert_eq!(file.read_f64("v").await.unwrap(), vec![1.0, 2.0]); + } + #[tokio::test] async fn async_error_display() { let io_err = AsyncHDF5Error::Io(io::Error::new(io::ErrorKind::NotFound, "gone")); diff --git a/crates/clawhdf5-io/src/mpi_vol.rs b/crates/clawhdf5-io/src/mpi_vol.rs index fe39c4e..87a82ea 100644 --- a/crates/clawhdf5-io/src/mpi_vol.rs +++ b/crates/clawhdf5-io/src/mpi_vol.rs @@ -180,8 +180,7 @@ fn mpi_collective_read(vol: &MpiVol, location: &str, path: &str) -> Result Result, } +/// The HDF5 bytes of a whole file and its superblock: from the superblock +/// (addresses are relative to it, so any user block is skipped) to the end +/// of file the superblock records. A file shorter than that is truncated +/// and refused, and nothing past it is read, as in libhdf5. +pub(crate) fn hdf5_view( + whole: &[u8], +) -> Result<(&[u8], clawhdf5_format::superblock::Superblock), VolError> { + use clawhdf5_format::{signature::split_user_block, superblock::Superblock}; + let err = |e: clawhdf5_format::error::FormatError| VolError::DataError(e.to_string()); + let (user_block, data) = split_user_block(whole).map_err(err)?; + let sb = Superblock::parse(data, 0).map_err(err)?; + let end = sb + .data_end(user_block.len() as u64, whole.len() as u64) + .map_err(err)?; + // data_end is at most the file length less the user block. + Ok((&data[..end as usize], sb)) +} + impl NativeVol { /// Create a new native VOL connector. pub fn new() -> Self { @@ -264,6 +282,8 @@ impl VirtualObjectLayer for NativeVol { fn open(&mut self, location: &str) -> Result<(), VolError> { let data = std::fs::read(location)?; + // Refuse a truncated file at open, as libhdf5 does. + hdf5_view(&data)?; self.data = Some(data); self.location = Some(location.to_string()); Ok(()) @@ -283,13 +303,10 @@ impl VirtualObjectLayer for NativeVol { use clawhdf5_format::{ data_layout::DataLayout, data_read::read_raw_data_full, dataspace::Dataspace, datatype::Datatype, filter_pipeline::FilterPipeline, group_v2::resolve_path_any, - message_type::MessageType, object_header::ObjectHeader, signature::split_user_block, - superblock::Superblock, + message_type::MessageType, object_header::ObjectHeader, }; - // Addresses are relative to the superblock: skip any user block. - let (_, data) = split_user_block(data).map_err(|e| VolError::DataError(e.to_string()))?; - let sb = Superblock::parse(data, 0).map_err(|e| VolError::DataError(e.to_string()))?; + let (data, sb) = hdf5_view(data)?; let addr = resolve_path_any(data, &sb, path) .map_err(|e| VolError::NotFound(format!("{path}: {e}")))?; @@ -387,6 +404,36 @@ mod tests { assert!(vol.as_bytes().is_none()); } + /// As libhdf5 does: a file shorter than the end of file its superblock + /// records is truncated and refused (at open, and when read from + /// memory), and bytes appended past that end are not part of the file. + #[test] + fn native_vol_refuses_truncated_files_and_ignores_trailing_bytes() { + use clawhdf5_format::file_writer::FileWriter as FmtWriter; + + let mut fw = FmtWriter::new(); + fw.create_dataset("x").with_f64_data(&[1.0, 2.0, 3.0]); + let bytes = fw.finish().unwrap(); + + let truncated = bytes[..bytes.len() - 8].to_vec(); + let err = NativeVol::from_bytes(truncated.clone()) + .read_dataset("x") + .unwrap_err(); + assert!(err.to_string().contains("truncated"), "{err}"); + let dir = std::env::temp_dir().join(format!("clawhdf5_vol_trunc_{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("truncated.h5"); + std::fs::write(&path, &truncated).unwrap(); + let err = NativeVol::open_path(path.to_str().unwrap()).err().unwrap(); + assert!(err.to_string().contains("truncated"), "{err}"); + std::fs::remove_dir_all(&dir).ok(); + + let mut appended = bytes.clone(); + appended.extend_from_slice(&[0xAB; 64]); + let raw = NativeVol::from_bytes(appended).read_dataset("x").unwrap(); + assert_eq!(raw.len(), 24); + } + #[test] fn vol_error_display() { let err = VolError::Unsupported("read_dataset".into());