diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index 9629dcc..87c660d 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -724,6 +724,17 @@ fn main() { return; } }; + // libhdf5 refuses a truncated file and reads nothing past the recorded + // end of file. + let base = (data.len() - hdf5.len()) as u64; + let hdf5 = match sb.data_end(base, data.len() as u64) { + Ok(end) => &hdf5[..end as usize], + Err(err) => { + top.insert("open_error".into(), Value::String(e(err))); + println!("{}", Value::Object(top)); + return; + } + }; top.insert("superblock_version".into(), json!(sb.version)); let ctx = Ctx { data: hdf5, diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index 814a5a7..b3cfd65 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -214,6 +214,14 @@ pub enum FormatError { /// dimension, a rank that does not match the dataspace, or a chunk of /// 4 GiB or more. 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). + TruncatedFile { + /// End of file recorded in the superblock (relative to byte 0). + stored_eof: u64, + /// The file's actual length in bytes. + actual_len: u64, + }, } impl fmt::Display for FormatError { @@ -467,6 +475,16 @@ impl fmt::Display for FormatError { FormatError::InvalidChunkDimensions(why) => { write!(f, "invalid chunk dimensions: {why}") } + FormatError::TruncatedFile { + stored_eof, + actual_len, + } => { + write!( + f, + "truncated file: the superblock records end of file {stored_eof}, \ + but the file is {actual_len} bytes" + ) + } } } } diff --git a/crates/clawhdf5-format/src/superblock.rs b/crates/clawhdf5-format/src/superblock.rs index d2ec4d6..a565321 100644 --- a/crates/clawhdf5-format/src/superblock.rs +++ b/crates/clawhdf5-format/src/superblock.rs @@ -100,6 +100,42 @@ pub mod swmr_flags { } impl Superblock { + /// Where the HDF5 data ends, relative to the superblock, for a file of + /// `file_len` bytes whose superblock is at `user_block` (both counted + /// from the start of the file), with libhdf5's truncation check + /// (`H5F__super_read`). + /// + /// The superblock records the end of the file's data as an absolute + /// address. A file shorter than that was truncated, and libhdf5 refuses + /// to open it ("truncated file"); so does this, with + /// [`FormatError::TruncatedFile`]. Bytes past that address are not part + /// of the file: libhdf5 fails any read of them ("addr overflow" / + /// "address plus size exceeds file eoa"), so a reader should parse only + /// the data up to the returned end. As libhdf5 does for a SWMR reader, + /// the check is skipped for a version-3 superblock whose writer is still + /// writing it in SWMR mode (it extends the file as it goes); the data + /// then ends at the end of the file. + /// + /// When the superblock's recorded base address differs from where the + /// superblock actually is (a user block added or removed after the file + /// was written), libhdf5 moves the recorded end of file by the same + /// amount, and so does this. + pub fn data_end(&self, user_block: u64, file_len: u64) -> Result { + let eof = + i128::from(self.eof_address) - i128::from(self.base_address) + i128::from(user_block); + if eof < 0 || eof > i128::from(file_len) { + if self.version >= 3 && self.is_swmr_write() { + return Ok(file_len.saturating_sub(user_block)); + } + return Err(FormatError::TruncatedFile { + stored_eof: u64::try_from(eof).unwrap_or(self.eof_address), + actual_len: file_len, + }); + } + // 0 <= eof <= file_len, so it fits a u64. + Ok((eof as u64).saturating_sub(user_block)) + } + /// Whether the file was opened with write access when the superblock was written. pub fn is_write_access(&self) -> bool { self.consistency_flags & swmr_flags::WRITE_ACCESS != 0 @@ -537,6 +573,30 @@ mod tests { buf } + #[test] + fn data_end_refuses_truncated_files_like_libhdf5() { + // build_v2_bytes records base 0, end of file 2048. + let sb = Superblock::parse(&build_v2_bytes(8, 2), 0).unwrap(); + assert_eq!(sb.data_end(0, 2048), Ok(2048)); + // Bytes past the recorded end are not part of the file. + assert_eq!(sb.data_end(0, 4096), Ok(2048)); + assert_eq!( + sb.data_end(0, 2047), + Err(FormatError::TruncatedFile { + stored_eof: 2048, + actual_len: 2047 + }) + ); + // A user block added in front after the file was written (the + // recorded base address is still 0): the end moves with it. + assert_eq!(sb.data_end(512, 2560), Ok(2048)); + assert!(sb.data_end(512, 2559).is_err()); + // A v3 superblock still being written in SWMR mode is not checked. + let mut swmr = Superblock::parse(&build_v2_bytes(8, 3), 0).unwrap(); + swmr.consistency_flags = swmr_flags::WRITE_ACCESS | swmr_flags::SWMR_WRITE; + assert_eq!(swmr.data_end(0, 1000), Ok(1000)); + } + #[test] fn parse_v0_8byte_offsets() { let data = build_v0_bytes(8); diff --git a/crates/clawhdf5/src/lazy.rs b/crates/clawhdf5/src/lazy.rs index cf70f70..d893485 100644 --- a/crates/clawhdf5/src/lazy.rs +++ b/crates/clawhdf5/src/lazy.rs @@ -43,6 +43,8 @@ pub struct LazyFile { /// Offset of the superblock in the file (the user-block size); every /// HDF5 address is relative to it. base: usize, + /// End of the HDF5 data (`Superblock::data_end`, absolute). + end: usize, superblock: Superblock, root_header: ObjectHeader, /// Cache of parsed object headers, keyed by address. @@ -74,9 +76,13 @@ impl LazyFile { /// /// Parses only the superblock and root group object header. pub fn open(reader: R) -> Result { + let whole_len = reader.as_bytes().len() as u64; let (user_block, data) = signature::split_user_block(reader.as_bytes())?; let base = user_block.len(); let superblock = Superblock::parse(data, 0)?; + // Refuse a truncated file; read nothing past the recorded end of file. + let end = base + superblock.data_end(base as u64, whole_len)? as usize; + let data = &reader.as_bytes()[base..end]; let root_header = ObjectHeader::parse( data, superblock.root_group_address as usize, @@ -86,6 +92,7 @@ impl LazyFile { Ok(Self { reader, base, + end, superblock, root_header, header_cache: RefCell::new(HashMap::new()), @@ -104,7 +111,7 @@ impl LazyFile { } fn hdf5_bytes(&self) -> &[u8] { - &self.reader.as_bytes()[self.base..] + &self.reader.as_bytes()[self.base..self.end] } /// Returns a reference to the parsed superblock. diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index 8c35163..7f544ca 100644 --- a/crates/clawhdf5/src/mmap_file.rs +++ b/crates/clawhdf5/src/mmap_file.rs @@ -35,6 +35,8 @@ pub struct MmapFile { /// Offset of the superblock in the mapped file (the user-block size); /// every HDF5 address is relative to it. base: usize, + /// End of the HDF5 data (`Superblock::data_end`, absolute). + end: usize, superblock: Superblock, } @@ -42,12 +44,16 @@ impl MmapFile { /// Open an HDF5 file using memory-mapped I/O. pub fn open>(path: P) -> Result { let reader = MmapReader::open(path).map_err(Error::Io)?; + let whole_len = reader.as_bytes().len() as u64; let (user_block, data) = signature::split_user_block(reader.as_bytes())?; let base = user_block.len(); let superblock = Superblock::parse(data, 0)?; + // Refuse a truncated file; read nothing past the recorded end of file. + let end = base + superblock.data_end(base as u64, whole_len)? as usize; Ok(Self { reader, base, + end, superblock, }) } @@ -55,7 +61,7 @@ impl MmapFile { /// The file's bytes from the superblock on — the space HDF5 addresses /// index into. fn hdf5_bytes(&self) -> &[u8] { - &self.reader.as_bytes()[self.base..] + &self.reader.as_bytes()[self.base..self.end] } /// Size of the user block before the superblock (0 for most files). diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index 5f2a027..6e506b4 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -45,26 +45,34 @@ impl Backing { } } -/// The file's bytes, viewed from the superblock on. A file may start with a -/// user block (the superblock at 512, 1024, …); every HDF5 address is -/// relative to the superblock, so all parsing goes through [`Self::as_bytes`]. +/// The file's bytes, viewed from the superblock on and up to the end of +/// file the superblock records. A file may start with a user block (the +/// superblock at 512, 1024, …); every HDF5 address is relative to the +/// superblock, so all parsing goes through [`Self::as_bytes`]. struct FileData { backing: Backing, /// Offset of the superblock in the file (the user-block size). base: usize, + /// End of the HDF5 data in the file (`Superblock::data_end`, absolute). + end: usize, } impl FileData { - /// Locate the superblock and parse it. + /// Locate the superblock and parse it. A truncated file is refused, and + /// bytes past the recorded end of file are not read, as in libhdf5. fn new(backing: Backing) -> Result<(Self, Superblock), Error> { - let (user_block, hdf5) = signature::split_user_block(backing.whole_file())?; + let whole = backing.whole_file(); + let (user_block, hdf5) = signature::split_user_block(whole)?; let base = user_block.len(); let superblock = Superblock::parse(hdf5, 0)?; - Ok((Self { backing, base }, superblock)) + let end = superblock.data_end(base as u64, whole.len() as u64)?; + // data_end is at most the file length (less the user block). + let end = base + end as usize; + Ok((Self { backing, base, end }, superblock)) } fn as_bytes(&self) -> &[u8] { - &self.backing.whole_file()[self.base..] + &self.backing.whole_file()[self.base..self.end] } fn len(&self) -> usize { diff --git a/crates/clawhdf5/tests/header_validation_interop.rs b/crates/clawhdf5/tests/header_validation_interop.rs index a2f3e7a..23509e1 100644 --- a/crates/clawhdf5/tests/header_validation_interop.rs +++ b/crates/clawhdf5/tests/header_validation_interop.rs @@ -153,3 +153,48 @@ for libver in ("earliest", "latest"): ], ); } + +#[test] +fn truncated_files_are_refused_and_nothing_past_the_end_of_file_is_read() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + // The superblock records where the file's data ends. A copy missing its + // last bytes is truncated: libhdf5 refuses to open it (this read what + // was left). Bytes appended after the end are not part of the file, and + // a superblock moved by prepending a user block (its recorded base + // address now wrong) has its end of file moved with it; both still read. + let verdicts = h5py_verdicts( + dir.path(), + 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="