diff --git a/crates/clawhdf5-format/src/datatype.rs b/crates/clawhdf5-format/src/datatype.rs index 4a1ba26..436a6cf 100644 --- a/crates/clawhdf5-format/src/datatype.rs +++ b/crates/clawhdf5-format/src/datatype.rs @@ -4,7 +4,7 @@ //! for compound, enumeration, variable-length, and array types. #[cfg(not(feature = "std"))] -use alloc::{boxed::Box, string::String, vec, vec::Vec}; +use alloc::{boxed::Box, format, string::String, vec, vec::Vec}; use byteorder::{ByteOrder, LittleEndian}; @@ -137,6 +137,17 @@ pub enum Datatype { }, } +/// Longest opaque tag that can be stored: its NUL-padded length must fit +/// the 8-bit length in the datatype's class bits. +pub const MAX_OPAQUE_TAG_LEN: usize = 248; + +/// An opaque tag up to (not including) its first NUL. +fn opaque_tag_text(tag: &[u8]) -> &[u8] { + tag.iter() + .position(|&b| b == 0) + .map_or(tag, |end| &tag[..end]) +} + fn ensure_len(data: &[u8], offset: usize, needed: usize) -> Result<(), FormatError> { match offset.checked_add(needed) { Some(end) if end <= data.len() => Ok(()), @@ -361,7 +372,10 @@ impl Datatype { // Opaque let tag_len = bf0 as usize; ensure_len(data, pos, tag_len)?; - let tag = data[pos..pos + tag_len].to_vec(); + // The stored tag is NUL-padded to a multiple of 8 bytes; the + // tag itself ends at the first NUL (libhdf5 reads it with + // `strndup`). + let tag = opaque_tag_text(&data[pos..pos + tag_len]).to_vec(); // Tags are padded to multiple of 8 bytes let padded = (tag_len + 7) & !7; let pos = 8 + padded; // from start of properties @@ -767,7 +781,77 @@ impl Datatype { buf.extend_from_slice(&base_type.serialize()); buf } - _ => Vec::new(), + Datatype::Time { + size, + bit_precision, + } => { + // Byte order is not modelled for time types; write little-endian. + let mut buf = Self::build_header(2, 1, [0, 0, 0], *size); + buf.extend_from_slice(&bit_precision.to_le_bytes()); + buf + } + Datatype::BitField { + size, + byte_order, + bit_offset, + bit_precision, + } => { + let bf0 = u8::from(matches!(byte_order, DatatypeByteOrder::BigEndian)); + let mut buf = Self::build_header(4, 1, [bf0, 0, 0], *size); + buf.extend_from_slice(&bit_offset.to_le_bytes()); + buf.extend_from_slice(&bit_precision.to_le_bytes()); + buf + } + Datatype::Opaque { size, tag } => { + // The tag is stored NUL-padded to a multiple of 8 bytes and the + // padded length goes in the class bits, as libhdf5 writes it. + // A tag longer than MAX_OPAQUE_TAG_LEN cannot be encoded; + // `check_encodable` rejects it before a file is written. + let tag = opaque_tag_text(tag); + let tag = &tag[..tag.len().min(MAX_OPAQUE_TAG_LEN)]; + let padded = tag.len().div_ceil(8) * 8; + let mut buf = Self::build_header(5, 1, [padded as u8, 0, 0], *size); + buf.extend_from_slice(tag); + buf.resize(8 + padded, 0); + buf + } + Datatype::Reference { size, ref_type } => { + // Legacy references are datatype version 1; the H5T_STD_REF + // kinds only exist from version 4, which also carries their + // encoding version (1) in the high nibble. + let (version, bf0) = match ref_type { + ReferenceType::Object => (1, 0), + ReferenceType::DatasetRegion => (1, 1), + ReferenceType::Object2 => (4, 0x12), + ReferenceType::DatasetRegion2 => (4, 0x13), + ReferenceType::Attribute => (4, 0x14), + }; + Self::build_header(7, version, [bf0, 0, 0], *size) + } + } + } + + /// Check that this datatype can be written: every part of it has an + /// on-disk encoding. [`Self::serialize`] cannot report errors, so the + /// writer calls this first. + pub fn check_encodable(&self) -> Result<(), FormatError> { + match self { + Datatype::Opaque { tag, .. } if opaque_tag_text(tag).len() > MAX_OPAQUE_TAG_LEN => { + Err(FormatError::SerializationError(format!( + "opaque tag is {} bytes; at most {MAX_OPAQUE_TAG_LEN} can be stored", + opaque_tag_text(tag).len() + ))) + } + Datatype::String { size: 0, .. } => Err(FormatError::SerializationError( + "fixed-length string datatype of size 0 (libhdf5 requires at least 1 byte)".into(), + )), + Datatype::Compound { members, .. } => members + .iter() + .try_for_each(|m| m.datatype.check_encodable()), + Datatype::Enumeration { base_type, .. } + | Datatype::VariableLength { base_type, .. } + | Datatype::Array { base_type, .. } => base_type.check_encodable(), + _ => Ok(()), } } @@ -1625,6 +1709,122 @@ mod tests { ); } + fn hex(s: &str) -> Vec { + (0..s.len()) + .step_by(2) + .map(|i| u8::from_str_radix(&s[i..i + 2], 16).unwrap()) + .collect() + } + + /// `serialize` used to return an empty message for these four classes, + /// which libhdf5 rejects ("ran off end of input buffer while decoding"). + /// Expected bytes are libhdf5's own encoding (HDF5 2.0 `H5Tencode`, or the + /// datatype message of an HDF5 2.0 file for `H5T_STD_REF`). + #[test] + fn serialize_matches_libhdf5_for_time_bitfield_opaque_reference() { + let cases = [ + ( + Datatype::Reference { + size: 8, + ref_type: ReferenceType::Object, + }, + "1700000008000000", + ), + ( + Datatype::Reference { + size: 12, + ref_type: ReferenceType::DatasetRegion, + }, + "170100000c000000", + ), + ( + Datatype::Reference { + size: 18, + ref_type: ReferenceType::Object2, + }, + "4712000012000000", + ), + ( + Datatype::BitField { + size: 1, + byte_order: DatatypeByteOrder::LittleEndian, + bit_offset: 0, + bit_precision: 8, + }, + "140000000100000000000800", + ), + ( + Datatype::BitField { + size: 2, + byte_order: DatatypeByteOrder::BigEndian, + bit_offset: 0, + bit_precision: 16, + }, + "140100000200000000001000", + ), + ( + Datatype::Opaque { + size: 4, + tag: b"mytag".to_vec(), + }, + "15080000040000006d79746167000000", + ), + ( + Datatype::Opaque { + size: 4, + tag: b"12345678".to_vec(), + }, + "15080000040000003132333435363738", + ), + ( + Datatype::Opaque { + size: 4, + tag: vec![], + }, + "1500000004000000", + ), + ( + Datatype::Time { + size: 4, + bit_precision: 32, + }, + "12000000040000002000", + ), + ]; + for (dt, expected) in cases { + let bytes = dt.serialize(); + assert_eq!(bytes, hex(expected), "{dt:?}"); + let (parsed, consumed) = Datatype::parse(&bytes).unwrap(); + assert_eq!(parsed, dt); + assert_eq!(consumed, bytes.len()); + } + } + + #[test] + fn opaque_tag_padding_is_not_part_of_the_tag() { + // libhdf5 pads "mytag" to 8 bytes; parsing must not return the NULs, + // or copying the type would grow the tag. + let (dt, _) = Datatype::parse(&hex("15080000040000006d79746167000000")).unwrap(); + assert_eq!( + dt, + Datatype::Opaque { + size: 4, + tag: b"mytag".to_vec() + } + ); + let long = Datatype::Opaque { + size: 1, + tag: vec![b'x'; MAX_OPAQUE_TAG_LEN + 1], + }; + assert!(long.check_encodable().is_err()); + let ok = Datatype::Opaque { + size: 1, + tag: vec![b'x'; MAX_OPAQUE_TAG_LEN], + }; + assert!(ok.check_encodable().is_ok()); + assert_eq!(Datatype::parse(&ok.serialize()).unwrap().0, ok); + } + #[test] fn test_error_invalid_reference_type() { let buf = build_dt_header(7, 1, [5, 0, 0], 8); diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 644b64e..95d5834 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -4,7 +4,7 @@ //! link messages, contiguous datasets, inline and dense attributes. #[cfg(not(feature = "std"))] -use alloc::{string::String, string::ToString, vec, vec::Vec}; +use alloc::{format, string::String, string::ToString, vec, vec::Vec}; use crate::attribute::AttributeMessage; use crate::chunked_write::{ @@ -19,7 +19,7 @@ use crate::metadata_index::{DatasetMetadata, MetadataBlock, MetadataIndex}; use crate::object_header_writer::ObjectHeaderWriter; use crate::superblock::Superblock; use crate::type_builders::{ - DatasetBuilder, FillTime, FinishedGroup, GroupBuilder, build_attr_message, + DatasetBuilder, FinishedGroup, GroupBuilder, build_attr_message, fill_value_message, }; // Re-export public types that moved to type_builders for API compatibility. @@ -33,6 +33,49 @@ pub(crate) const OFFSET_SIZE: u8 = 8; pub(crate) const LENGTH_SIZE: u8 = 8; const SUPERBLOCK_SIZE: usize = 48; +/// Largest raw data a compact dataset can hold: the layout message (version, +/// class, 2-byte size, data) must fit an object header message, whose size +/// field is 2 bytes. Bigger "compact" requests fall back to contiguous storage. +const MAX_COMPACT_DATA_SIZE: usize = crate::object_header_writer::MAX_MESSAGE_SIZE - 4; + +/// libhdf5's bounds on a file space page size (`H5F_FILE_SPACE_PAGE_SIZE_MIN` +/// and `_MAX`). +const MIN_FILE_SPACE_PAGE_SIZE: u32 = 512; +const MAX_FILE_SPACE_PAGE_SIZE: u32 = 1024 * 1024 * 1024; + +/// Superblock extension object header for a file using the paged file-space +/// strategy: a single File Space Info message (0x0017), as libhdf5 writes it +/// for `fs_strategy="page"` without persisted free space. +fn build_paged_superblock_extension(page_size: u32) -> Result, FormatError> { + let mut fsinfo = Vec::new(); + fsinfo.push(1); // version + fsinfo.push(1); // strategy: H5F_FSPACE_STRATEGY_PAGE + fsinfo.push(0); // persisting free space: no + write_length(&mut fsinfo, 1, LENGTH_SIZE); // free-space section threshold + write_length(&mut fsinfo, u64::from(page_size), LENGTH_SIZE); + fsinfo.extend_from_slice(&0u16.to_le_bytes()); // page end metadata threshold + write_undef_offset(&mut fsinfo, OFFSET_SIZE); // EOA before free-space info + let mut w = ObjectHeaderWriter::new(); + // Flags as libhdf5 sets them: bit 2 (never share) and bit 4 (mark if + // unknown). Not constant: libhdf5 rewrites the message when it closes a + // file it opened for writing. + w.add_message_with_flags(MessageType::Unknown(0x0017), fsinfo, 0x14); + w.serialize() +} + +/// A group or dataset name must be one path component: not empty, not ".", +/// and without '/'. `FileWriter` writes a root group plus one level of +/// groups, and cannot create intermediate groups for a path. +fn check_link_name(name: &str) -> Result<(), FormatError> { + if name.is_empty() || name == "." || name.contains('/') { + return Err(FormatError::SerializationError(format!( + "invalid object name {name:?}: names must be a single path component \ + (FileWriter does not create nested groups)" + ))); + } + Ok(()) +} + /// Threshold for switching from compact (inline) to dense attribute storage. const DENSE_ATTR_THRESHOLD: usize = 8; @@ -50,12 +93,12 @@ pub(crate) fn build_chunked_dataset_oh( pipeline_message: Option<&[u8]>, attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, - fill_time: FillTime, -) -> Vec { + fill_message: &[u8], +) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01); w.add_message(MessageType::Dataspace, ds.serialize(LENGTH_SIZE)); - w.add_message_with_flags(MessageType::FillValue, vec![3, fill_time.to_byte()], 0x01); + w.add_message_with_flags(MessageType::FillValue, fill_message.to_vec(), 0x01); w.add_message(MessageType::DataLayout, layout_message.to_vec()); if let Some(pm) = pipeline_message { w.add_message(MessageType::FilterPipeline, pm.to_vec()); @@ -77,12 +120,12 @@ pub(crate) fn build_dataset_oh( data_size: u64, attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, - fill_time: FillTime, -) -> Vec { + fill_message: &[u8], +) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01); w.add_message(MessageType::Dataspace, ds.serialize(LENGTH_SIZE)); - w.add_message_with_flags(MessageType::FillValue, vec![3, fill_time.to_byte()], 0x01); + w.add_message_with_flags(MessageType::FillValue, fill_message.to_vec(), 0x01); let mut dl = Vec::new(); dl.push(4); // version dl.push(1); // class = contiguous @@ -112,12 +155,12 @@ pub(crate) fn build_compact_dataset_oh( data: &[u8], attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, - fill_time: FillTime, -) -> Vec { + fill_message: &[u8], +) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01); w.add_message(MessageType::Dataspace, ds.serialize(LENGTH_SIZE)); - w.add_message_with_flags(MessageType::FillValue, vec![3, fill_time.to_byte()], 0x01); + w.add_message_with_flags(MessageType::FillValue, fill_message.to_vec(), 0x01); // Compact layout message: version=4, class=0, u16 size, inline data let mut dl = Vec::new(); dl.push(4); // version @@ -140,7 +183,7 @@ pub(crate) fn build_group_oh( dense_link_info: Option<&[u8]>, attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, -) -> Vec { +) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); if let Some(li) = dense_link_info { // Dense link storage: a LinkInfo pointing at the fractal heap + name @@ -902,12 +945,12 @@ pub(crate) fn build_vds_dataset_oh( global_heap_addr: u64, attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, - fill_time: FillTime, -) -> Vec { + fill_message: &[u8], +) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01); w.add_message(MessageType::Dataspace, ds.serialize(LENGTH_SIZE)); - w.add_message_with_flags(MessageType::FillValue, vec![3, fill_time.to_byte()], 0x01); + w.add_message_with_flags(MessageType::FillValue, fill_message.to_vec(), 0x01); // VDS layout message: version=4, class=3, global_heap_address(8), global_heap_index=1(4) let mut dl = Vec::new(); dl.push(4u8); // version @@ -956,7 +999,9 @@ pub struct FileWriter { alignment_threshold: usize, /// Global alignment boundary in bytes (0 = disabled). alignment_bytes: usize, - /// Page size for page-buffer mode. When set, a v4 superblock is written. + /// File space page size. When set, the file uses libhdf5's paged + /// file-space strategy (a File Space Info message in the superblock + /// extension). page_size: Option, } @@ -988,9 +1033,16 @@ impl FileWriter { self } - /// Enable page-buffer mode with the given page size. Writing this causes - /// the file to be written with a v4 superblock (page_size field) instead - /// of the default v3. + /// Write the file with libhdf5's *paged* file-space strategy and the given + /// page size, as `H5Pset_file_space_strategy(H5F_FSPACE_STRATEGY_PAGE)` + + /// `H5Pset_file_space_page_size` (h5py: `fs_strategy="page"`, + /// `fs_page_size=...`) do: a v3 superblock with an extension holding a + /// File Space Info message, and the file padded to a whole number of + /// pages. Readers with a page buffer can then fetch metadata page by page. + /// + /// `page_size` must be between 512 bytes and 1 GiB (libhdf5's limits); + /// [`Self::finish`] fails otherwise. This used to write a "version 4" + /// superblock, which does not exist and no HDF5 library can open. pub fn with_page_size(&mut self, page_size: u32) -> &mut Self { self.page_size = Some(page_size); self @@ -1015,6 +1067,14 @@ impl FileWriter { pub fn finish(self) -> Result, FormatError> { let page_size = self.page_size; + if let Some(ps) = page_size + && !(MIN_FILE_SPACE_PAGE_SIZE..=MAX_FILE_SPACE_PAGE_SIZE).contains(&ps) + { + return Err(FormatError::SerializationError(format!( + "file space page size {ps} is outside libhdf5's \ + {MIN_FILE_SPACE_PAGE_SIZE}..={MAX_FILE_SPACE_PAGE_SIZE} bytes" + ))); + } struct DsFlat { name: String, dt: Datatype, @@ -1023,7 +1083,8 @@ impl FileWriter { attrs: Vec, chunk_options: ChunkOptions, maxshape: Option>, - fill_time: FillTime, + /// Serialized Fill Value message. + fill_message: Vec, compact: bool, alignment: usize, /// VDS source mappings (set for Virtual datasets). @@ -1073,6 +1134,7 @@ impl FileWriter { }; attrs.extend(p.build_attrs(&raw)); } + let fill_message = fill_value_message(db.fill_time, db.fill_value.as_deref(), &dt)?; Ok(DsFlat { name: db.name, dt, @@ -1081,13 +1143,26 @@ impl FileWriter { attrs, chunk_options: db.chunk_options, maxshape: db.maxshape, - fill_time: db.fill_time, + fill_message, compact: db.compact, alignment: db.alignment, virtual_sources: db.virtual_sources, }) }; + // Every name becomes a single link in its parent group. The writer + // has no nested groups, so a path like "a/b" would be stored as one + // link literally named "a/b" — which no HDF5 reader can resolve. + let root_names = self.root_datasets.iter().map(|d| d.name.as_str()); + let group_names = self.groups.iter().flat_map(|g| { + core::iter::once(g.name.as_str()) + .chain(g.datasets.iter().map(|d| d.name.as_str())) + .chain(g.external_links.iter().map(|l| l.0.as_str())) + }); + for name in root_names.chain(group_names) { + check_link_name(name)?; + } + let mut all_ds: Vec = Vec::new(); let mut groups: Vec = Vec::new(); let mut root_ds_indices: Vec = Vec::new(); @@ -1120,6 +1195,17 @@ impl FileWriter { root_attrs.push(build_attr_message(n, v)); } + // Every datatype must have an on-disk encoding before anything is laid + // out: `Datatype::serialize` itself cannot report a failure. + let group_attrs = groups.iter().flat_map(|g| &g.attrs); + let ds_attrs = all_ds.iter().flat_map(|d| &d.attrs); + for a in root_attrs.iter().chain(group_attrs).chain(ds_attrs) { + a.datatype.check_encodable()?; + } + for d in &all_ds { + d.dt.check_encodable()?; + } + let is_vds: Vec = all_ds.iter().map(|d| d.virtual_sources.is_some()).collect(); let is_chunked: Vec = all_ds .iter() @@ -1135,7 +1221,9 @@ impl FileWriter { let is_compact: Vec = all_ds .iter() .enumerate() - .map(|(i, d)| !is_vds[i] && !is_chunked[i] && d.compact && d.raw.len() <= 65535) + .map(|(i, d)| { + !is_vds[i] && !is_chunked[i] && d.compact && d.raw.len() <= MAX_COMPACT_DATA_SIZE + }) .collect(); let root_dense = root_attrs.len() > DENSE_ATTR_THRESHOLD; let group_dense: Vec = groups @@ -1174,9 +1262,9 @@ impl FileWriter { } let attr_blob = group_dense[gi].then(|| build_dense_attrs(&g.attrs, 0)); let dl = group_links_dense[gi].then_some(dummy_link_info.as_slice()); - build_group_oh(&dummy_links, dl, &g.attrs, attr_blob.as_ref()).len() + build_group_oh(&dummy_links, dl, &g.attrs, attr_blob.as_ref()).map(|oh| oh.len()) }) - .collect(); + .collect::>()?; let root_dummy_links: Vec = { let mut links = Vec::new(); @@ -1191,7 +1279,7 @@ impl FileWriter { let root_oh_size = { let attr_blob = root_dense.then(|| build_dense_attrs(&root_attrs, 0)); let dl = root_links_dense.then_some(dummy_link_info.as_slice()); - build_group_oh(&root_dummy_links, dl, &root_attrs, attr_blob.as_ref()).len() + build_group_oh(&root_dummy_links, dl, &root_attrs, attr_blob.as_ref())?.len() }; struct DataBlob { @@ -1219,8 +1307,8 @@ impl FileWriter { 0, // dummy address &d.attrs, dense_blob.as_ref(), - d.fill_time, - ); + &d.fill_message, + )?; // Global heap blob size is address-independent; compute it now // so pass 2 can place it correctly. let vds_mappings = d.virtual_sources.as_deref().unwrap_or(&[]); @@ -1263,8 +1351,8 @@ impl FileWriter { result.pipeline_message.as_deref(), &d.attrs, dense_blob.as_ref(), - d.fill_time, - ); + &d.fill_message, + )?; dummy_blobs.push(DataBlob { data: result.data_bytes, oh_bytes: oh, @@ -1282,8 +1370,8 @@ impl FileWriter { &d.raw, &d.attrs, dense_blob.as_ref(), - d.fill_time, - ); + &d.fill_message, + )?; dummy_blobs.push(DataBlob { data: vec![], oh_bytes: oh, @@ -1302,8 +1390,8 @@ impl FileWriter { d.raw.len() as u64, &d.attrs, dense_blob.as_ref(), - d.fill_time, - ); + &d.fill_message, + )?; dummy_blobs.push(DataBlob { data: d.raw.clone(), oh_bytes: oh, @@ -1315,12 +1403,12 @@ impl FileWriter { let actual_ds_oh_sizes: Vec = dummy_blobs.iter().map(|b| b.oh_bytes.len()).collect(); // Pass 2: compute real addresses - // v4 superblocks add a 4-byte page_size field before the checksum. - let superblock_size = if page_size.is_some() { - SUPERBLOCK_SIZE + 4 - } else { - SUPERBLOCK_SIZE - }; + // A paged file carries its File Space Info in a superblock extension + // object header, placed right after the superblock. + let sb_ext = page_size + .map(build_paged_superblock_extension) + .transpose()?; + let superblock_size = SUPERBLOCK_SIZE + sb_ext.as_ref().map_or(0, Vec::len); let root_group_addr = superblock_size as u64; let mut cursor2 = superblock_size + root_oh_size; @@ -1411,8 +1499,8 @@ impl FileWriter { heap_addr, &d.attrs, ds_dense_blobs[i].as_ref(), - d.fill_time, - ); + &d.fill_message, + )?; ds_blobs2.push(DataBlob { data: gcol_bytes.clone(), oh_bytes: oh, @@ -1438,8 +1526,8 @@ impl FileWriter { result.pipeline_message.as_deref(), &d.attrs, ds_dense_blobs[i].as_ref(), - d.fill_time, - ); + &d.fill_message, + )?; ds_blobs2.push(DataBlob { data: result.data_bytes, oh_bytes: oh, @@ -1453,8 +1541,8 @@ impl FileWriter { &d.raw, &d.attrs, ds_dense_blobs[i].as_ref(), - d.fill_time, - ); + &d.fill_message, + )?; ds_blobs2.push(DataBlob { data: vec![], oh_bytes: oh, @@ -1478,8 +1566,8 @@ impl FileWriter { d.raw.len() as u64, &d.attrs, ds_dense_blobs[i].as_ref(), - d.fill_time, - ); + &d.fill_message, + )?; let mut data = vec![0u8; padding]; data.extend_from_slice(&d.raw); cursor2 += d.raw.len(); @@ -1494,11 +1582,16 @@ impl FileWriter { let actual_ds_oh_sizes2: Vec = ds_blobs2.iter().map(|b| b.oh_bytes.len()).collect(); debug_assert_eq!(actual_ds_oh_sizes, actual_ds_oh_sizes2); + // libhdf5 ends a paged file on a page boundary. + let data_end = cursor2; + if let Some(ps) = page_size { + cursor2 = cursor2.next_multiple_of(ps as usize); + } let eof_addr2 = cursor2 as u64; let mut buf = Vec::with_capacity(cursor2); let sb = Superblock { - version: if page_size.is_some() { 4 } else { 3 }, + version: 3, offset_size: OFFSET_SIZE, length_size: LENGTH_SIZE, base_address: 0, @@ -1510,11 +1603,18 @@ impl FileWriter { free_space_address: None, driver_info_address: None, consistency_flags: 0, - superblock_extension_address: Some(u64::MAX), + superblock_extension_address: Some(if sb_ext.is_some() { + SUPERBLOCK_SIZE as u64 + } else { + u64::MAX + }), checksum: None, - page_size, + page_size: None, }; buf.extend_from_slice(&sb.serialize()); + if let Some(ref ext) = sb_ext { + buf.extend_from_slice(ext); + } // Root group OH let mut root_links: Vec = Vec::new(); @@ -1535,7 +1635,7 @@ impl FileWriter { root_dl, &root_attrs, root_dense_blob.as_ref(), - )); + )?); if let Some(ref b) = root_link_blob { buf.extend_from_slice(&b.blob); } @@ -1560,7 +1660,7 @@ impl FileWriter { dl, &g.attrs, group_dense_blobs[gi].as_ref(), - )); + )?); if let Some(ref b) = link_blob { buf.extend_from_slice(&b.blob); } @@ -1582,7 +1682,8 @@ impl FileWriter { buf.extend_from_slice(&blob.data); } - debug_assert_eq!(buf.len(), cursor2); + debug_assert_eq!(buf.len(), data_end); + buf.resize(cursor2, 0); Ok(buf) } } @@ -2156,7 +2257,8 @@ mod tests { } #[test] - fn file_writer_v4_superblock() { + fn file_writer_paged_file_uses_v3_superblock_and_fsinfo_extension() { + // This used to write superblock "version 4", which does not exist. let mut fw = FileWriter::new(); fw.with_page_size(4096); fw.create_dataset("data").with_f64_data(&[1.0, 2.0]); @@ -2164,8 +2266,31 @@ mod tests { let sig = signature::find_signature(&bytes).unwrap(); let sb = Superblock::parse(&bytes, sig).unwrap(); - assert_eq!(sb.version, 4, "expected superblock v4"); - assert_eq!(sb.page_size, Some(4096)); + assert_eq!(sb.version, 3); + assert_eq!(sb.superblock_extension_address, Some(48)); + assert_eq!(bytes.len() % 4096, 0); + assert_eq!(sb.eof_address, bytes.len() as u64); + let ext = ObjectHeader::parse(&bytes, 48, 8, 8).unwrap(); + let fsinfo = &ext.messages[0]; + assert_eq!(fsinfo.msg_type, MessageType::Unknown(0x0017)); + // Byte-for-byte what HDF5 2.0 writes for fs_strategy="page", + // fs_page_size=4096. + let mut expected = vec![1u8, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0]; + expected.extend_from_slice(&4096u64.to_le_bytes()); + expected.extend_from_slice(&[0, 0]); + expected.extend_from_slice(&[0xff; 8]); + assert_eq!(fsinfo.data, expected); + assert_eq!(fsinfo.flags, 0x14); + assert_eq!(read_dataset_f64(&bytes, "data"), vec![1.0, 2.0]); + } + + #[test] + fn file_writer_rejects_page_sizes_libhdf5_would() { + for ps in [0u32, 511, MAX_FILE_SPACE_PAGE_SIZE + 1] { + let mut fw = FileWriter::new(); + fw.with_page_size(ps); + assert!(fw.finish().is_err(), "page size {ps}"); + } } #[test] diff --git a/crates/clawhdf5-format/src/fill_value.rs b/crates/clawhdf5-format/src/fill_value.rs index d10ba47..3b18790 100644 --- a/crates/clawhdf5-format/src/fill_value.rs +++ b/crates/clawhdf5-format/src/fill_value.rs @@ -98,15 +98,50 @@ pub fn parse_fill_value(msg: &HeaderMessage) -> Result>, FormatEr /// The fill value that applies to a dataset given its header messages. The new /// message wins over the old one when both are present. +/// +/// A *shared* fill value message holds only a reference to the real message, +/// which cannot be followed without the file: this returns +/// [`FormatError::UnresolvedSharedMessage`] for one (it used to answer "zeros"). +/// Use [`dataset_fill_value_in`] when the file bytes are at hand. pub fn dataset_fill_value(messages: &[HeaderMessage]) -> Result>, FormatError> { + fill_value_from(messages, |_| Err(FormatError::UnresolvedSharedMessage)) +} + +/// [`dataset_fill_value`] for a dataset in `file_data`, following a shared +/// fill value message to where it lives: another object header, or the +/// file's shared-message (SOHM) heap, as libhdf5 writes it when the file has +/// a SOHM index for fill values. +pub fn dataset_fill_value_in( + file_data: &[u8], + messages: &[HeaderMessage], + offset_size: u8, + length_size: u8, +) -> Result>, FormatError> { + fill_value_from(messages, |msg| { + crate::shared_message::message_data_with_sohm(file_data, msg, offset_size, length_size) + .map(|data| data.into_owned()) + }) +} + +fn fill_value_from( + messages: &[HeaderMessage], + resolve_shared: impl Fn(&HeaderMessage) -> Result, FormatError>, +) -> Result>, FormatError> { for wanted in [MessageType::FillValue, MessageType::FillValueOld] { if let Some(msg) = messages.iter().find(|m| m.msg_type == wanted) { - if crate::shared_message::is_shared(msg.flags) { - // A shared fill value is legal but vanishingly rare; treat it - // as the default rather than misparsing the reference. - return Ok(None); - } - if let Some(value) = parse_fill_value(msg)? { + let value = if crate::shared_message::is_shared(msg.flags) { + let data = resolve_shared(msg)?; + parse_fill_value(&HeaderMessage { + msg_type: msg.msg_type, + size: data.len(), + flags: msg.flags & !0x02, + creation_order: msg.creation_order, + data, + })? + } else { + parse_fill_value(msg)? + }; + if let Some(value) = value { return Ok(Some(value)); } } @@ -174,7 +209,7 @@ pub fn read_full_with_fill>( { return Err(FormatError::ExternalDataFilesUnsupported.into()); } - let fill = dataset_fill_value(messages)?; + let fill = dataset_fill_value_in(file_data, messages, offset_size, length_size)?; if !has_storage(layout) { return Ok(filled_dataset(dataspace, elem_size, fill.as_deref())?); } diff --git a/crates/clawhdf5-format/src/object_header.rs b/crates/clawhdf5-format/src/object_header.rs index 8306185..4b8067f 100644 --- a/crates/clawhdf5-format/src/object_header.rs +++ b/crates/clawhdf5-format/src/object_header.rs @@ -146,12 +146,7 @@ impl ObjectHeader { ensure_len(data, pos, msg_data_size)?; let msg_type = MessageType::from_u16(msg_type_raw); - // Check if unknown + must-understand (bit 3 of msg_flags) - if let MessageType::Unknown(id) = msg_type - && msg_flags & 0x08 != 0 - { - return Err(FormatError::UnsupportedMessage(id)); - } + check_unknown_message(msg_type, msg_flags)?; if msg_type != MessageType::Nil { messages.push(HeaderMessage { @@ -229,11 +224,7 @@ impl ObjectHeader { let msg_type = MessageType::from_u16(msg_type_raw); - if let MessageType::Unknown(id) = msg_type - && msg_flags & 0x08 != 0 - { - return Err(FormatError::UnsupportedMessage(id)); - } + check_unknown_message(msg_type, msg_flags)?; if msg_type != MessageType::Nil { messages.push(HeaderMessage { @@ -424,11 +415,7 @@ impl ObjectHeader { let msg_type = MessageType::from_u16(msg_type_raw); - if let MessageType::Unknown(id) = msg_type - && msg_flags & 0x08 != 0 - { - return Err(FormatError::UnsupportedMessage(id)); - } + check_unknown_message(msg_type, msg_flags)?; let msg_data = data[pos..pos + msg_data_size].to_vec(); @@ -509,6 +496,24 @@ impl ObjectHeader { } } +/// Header message flag bit 7: fail if the message is unknown, always. +const MSG_FLAG_FAIL_IF_UNKNOWN_ALWAYS: u8 = 0x80; + +/// Refuse an unknown message the file says no reader may skip. +/// +/// The parser only ever reads, so bit 3 (fail only when opened for writing) +/// is ignored, as libhdf5 ignores it for a read-only open; bit 7 fails +/// regardless of access mode. This had the two the wrong way round, failing +/// objects libhdf5 reads and reading ones it refuses (`tbogus.h5`). +fn check_unknown_message(msg_type: MessageType, msg_flags: u8) -> Result<(), FormatError> { + match msg_type { + MessageType::Unknown(id) if msg_flags & MSG_FLAG_FAIL_IF_UNKNOWN_ALWAYS != 0 => { + Err(FormatError::UnsupportedMessage(id)) + } + _ => Ok(()), + } +} + #[cfg(test)] mod tests { use super::*; @@ -632,14 +637,38 @@ mod tests { } #[test] - fn parse_v1_unknown_must_understand_errors() { - // Bit 3 of msg_flags = must understand - let messages = [(0x00FFu16, &[0xAA][..], 0x08u8)]; + fn parse_v1_unknown_fail_always_errors() { + // Bit 7 of msg_flags = fail if unknown, whatever the access mode. + let messages = [(0x00FFu16, &[0xAA][..], 0x80u8)]; let data = build_v1_header(&messages, 8, 8); let err = ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(); assert_eq!(err, FormatError::UnsupportedMessage(0x00FF)); } + #[test] + fn parse_v1_unknown_fail_on_write_is_ignored_when_reading() { + // Bit 3 = fail if unknown *and the file is opened for writing*. This + // parser only reads, so libhdf5 (read-only) opens such an object and + // so must we. Bits 4/5 (mark if unknown / was unknown) never fail. + for flags in [0x08u8, 0x10, 0x20, 0x38] { + let messages = [(0x00FFu16, &[0xAA][..], flags)]; + let data = build_v1_header(&messages, 8, 8); + let hdr = ObjectHeader::parse(&data, 0, 8, 8).unwrap(); + assert_eq!(hdr.messages[0].msg_type, MessageType::Unknown(0x00FF)); + } + } + + #[test] + fn parse_v2_unknown_message_flags() { + let data = build_v2_header(0x00, &[(0xF0, &[1, 2], 0x08)], None); + assert!(ObjectHeader::parse(&data, 0, 8, 8).is_ok()); + let data = build_v2_header(0x00, &[(0xF0, &[1, 2], 0x80)], None); + assert_eq!( + ObjectHeader::parse(&data, 0, 8, 8).unwrap_err(), + FormatError::UnsupportedMessage(0xF0) + ); + } + #[test] fn parse_v2_no_timestamps_one_message() { let data = build_v2_header(0x00, &[(0x01, &[10, 20], 0)], None); diff --git a/crates/clawhdf5-format/src/object_header_writer.rs b/crates/clawhdf5-format/src/object_header_writer.rs index 9142452..8d52ad4 100644 --- a/crates/clawhdf5-format/src/object_header_writer.rs +++ b/crates/clawhdf5-format/src/object_header_writer.rs @@ -1,11 +1,17 @@ //! Object header writer for v2 format. #[cfg(not(feature = "std"))] -use alloc::vec::Vec; +use alloc::{format, vec::Vec}; use crate::checksum::jenkins_lookup3; +use crate::error::FormatError; use crate::message_type::MessageType; +/// Largest message payload a v2 object header can describe: the per-message +/// size field is 2 bytes. A bigger message cannot be encoded at all — writing +/// its size truncated to 16 bits produced files libhdf5 refuses. +pub const MAX_MESSAGE_SIZE: usize = u16::MAX as usize; + /// Writer for v2 object headers with proper checksums. pub struct ObjectHeaderWriter { messages: Vec<(MessageType, Vec, u8)>, // (type, data, msg_flags) @@ -30,7 +36,22 @@ impl ObjectHeaderWriter { } /// Serialize the complete v2 object header (OHDR + messages + checksum). - pub fn serialize(&self) -> Vec { + /// + /// Fails with [`FormatError::SerializationError`] when a message is larger + /// than [`MAX_MESSAGE_SIZE`] (e.g. an attribute over ~64 KiB, which would + /// need dense attribute storage), rather than writing a corrupt header. + pub fn serialize(&self) -> Result, FormatError> { + if let Some((msg_type, data, _)) = self + .messages + .iter() + .find(|(_, data, _)| data.len() > MAX_MESSAGE_SIZE) + { + return Err(FormatError::SerializationError(format!( + "{msg_type:?} message is {} bytes; an object header message holds at most \ + {MAX_MESSAGE_SIZE} bytes", + data.len() + ))); + } // Calculate total message bytes: each message has type(1) + size(2) + flags(1) + data let msg_bytes_total: usize = self .messages @@ -80,7 +101,7 @@ impl ObjectHeaderWriter { let checksum = jenkins_lookup3(&buf); buf.extend_from_slice(&checksum.to_le_bytes()); - buf + Ok(buf) } } @@ -125,15 +146,22 @@ impl BatchObjectHeaderWriter { /// Compute the serialized size of each header without actually serializing. /// Returns sizes in the same order as headers were added. - pub fn compute_sizes(&self) -> Vec { - self.headers.iter().map(|h| h.serialize().len()).collect() + pub fn compute_sizes(&self) -> Result, FormatError> { + self.headers + .iter() + .map(|h| h.serialize().map(|b| b.len())) + .collect() } /// Serialize all headers into a single contiguous buffer. /// Returns `(combined_bytes, offsets)` where `offsets[i]` is the byte /// offset of header `i` within the combined buffer. - pub fn serialize_all(&self) -> (Vec, Vec) { - let serialized: Vec> = self.headers.iter().map(|h| h.serialize()).collect(); + pub fn serialize_all(&self) -> Result<(Vec, Vec), FormatError> { + let serialized: Vec> = self + .headers + .iter() + .map(|h| h.serialize()) + .collect::>()?; let total: usize = serialized.iter().map(|s| s.len()).sum(); let mut buf = Vec::with_capacity(total); let mut offsets = Vec::with_capacity(serialized.len()); @@ -141,7 +169,7 @@ impl BatchObjectHeaderWriter { offsets.push(buf.len()); buf.extend_from_slice(s); } - (buf, offsets) + Ok((buf, offsets)) } } @@ -159,7 +187,7 @@ mod tests { #[test] fn empty_header_roundtrip() { let writer = ObjectHeaderWriter::new(); - let bytes = writer.serialize(); + let bytes = writer.serialize().unwrap(); let hdr = ObjectHeader::parse(&bytes, 0, 8, 8).unwrap(); assert_eq!(hdr.version, 2); assert_eq!(hdr.messages.len(), 0); @@ -170,7 +198,7 @@ mod tests { let mut writer = ObjectHeaderWriter::new(); writer.add_message(MessageType::Dataspace, vec![1, 2, 3, 4]); writer.add_message(MessageType::Datatype, vec![5, 6]); - let bytes = writer.serialize(); + let bytes = writer.serialize().unwrap(); let hdr = ObjectHeader::parse(&bytes, 0, 8, 8).unwrap(); assert_eq!(hdr.messages.len(), 2); assert_eq!(hdr.messages[0].msg_type, MessageType::Dataspace); @@ -184,12 +212,30 @@ mod tests { let mut writer = ObjectHeaderWriter::new(); // Add a message with >255 bytes of payload writer.add_message(MessageType::Datatype, vec![0xAA; 300]); - let bytes = writer.serialize(); + let bytes = writer.serialize().unwrap(); let hdr = ObjectHeader::parse(&bytes, 0, 8, 8).unwrap(); assert_eq!(hdr.messages.len(), 1); assert_eq!(hdr.messages[0].data.len(), 300); } + #[test] + fn oversized_message_is_an_error_not_a_truncated_size() { + // 65535 bytes is the largest encodable payload. + let mut writer = ObjectHeaderWriter::new(); + writer.add_message(MessageType::Attribute, vec![0; MAX_MESSAGE_SIZE]); + let bytes = writer.serialize().unwrap(); + let hdr = ObjectHeader::parse(&bytes, 0, 8, 8).unwrap(); + assert_eq!(hdr.messages[0].data.len(), MAX_MESSAGE_SIZE); + + // One byte more used to be written with its size wrapped to 0. + let mut writer = ObjectHeaderWriter::new(); + writer.add_message(MessageType::Attribute, vec![0; MAX_MESSAGE_SIZE + 1]); + assert!(matches!( + writer.serialize(), + Err(FormatError::SerializationError(_)) + )); + } + #[test] fn batch_writer_serialize_all() { let mut batch = BatchObjectHeaderWriter::new(); @@ -204,7 +250,7 @@ mod tests { batch.add(w2); assert_eq!(batch.len(), 2); - let (buf, offsets) = batch.serialize_all(); + let (buf, offsets) = batch.serialize_all().unwrap(); assert_eq!(offsets.len(), 2); assert_eq!(offsets[0], 0); @@ -222,7 +268,7 @@ mod tests { fn batch_writer_empty() { let batch = BatchObjectHeaderWriter::new(); assert!(batch.is_empty()); - let (buf, offsets) = batch.serialize_all(); + let (buf, offsets) = batch.serialize_all().unwrap(); assert!(buf.is_empty()); assert!(offsets.is_empty()); } diff --git a/crates/clawhdf5-format/src/property_list.rs b/crates/clawhdf5-format/src/property_list.rs index 15ea6b9..adf21d4 100644 --- a/crates/clawhdf5-format/src/property_list.rs +++ b/crates/clawhdf5-format/src/property_list.rs @@ -43,7 +43,7 @@ impl Default for DatasetCreateProps { fletcher32: false, lz4: false, zstd_level: None, - fill_time: FillTime::Alloc, + fill_time: FillTime::IfSet, compact: false, alignment: 0, } @@ -335,7 +335,7 @@ mod tests { fn dcpl_defaults() { let dcpl = DatasetCreateProps::new(); assert!(dcpl.chunk_dims.is_none()); - assert_eq!(dcpl.fill_time, FillTime::Alloc); + assert_eq!(dcpl.fill_time, FillTime::IfSet); assert!(!dcpl.compact); } diff --git a/crates/clawhdf5-format/src/shared_message.rs b/crates/clawhdf5-format/src/shared_message.rs index 954618d..33334fa 100644 --- a/crates/clawhdf5-format/src/shared_message.rs +++ b/crates/clawhdf5-format/src/shared_message.rs @@ -225,9 +225,12 @@ pub fn parse_sohm_table_message( /// Parse the SOHM table structure (signature "SMTB") from the file. /// -/// Each index entry: index_type(1) + mesg_types(2) + min_mesg_size(4) + -/// list_max(2) + btree_min(2) + num_messages(2) + index_addr(offset_size) + -/// heap_addr(offset_size) +/// Each index entry: version(1) + index_type(1) + mesg_types(2) + +/// min_mesg_size(4) + list_max(2) + btree_min(2) + num_messages(2) + +/// index_addr(offset_size) + heap_addr(offset_size) +/// +/// The leading per-index version byte (0) was missing here, so every field +/// after it was read one byte off — verified against an HDF5 2.0 file. pub fn parse_sohm_table( file_data: &[u8], table_addr: usize, @@ -240,11 +243,16 @@ pub fn parse_sohm_table( } let mut pos = table_addr + 4; let os = offset_size as usize; - let entry_size = 1 + 2 + 4 + 2 + 2 + 2 + os + os; // 13 + 2*offset_size + let entry_size = 1 + 1 + 2 + 4 + 2 + 2 + 2 + os + os; // 14 + 2*offset_size let mut indexes = Vec::with_capacity(nindexes as usize); for _ in 0..nindexes { ensure_len(file_data, pos, entry_size)?; + let version = file_data[pos]; + if version != 0 { + return Err(FormatError::InvalidSohmTableVersion(version)); + } + pos += 1; let index_type = file_data[pos]; pos += 1; let mesg_types = u16::from_le_bytes([file_data[pos], file_data[pos + 1]]); @@ -381,6 +389,68 @@ pub fn parse_sohm_btree_entries( // ---- SOHM resolution ---- /// Find the SOHM index that handles the given message type. +/// Load a file's SOHM table: superblock → superblock extension → Shared +/// Message Table message → SMTB. `Ok(None)` when the file has no superblock +/// extension or no shared-message table. +pub fn load_sohm_table( + file_data: &[u8], + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + let sig = crate::signature::find_signature(file_data)?; + let sb = crate::superblock::Superblock::parse(file_data, sig)?; + let Some(ext_addr) = sb + .superblock_extension_address + .filter(|&a| !is_undefined(a, offset_size)) + else { + return Ok(None); + }; + let ext = ObjectHeader::parse(file_data, ext_addr as usize, offset_size, length_size)?; + let Some(msg) = ext + .messages + .iter() + .find(|m| m.msg_type == MessageType::SharedMessageTable) + else { + return Ok(None); + }; + let table_msg = parse_sohm_table_message(&msg.data, offset_size)?; + parse_sohm_table( + file_data, + table_msg.table_address as usize, + table_msg.nindexes, + offset_size, + ) + .map(Some) +} + +/// Like [`message_data`], but also follows references into the file's SOHM +/// heap (shared object header messages), loading the SOHM table on demand. +pub fn message_data_with_sohm<'a>( + file_data: &[u8], + msg: &'a crate::object_header::HeaderMessage, + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + if !is_shared(msg.flags) { + return Ok(Cow::Borrowed(&msg.data)); + } + let shared_ref = parse_shared_ref(&msg.data, offset_size)?; + let table = if shared_ref.heap_id.is_some() { + load_sohm_table(file_data, offset_size, length_size)? + } else { + None + }; + resolve_shared_message_with_sohm( + file_data, + &shared_ref, + msg.msg_type, + offset_size, + length_size, + table.as_ref(), + ) + .map(Cow::Owned) +} + fn find_index_for_msg_type(table: &SohmTable, msg_type: MessageType) -> Option<&SohmIndex> { let type_bit = 1u16 << msg_type.to_u16(); table @@ -707,6 +777,7 @@ mod tests { let mut buf = Vec::new(); buf.extend_from_slice(b"SMTB"); for idx in indexes { + buf.push(0); // version buf.push(idx.index_type); buf.extend_from_slice(&idx.mesg_types.to_le_bytes()); buf.extend_from_slice(&idx.min_mesg_size.to_le_bytes()); diff --git a/crates/clawhdf5-format/src/superblock.rs b/crates/clawhdf5-format/src/superblock.rs index aedcc55..e971495 100644 --- a/crates/clawhdf5-format/src/superblock.rs +++ b/crates/clawhdf5-format/src/superblock.rs @@ -39,7 +39,13 @@ pub struct Superblock { pub superblock_extension_address: Option, /// CRC32C checksum (v2/v3 only). pub checksum: Option, - /// Page size for page-buffer mode (v4 only). `None` for v0–v3. + /// Page size of the non-standard "version 4" superblock layout (v4 only). + /// `None` for v0–v3. + /// + /// HDF5 has no superblock version 4 — libhdf5 refuses it. A real paged + /// file is a v2/v3 superblock whose extension holds a File Space Info + /// message (what `FileWriter::with_page_size` writes). This field is kept + /// only so such files written by older clawhdf5 versions still parse. pub page_size: Option, } @@ -127,8 +133,9 @@ impl Superblock { /// Serialize this superblock to bytes. /// - /// Writes v2/v3 format, or v4 (with `page_size`) when `self.version == 4`. - /// Computes and appends Jenkins lookup3 checksum. + /// Writes v2/v3 format, or the non-standard v4 (with `page_size`) when + /// `self.version == 4` — which no HDF5 library opens; see + /// [`Self::page_size`]. Computes and appends Jenkins lookup3 checksum. pub fn serialize(&self) -> Vec { let mut buf = Vec::with_capacity(48); buf.extend_from_slice(&HDF5_SIGNATURE); diff --git a/crates/clawhdf5-format/src/type_builders.rs b/crates/clawhdf5-format/src/type_builders.rs index e698899..09eaeba 100644 --- a/crates/clawhdf5-format/src/type_builders.rs +++ b/crates/clawhdf5-format/src/type_builders.rs @@ -15,29 +15,81 @@ use crate::datatype::{ /// Controls when fill values are written to dataset storage. /// -/// Corresponds to the HDF5 fill value message's "fill time" field. +/// Corresponds to the HDF5 fill value message's "fill time" field +/// (`H5D_fill_time_t`). #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum FillTime { - /// Never write fill values (0x02). Avoids initialization overhead - /// for datasets that will be fully written before any read. + /// Never write fill values (`H5D_FILL_TIME_NEVER`). Avoids + /// initialization overhead for datasets that will be fully written + /// before any read. Never, - /// Write fill values at allocation time (0x0a). This is the default - /// and matches the HDF5 C library's behavior. - #[default] + /// Write fill values when storage is allocated (`H5D_FILL_TIME_ALLOC`). Alloc, - /// Write fill values only when the fill value has been explicitly set (0x06). + /// Write fill values at allocation only if one was set explicitly + /// (`H5D_FILL_TIME_IFSET`). The default, as in the HDF5 C library. + #[default] IfSet, } +/// Space allocation time written with every fill value message: late +/// (`H5D_ALLOC_TIME_LATE`), bits 0-1 of the flags byte. +const ALLOC_TIME_LATE: u8 = 2; + impl FillTime { - /// Serialize to the byte used in the fill value message (version 3). + /// Serialize to the flags byte of a version 3 fill value message: the + /// space allocation time (late) in bits 0-1 and the fill time in bits + /// 2-3 (`H5D_FILL_TIME_ALLOC` = 0, `NEVER` = 1, `IFSET` = 2). + /// + /// This used to put `Never` in the ALLOC slot, `Alloc` in IFSET and + /// `IfSet` in NEVER, so libhdf5 saw every choice as a different one. pub fn to_byte(self) -> u8 { - match self { - FillTime::Never => 0x02, - FillTime::Alloc => 0x0a, - FillTime::IfSet => 0x06, + ALLOC_TIME_LATE | (self.code() << 2) + } + + /// Decode the fill time from a version 3 fill value message's flags. + pub fn from_byte(flags: u8) -> Option { + match (flags >> 2) & 0x03 { + 0 => Some(FillTime::Alloc), + 1 => Some(FillTime::Never), + 2 => Some(FillTime::IfSet), + _ => None, } } + + fn code(self) -> u8 { + match self { + FillTime::Alloc => 0, + FillTime::Never => 1, + FillTime::IfSet => 2, + } + } +} + +/// Serialize a version 3 Fill Value message for a dataset of `dt`: the fill +/// time, and the user-defined fill value if there is one (bit 5). +pub(crate) fn fill_value_message( + fill_time: FillTime, + value: Option<&[u8]>, + dt: &Datatype, +) -> Result, crate::error::FormatError> { + let mut msg = vec![3, fill_time.to_byte()]; + if let Some(value) = value { + if matches!(dt, Datatype::VariableLength { .. }) { + return Err(crate::error::FormatError::SerializationError( + "a fill value for a variable-length datatype is not supported".into(), + )); + } + if value.len() != dt.type_size() as usize { + return Err(crate::error::FormatError::DataSizeMismatch { + expected: dt.type_size() as usize, + actual: value.len(), + }); + } + msg[1] |= 0x20; // fill value defined + msg.extend_from_slice(&(value.len() as u32).to_le_bytes()); + msg.extend_from_slice(value); + } + Ok(msg) } // ---- Datatype constructors ---- @@ -332,7 +384,11 @@ pub(crate) fn build_attr_message(name: &str, value: &AttrValue) -> AttributeMess raw_data: data.clone(), }, AttrValue::String(s) => { - let bytes = s.as_bytes(); + // A fixed-length string type must be at least 1 byte: libhdf5 + // rejects size 0 ("invalid datatype size") and with it every + // attribute on the object. h5py stores "" as one NUL byte. + let mut bytes = s.as_bytes().to_vec(); + bytes.resize(bytes.len().max(1), 0); AttributeMessage { name: name.to_string(), datatype: Datatype::String { @@ -341,11 +397,12 @@ pub(crate) fn build_attr_message(name: &str, value: &AttrValue) -> AttributeMess charset: CharacterSet::Utf8, }, dataspace: scalar_ds(), - raw_data: bytes.to_vec(), + raw_data: bytes, } } AttrValue::StringArray(arr) => { - let max_len = arr.iter().map(|s| s.len()).max().unwrap_or(0); + // At least 1 byte per element, as for a single string. + let max_len = arr.iter().map(|s| s.len()).max().unwrap_or(0).max(1); let mut raw = Vec::new(); for s in arr { let mut b = s.as_bytes().to_vec(); @@ -431,8 +488,10 @@ pub struct DatasetBuilder { pub(crate) data: Option>, pub(crate) attrs: Vec<(String, AttrValue)>, pub(crate) chunk_options: ChunkOptions, - /// Controls when fill values are written. Default is `FillTime::Alloc`. + /// Controls when fill values are written. Default is `FillTime::IfSet`. pub(crate) fill_time: FillTime, + /// User-defined fill value: one element's bytes, as stored. + pub(crate) fill_value: Option>, /// Use compact (inline) storage: data is stored in the object header. /// Only valid when raw data is <= 65536 bytes and dataset is not chunked. pub(crate) compact: bool, @@ -459,6 +518,7 @@ impl DatasetBuilder { attrs: Vec::new(), chunk_options: ChunkOptions::default(), fill_time: FillTime::default(), + fill_value: None, compact: false, alignment: 0, virtual_sources: None, @@ -715,10 +775,20 @@ impl DatasetBuilder { self } + /// Set the dataset's fill value: what readers return for storage that + /// was never written (e.g. after the dataset is extended). `value` is one + /// element's bytes as stored — the dataset datatype's size and byte order + /// (`(-1i32).to_le_bytes()` for an `i32` dataset). A size mismatch, or a + /// variable-length datatype, makes `finish` fail. + pub fn with_fill_value(&mut self, value: &[u8]) -> &mut Self { + self.fill_value = Some(value.to_vec()); + self + } + /// Use compact (inline) storage for this dataset. /// /// The raw data is stored directly in the dataset's object header rather - /// than as a separate data blob. Only effective when raw data <= 65536 bytes + /// than as a separate data blob. Only effective when raw data <= 65531 bytes /// and the dataset is not chunked. pub fn compact(&mut self) -> &mut Self { self.compact = true; diff --git a/crates/clawhdf5-format/tests/fixtures/gen_shared_fill.py b/crates/clawhdf5-format/tests/fixtures/gen_shared_fill.py new file mode 100644 index 0000000..431719c --- /dev/null +++ b/crates/clawhdf5-format/tests/fixtures/gen_shared_fill.py @@ -0,0 +1,49 @@ +"""Generate shared_fill_value.h5: datasets whose Fill Value message is +*shared*, in the two ways libhdf5 can share one. + +- /sohm_a, /sohm_b: the file has a shared-object-header-message (SOHM) index + for fill values, so libhdf5 stores the fill value (-7, int32) in the SOHM + heap and /sohm_b's header holds only a reference to it. Chunked, with only + the first chunk written, so the rest reads as the fill value. +- /unwritten_a, /unwritten_b: the same, never written: no storage at all, + read entirely as the fill value. + +h5py has no API for SOHM indexes, so the file creation property list is +configured by calling the libhdf5 bundled in the h5py wheel through ctypes. +Written with h5py 3.16.0 / HDF5 2.0.0. Re-run only to regenerate: + + python gen_shared_fill.py shared_fill_value.h5 +""" +import ctypes +import glob +import os +import sys + +import h5py +import numpy as np + +libdir = os.path.join(os.path.dirname(os.path.dirname(h5py.__file__)), "h5py.libs") +libs = [p for p in glob.glob(os.path.join(libdir, "libhdf5*.so*")) if "_hl" not in os.path.basename(p)] +lib = ctypes.CDLL(libs[0]) +lib.H5open() + +H5O_SHMESG_FILL_FLAG = 1 << 0x0005 + +fcpl = h5py.h5p.create(h5py.h5p.FILE_CREATE) +lib.H5Pset_shared_mesg_nindexes.argtypes = [ctypes.c_int64, ctypes.c_uint] +lib.H5Pset_shared_mesg_index.argtypes = [ctypes.c_int64, ctypes.c_uint, ctypes.c_uint, ctypes.c_uint] +assert lib.H5Pset_shared_mesg_nindexes(fcpl.id, 1) >= 0 +assert lib.H5Pset_shared_mesg_index(fcpl.id, 0, H5O_SHMESG_FILL_FLAG, 0) >= 0 + +fapl = h5py.h5p.create(h5py.h5p.FILE_ACCESS) +fapl.set_libver_bounds(h5py.h5f.LIBVER_LATEST, h5py.h5f.LIBVER_LATEST) +fid = h5py.h5f.create(sys.argv[1].encode(), h5py.h5f.ACC_TRUNC, fcpl=fcpl, fapl=fapl) +with h5py.File(fid) as f: + # Chunked, with only the first chunk written: the rest reads as fill. + # libhdf5 keeps the first copy of a message in its own header; the second + # identical one (the `_b` datasets) is the SOHM reference. + for name in ("sohm_a", "sohm_b"): + d = f.create_dataset(name, shape=(8,), chunks=(4,), dtype=" (Superblock, ObjectHeader) { + let sig = signature::find_signature(bytes).unwrap(); + let sb = Superblock::parse(bytes, sig).unwrap(); + let addr = if path == "/" { + sb.root_group_address + } else { + resolve_path_any(bytes, &sb, path).unwrap() + }; + let oh = ObjectHeader::parse(bytes, addr as usize, sb.offset_size, sb.length_size).unwrap(); + (sb, oh) +} + +fn layout_of(bytes: &[u8], path: &str) -> DataLayout { + let (sb, oh) = header_at(bytes, path); + let msg = oh + .messages + .iter() + .find(|m| m.msg_type == MessageType::DataLayout) + .unwrap(); + DataLayout::parse(&msg.data, sb.offset_size, sb.length_size).unwrap() +} + +fn python() -> String { + std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string()) +} + +fn write_tmp(name: &str, bytes: &[u8]) -> std::path::PathBuf { + let path = std::env::temp_dir().join(format!("clawhdf5_writer_meta_{name}.h5")); + std::fs::write(&path, bytes).unwrap(); + path +} + +/// Run `script` (with `path` bound to the file) under h5py; return stdout. +fn h5py(path: &std::path::Path, script: &str) -> String { + let full = format!( + "import h5py, numpy as np, json\npath = {:?}\n{script}", + path.display().to_string() + ); + let o = std::process::Command::new(python()) + .args(["-c", &full]) + .output() + .expect("python interpreter"); + assert!( + o.status.success(), + "h5py failed: {}", + String::from_utf8_lossy(&o.stderr) + ); + String::from_utf8(o.stdout).unwrap().trim().to_string() +} + +/// `h5dump` must read the whole file without error. +fn h5dump_ok(path: &std::path::Path) { + let o = std::process::Command::new("h5dump") + .arg(path) + .output() + .expect("h5dump"); + assert!( + o.status.success(), + "h5dump failed: {}{}", + String::from_utf8_lossy(&o.stdout), + String::from_utf8_lossy(&o.stderr) + ); +} + +fn u8_ramp(n: usize) -> Vec { + (0..n).map(|i| (i % 251) as u8).collect() +} + +// ---- 1. object header message size limit ---- + +#[test] +fn attribute_too_big_for_a_header_message_is_an_error() { + // Measured: a 70000-byte attribute was written with its message size + // wrapped to 16 bits, and libhdf5 refused the whole root group. + let mut fw = FileWriter::new(); + fw.set_root_attr( + "a", + AttrValue::Raw { + datatype: make_u8_type(), + shape: vec![70_000], + data: u8_ramp(70_000), + }, + ); + assert!(fw.finish().is_err()); + + // 65500 bytes still fits and still works. + let mut fw = FileWriter::new(); + fw.set_root_attr( + "a", + AttrValue::Raw { + datatype: make_u8_type(), + shape: vec![65_500], + data: u8_ramp(65_500), + }, + ); + let bytes = fw.finish().unwrap(); + let (sb, oh) = header_at(&bytes, "/"); + let attrs = clawhdf5_format::attribute::extract_attributes(&oh, sb.length_size).unwrap(); + assert_eq!(attrs[0].raw_data, u8_ramp(65_500)); +} + +#[test] +fn compact_layout_falls_back_to_contiguous_past_the_message_limit() { + // Layout message = 4 bytes + data; data may be at most 65531 bytes. + for (n, compact) in [(65_531, true), (65_532, false), (65_534, false)] { + let mut fw = FileWriter::new(); + fw.create_dataset("d").with_u8_data(&u8_ramp(n)).compact(); + let bytes = fw.finish().unwrap(); + match layout_of(&bytes, "d") { + DataLayout::Compact { data } => { + assert!(compact, "{n} bytes must not be compact"); + assert_eq!(data, u8_ramp(n)); + } + DataLayout::Contiguous { .. } => assert!(!compact, "{n} bytes should be compact"), + other => panic!("unexpected layout {other:?}"), + } + } +} + +#[test] +#[ignore = "requires Python h5py module and h5dump"] +fn h5py_reads_compact_datasets_at_the_limit() { + for n in [65_531usize, 65_534] { + let mut fw = FileWriter::new(); + fw.create_dataset("d").with_u8_data(&u8_ramp(n)).compact(); + let path = write_tmp(&format!("compact_{n}"), &fw.finish().unwrap()); + let out = h5py( + &path, + "f = h5py.File(path, 'r'); v = f['d'][()]\n\ + print(bool((v == (np.arange(v.size) % 251).astype(np.uint8)).all()), v.size)", + ); + assert_eq!(out, format!("True {n}")); + h5dump_ok(&path); + } +} + +// ---- 2. Time / BitField / Opaque / Reference datatypes ---- + +fn exotic_types() -> Vec<(&'static str, Datatype, Vec)> { + // Four elements each. The object references point at the root group, + // which a v3-superblock file without an extension puts at address 48. + let refs: Vec = (0..4).flat_map(|_| 48u64.to_le_bytes()).collect(); + vec![ + ( + "bits", + Datatype::BitField { + size: 1, + byte_order: DatatypeByteOrder::LittleEndian, + bit_offset: 0, + bit_precision: 8, + }, + vec![1, 2, 4, 8], + ), + ( + "opaque", + Datatype::Opaque { + size: 4, + tag: b"mytag".to_vec(), + }, + (0..16).collect(), + ), + ( + "ref", + Datatype::Reference { + size: 8, + ref_type: ReferenceType::Object, + }, + refs, + ), + ( + "time", + Datatype::Time { + size: 4, + bit_precision: 32, + }, + (0..16).collect(), + ), + ] +} + +fn exotic_file() -> Vec { + let mut fw = FileWriter::new(); + for (name, dt, raw) in exotic_types() { + fw.create_dataset(name) + .with_compound_data(dt.clone(), raw.clone(), 4); + fw.set_root_attr( + name, + AttrValue::Raw { + datatype: dt, + shape: vec![4], + data: raw, + }, + ); + } + fw.finish().unwrap() +} + +#[test] +fn exotic_datatypes_are_written_not_emptied() { + let bytes = exotic_file(); + let (sb, root) = header_at(&bytes, "/"); + assert_eq!(sb.root_group_address, 48); + let attrs = clawhdf5_format::attribute::extract_attributes(&root, sb.length_size).unwrap(); + for (name, dt, raw) in exotic_types() { + let (_, oh) = header_at(&bytes, name); + let msg = oh + .messages + .iter() + .find(|m| m.msg_type == MessageType::Datatype) + .unwrap(); + assert_eq!(msg.data, dt.serialize(), "{name}"); + assert_eq!(Datatype::parse(&msg.data).unwrap().0, dt, "{name}"); + let attr = attrs.iter().find(|a| a.name == name).unwrap(); + assert_eq!(attr.datatype, dt, "{name}"); + assert_eq!(attr.raw_data, raw, "{name}"); + } +} + +#[test] +#[ignore = "requires Python h5py module and h5dump"] +fn h5py_reads_exotic_datatypes() { + let path = write_tmp("exotic", &exotic_file()); + let out = h5py( + &path, + "from h5py import h5t, h5s\n\ + f = h5py.File(path, 'r')\n\ + r = {}\n\ + buf = np.zeros(4, dtype='V4')\n\ + f['opaque'].id.read(h5s.ALL, h5s.ALL, buf, mtype=f['opaque'].id.get_type())\n\ + r['bits'] = f['bits'][()].tolist(), f.attrs['bits'].tolist()\n\ + r['opaque'] = (f['opaque'].id.get_type().get_tag().decode(),\n\ + \x20 f.attrs.get_id('opaque').get_type().get_tag().decode(),\n\ + \x20 buf.tobytes().hex())\n\ + r['ref'] = [f[x].name for x in f['ref'][()]] + [f[x].name for x in f.attrs['ref']]\n\ + r['time'] = (f['time'].id.get_type().get_class() == h5t.TIME,\n\ + \x20 f.attrs.get_id('time').get_type().get_class() == h5t.TIME)\n\ + print(json.dumps(r))", + ); + let v: serde_json::Value = serde_json::from_str(&out).unwrap(); + assert_eq!(v["bits"], serde_json::json!([[1, 2, 4, 8], [1, 2, 4, 8]])); + assert_eq!( + v["opaque"], + serde_json::json!(["mytag", "mytag", "000102030405060708090a0b0c0d0e0f"]) + ); + assert_eq!(v["ref"], serde_json::json!(vec!["/"; 8])); + assert_eq!(v["time"], serde_json::json!([true, true])); + h5dump_ok(&path); +} + +#[test] +#[ignore = "requires Python h5py module and h5dump"] +fn raw_attributes_copied_from_h5py_survive_a_rewrite() { + // Read Raw attributes of the exotic classes out of an h5py file and write + // them back: this used to emit empty datatype messages. + let src = std::env::temp_dir().join("clawhdf5_writer_meta_exotic_src.h5"); + h5py( + &src, + "from h5py import h5t, h5s, h5a\n\ + f = h5py.File(path, 'w')\n\ + f.attrs['ref'] = np.array([f.ref, f.ref], dtype=h5py.ref_dtype)\n\ + f.attrs.create('opaque', np.frombuffer(b'abcdefgh', dtype='V4'))\n\ + t = h5t.STD_B16BE.copy()\n\ + a = h5a.create(f.id, b'bits', t, h5s.create_simple((2,)))\n\ + a.write(np.array([0x0102, 0x0304], dtype='>u2'), mtype=t)\n\ + a.close()\n\ + f.close()", + ); + let src_bytes = std::fs::read(&src).unwrap(); + let (sb, root) = header_at(&src_bytes, "/"); + let attrs = clawhdf5_format::attribute::extract_attributes(&root, sb.length_size).unwrap(); + assert_eq!(attrs.len(), 3); + let mut fw = FileWriter::new(); + for a in &attrs { + let data = if a.name == "ref" { + // Re-target the references at our root group. + 48u64.to_le_bytes().repeat(2) + } else { + a.raw_data.clone() + }; + fw.set_root_attr( + &a.name, + AttrValue::Raw { + datatype: a.datatype.clone(), + shape: a.dataspace.dimensions.clone(), + data, + }, + ); + } + let path = write_tmp("exotic_copy", &fw.finish().unwrap()); + let out = h5py( + &path, + "f = h5py.File(path, 'r')\n\ + print(json.dumps([[f[x].name for x in f.attrs['ref']],\n\ + \x20 f.attrs['opaque'].tobytes().decode(),\n\ + \x20 f.attrs.get_id('bits').get_type().get_order(),\n\ + \x20 f.attrs['bits'].tolist()]))", + ); + assert_eq!(out, r#"[["/", "/"], "abcdefgh", 1, [258, 772]]"#); + h5dump_ok(&path); +} + +// ---- 3. paged file-space strategy ---- + +fn paged_file(page_size: u32) -> Vec { + let mut fw = FileWriter::new(); + fw.with_page_size(page_size); + fw.create_dataset("d").with_f64_data(&[1.0, 2.0, 3.0]); + fw.create_dataset("c") + .with_i32_data(&(0..100).collect::>()) + .with_chunks(&[10]); + fw.set_root_attr("a", AttrValue::I64(7)); + let mut g = fw.create_group("g"); + g.create_dataset("e").with_u8_data(&[9; 5000]); + fw.add_group(g.finish()); + fw.finish().unwrap() +} + +#[test] +fn paged_file_has_a_real_superblock() { + // Measured: `with_page_size` wrote superblock version 4, which does not + // exist ("bad superblock version number" in libhdf5). + for ps in [512u32, 4096, 65536] { + let bytes = paged_file(ps); + let (sb, _) = header_at(&bytes, "/"); + assert_eq!(sb.version, 3); + assert_eq!(bytes.len() % ps as usize, 0); + let (_, e) = header_at(&bytes, "g/e"); + assert!( + e.messages + .iter() + .any(|m| m.msg_type == MessageType::Dataspace) + ); + } +} + +#[test] +#[ignore = "requires Python h5py module and h5dump"] +fn h5py_opens_paged_files() { + for ps in [512u32, 4096, 65536] { + let path = write_tmp(&format!("paged_{ps}"), &paged_file(ps)); + let out = h5py( + &path, + "f = h5py.File(path, 'r')\n\ + p = f.id.get_create_plist()\n\ + print(json.dumps([p.get_file_space_strategy()[0], p.get_file_space_page_size(),\n\ + \x20 f['d'][()].tolist(), int(f['c'][()].sum()), int(f.attrs['a']),\n\ + \x20 int(f['g/e'][()].sum())]))", + ); + assert_eq!( + out, + format!("[1, {ps}, [1.0, 2.0, 3.0], 4950, 7, 45000]"), + "page size {ps}" + ); + h5dump_ok(&path); + } +} + +// ---- 4. fill time and fill value ---- + +fn fill_message(bytes: &[u8], path: &str) -> clawhdf5_format::object_header::HeaderMessage { + let (_, oh) = header_at(bytes, path); + oh.messages + .into_iter() + .find(|m| m.msg_type == MessageType::FillValue) + .unwrap() +} + +fn fill_file() -> Vec { + let mut fw = FileWriter::new(); + fw.create_dataset("never") + .with_f64_data(&[1.0, 2.0]) + .fill_time(FillTime::Never); + fw.create_dataset("alloc") + .with_f64_data(&[1.0, 2.0]) + .fill_time(FillTime::Alloc); + fw.create_dataset("ifset") + .with_f64_data(&[1.0, 2.0]) + .fill_time(FillTime::IfSet); + fw.create_dataset("default").with_f64_data(&[1.0, 2.0]); + fw.create_dataset("filled") + .with_i32_data(&[1, 2, 3, 4]) + .with_chunks(&[2]) + .with_maxshape(&[u64::MAX]) + .with_fill_value(&(-1i32).to_le_bytes()); + fw.finish().unwrap() +} + +#[test] +fn fill_time_uses_libhdf5_codes() { + // H5D_FILL_TIME_ALLOC = 0, NEVER = 1, IFSET = 2, in bits 2-3. Measured: + // h5py saw our Never as ALLOC, Alloc as IFSET and IfSet as NEVER. + let bytes = fill_file(); + for (path, code) in [("never", 1), ("alloc", 0), ("ifset", 2), ("default", 2)] { + let msg = fill_message(&bytes, path); + assert_eq!((msg.data[1] >> 2) & 3, code, "{path}"); + assert_eq!(msg.data[1] & 3, 2, "{path}: allocation time stays late"); + } + for ft in [FillTime::Never, FillTime::Alloc, FillTime::IfSet] { + assert_eq!(FillTime::from_byte(ft.to_byte()), Some(ft)); + } + assert_eq!(FillTime::default(), FillTime::IfSet); +} + +#[test] +fn fill_value_is_written_and_read_back() { + let bytes = fill_file(); + let msg = fill_message(&bytes, "filled"); + assert_eq!( + clawhdf5_format::fill_value::parse_fill_value(&msg).unwrap(), + Some((-1i32).to_le_bytes().to_vec()) + ); + assert_eq!( + clawhdf5_format::fill_value::parse_fill_value(&fill_message(&bytes, "ifset")).unwrap(), + None + ); + + // One element's bytes, no more, no less. + let mut fw = FileWriter::new(); + fw.create_dataset("d") + .with_f64_data(&[1.0]) + .with_fill_value(&[0; 4]); + assert!(fw.finish().is_err()); +} + +#[test] +#[ignore = "requires Python h5py module and h5dump"] +fn h5py_sees_our_fill_time_and_fill_value() { + let path = write_tmp("fill", &fill_file()); + let out = h5py( + &path, + "from h5py import h5d\n\ + f = h5py.File(path, 'r')\n\ + names = {h5d.FILL_TIME_NEVER: 'never', h5d.FILL_TIME_ALLOC: 'alloc', h5d.FILL_TIME_IFSET: 'ifset'}\n\ + t = [names[f[n].id.get_create_plist().get_fill_time()] for n in ('never', 'alloc', 'ifset', 'default')]\n\ + print(json.dumps([t, int(f['filled'].fillvalue), f['filled'][()].tolist()]))\n\ + f.close()\n\ + f = h5py.File(path, 'r+')\n\ + f['filled'].resize((7,))\n\ + f.close()\n\ + print(json.dumps(h5py.File(path, 'r')['filled'][()].tolist()))", + ); + assert_eq!( + out, + "[[\"never\", \"alloc\", \"ifset\", \"ifset\"], -1, [1, 2, 3, 4]]\n[1, 2, 3, 4, -1, -1, -1]" + ); + h5dump_ok(&path); +} + +// ---- 5. empty string attributes ---- + +fn empty_string_file() -> Vec { + let mut fw = FileWriter::new(); + fw.set_root_attr("empty", AttrValue::String(String::new())); + fw.set_root_attr("x", AttrValue::String("héllo".into())); + fw.set_root_attr( + "empties", + AttrValue::StringArray(vec![String::new(), String::new()]), + ); + fw.set_root_attr("n", AttrValue::I64(3)); + fw.finish().unwrap() +} + +#[test] +fn empty_string_attribute_has_a_one_byte_type() { + // Measured: "" got a size-0 string type, and libhdf5 then refused every + // attribute on the object ("invalid datatype size"). + let bytes = empty_string_file(); + let (sb, root) = header_at(&bytes, "/"); + let attrs = clawhdf5_format::attribute::extract_attributes(&root, sb.length_size).unwrap(); + for name in ["empty", "empties"] { + let a = attrs.iter().find(|a| a.name == name).unwrap(); + assert_eq!(a.datatype.type_size(), 1, "{name}"); + let strings = a.read_as_strings().unwrap(); + assert!(strings.iter().all(String::is_empty), "{name}: {strings:?}"); + } + + // A size-0 string type handed in directly is refused, not written. + let mut fw = FileWriter::new(); + fw.set_root_attr( + "raw", + AttrValue::Raw { + datatype: Datatype::String { + size: 0, + padding: clawhdf5_format::datatype::StringPadding::NullPad, + charset: clawhdf5_format::datatype::CharacterSet::Ascii, + }, + shape: vec![], + data: vec![], + }, + ); + assert!(fw.finish().is_err()); +} + +#[test] +#[ignore = "requires Python h5py module and h5dump"] +fn h5py_reads_all_attributes_next_to_an_empty_string() { + let path = write_tmp("empty_str", &empty_string_file()); + let out = h5py( + &path, + "f = h5py.File(path, 'r')\n\ + d = lambda v: v.decode() if isinstance(v, bytes) else v\n\ + print(json.dumps([d(f.attrs['empty']), d(f.attrs['x']),\n\ + \x20 [d(s) for s in f.attrs['empties']], int(f.attrs['n'])], ensure_ascii=False))", + ); + assert_eq!(out, r#"["", "héllo", ["", ""], 3]"#); + h5dump_ok(&path); +} + +// ---- 6. path-like names ---- + +#[test] +fn slash_in_a_group_or_dataset_name_is_an_error() { + // Measured: create_group("a/b") wrote one link literally named "a/b", + // which h5py cannot reach ("component not found"). The writer has no + // nested groups, so such names are refused. + let mut fw = FileWriter::new(); + let mut g = fw.create_group("a/b"); + g.create_dataset("c").with_f64_data(&[1.0]); + fw.add_group(g.finish()); + assert!(fw.finish().is_err()); + + let mut fw = FileWriter::new(); + fw.create_dataset("x/y").with_f64_data(&[1.0]); + assert!(fw.finish().is_err()); + + let mut fw = FileWriter::new(); + let mut g = fw.create_group("g"); + g.create_dataset("x/y").with_f64_data(&[1.0]); + fw.add_group(g.finish()); + assert!(fw.finish().is_err()); + + for bad in ["", "."] { + let mut fw = FileWriter::new(); + fw.create_dataset(bad).with_f64_data(&[1.0]); + assert!(fw.finish().is_err(), "{bad:?}"); + } + + // One level of groups still works, and '/' stays legal in attribute names. + let mut fw = FileWriter::new(); + let mut g = fw.create_group("g"); + g.create_dataset("c").with_f64_data(&[1.0]); + g.set_attr("m/s", AttrValue::I64(1)); + fw.add_group(g.finish()); + let bytes = fw.finish().unwrap(); + header_at(&bytes, "g/c"); +} + +// ---- 7. unknown-message flags on read ---- + +#[test] +fn unknown_message_flags_follow_libhdf5_on_tbogus() { + // libhdf5's own test file (test/testfiles/tbogus.h5): datasets carrying + // an unknown message with various flags. libhdf5 (read-only) opens + // Dataset1, 2, 4 and 5 and refuses Dataset3 ("unknown message with 'fail + // if unknown' flag found"). We used to refuse Dataset2 (bit 3, which only + // applies when writing) and open Dataset3 (bit 7, fail always). + let bytes = include_bytes!("fixtures/tbogus.h5"); + let sig = signature::find_signature(bytes).unwrap(); + let sb = Superblock::parse(bytes, sig).unwrap(); + for (name, readable) in [ + ("Dataset1", true), + ("Dataset2", true), + ("Dataset3", false), + ("Dataset4", true), + ("Dataset5", true), + ] { + let addr = resolve_path_any(bytes, &sb, name).unwrap(); + let parsed = ObjectHeader::parse(bytes, addr as usize, sb.offset_size, sb.length_size); + match parsed { + Ok(_) => assert!(readable, "{name} must be refused"), + Err(e) => { + assert!(!readable, "{name} must be readable, got {e:?}"); + assert!(matches!( + e, + clawhdf5_format::error::FormatError::UnsupportedMessage(_) + )); + } + } + } +} + +// ---- 8. shared fill value messages ---- + +#[test] +fn shared_fill_value_is_resolved_not_zero() { + // gen_shared_fill.py: HDF5 2.0 with a SOHM index for fill values, so each + // dataset's fill value message is a reference into the SOHM heap. It + // used to be read as "no fill value" (zeros) instead of -7. + let bytes = include_bytes!("fixtures/shared_fill_value.h5"); + for (name, shared) in [ + ("sohm_a", false), + ("sohm_b", true), + ("unwritten_a", false), + ("unwritten_b", true), + ] { + let (sb, oh) = header_at(bytes, name); + let msg = oh + .messages + .iter() + .find(|m| m.msg_type == MessageType::FillValue) + .unwrap(); + assert_eq!( + clawhdf5_format::shared_message::is_shared(msg.flags), + shared, + "{name}: fixture layout" + ); + if shared { + // Without the file the reference cannot be followed: an error, + // never a silent default. + assert_eq!( + clawhdf5_format::fill_value::dataset_fill_value(&oh.messages), + Err(clawhdf5_format::error::FormatError::UnresolvedSharedMessage) + ); + } + assert_eq!( + clawhdf5_format::fill_value::dataset_fill_value_in( + bytes, + &oh.messages, + sb.offset_size, + sb.length_size + ) + .unwrap(), + Some((-7i32).to_le_bytes().to_vec()), + "{name}" + ); + } +} diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index 64f96ef..bb6a8fd 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -478,7 +478,12 @@ impl<'f> Dataset<'f> { // sparse) dataset — select from a fill-aware full read instead. (The // selection reader currently decodes the full dataset too, so this // costs nothing extra.) - let fill = clawhdf5_format::fill_value::dataset_fill_value(&self.header.messages)?; + let fill = clawhdf5_format::fill_value::dataset_fill_value_in( + self.file.data.as_bytes(), + &self.header.messages, + self.file.offset_size(), + self.file.length_size(), + )?; let fill_matters = !clawhdf5_format::fill_value::has_storage(&dl) || (matches!(dl, DataLayout::Chunked { .. }) && !clawhdf5_format::fill_value::is_default(fill.as_deref())); diff --git a/crates/clawhdf5/tests/shared_fill_value.rs b/crates/clawhdf5/tests/shared_fill_value.rs new file mode 100644 index 0000000..9f1c50b --- /dev/null +++ b/crates/clawhdf5/tests/shared_fill_value.rs @@ -0,0 +1,49 @@ +//! Datasets whose Fill Value message is shared through the file's SOHM heap +//! (fixture written by HDF5 2.0, see `gen_shared_fill.py`). Their unwritten +//! storage must read as the fill value (-7), not as zeros. + +use clawhdf5::File; +use clawhdf5_format::selection::Selection; + +const FIXTURE: &[u8] = include_bytes!("../../clawhdf5-format/tests/fixtures/shared_fill_value.h5"); + +#[test] +fn shared_fill_value_applies_to_unwritten_storage() { + let file = File::from_bytes(FIXTURE.to_vec()).unwrap(); + // `_a` keeps its fill value in its own header, `_b` references the SOHM + // heap; both must read the same. + for name in ["sohm_a", "sohm_b"] { + assert_eq!( + file.dataset(name).unwrap().read_i32().unwrap(), + [0, 1, 2, 3, -7, -7, -7, -7], + "{name}" + ); + } + for name in ["unwritten_a", "unwritten_b"] { + assert_eq!( + file.dataset(name).unwrap().read_i32().unwrap(), + [-7, -7, -7], + "{name}" + ); + } + + // The selection path decides on its own whether the fill value matters. + let slab = Selection::Hyperslab { + start: vec![2], + stride: vec![1], + count: vec![4], + block: vec![1], + }; + let raw = file + .dataset("sohm_b") + .unwrap() + .read_selection(&slab) + .unwrap(); + let values: Vec = raw + .as_chunks::<4>() + .0 + .iter() + .map(|b| i32::from_le_bytes(*b)) + .collect(); + assert_eq!(values, [2, 3, -7, -7]); +}