diff --git a/crates/clawhdf5-format/src/datatype.rs b/crates/clawhdf5-format/src/datatype.rs index 4a1ba26..90c9657 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,74 @@ 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::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 +1706,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 d5260c2..b8d0edb 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -1125,6 +1125,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() diff --git a/crates/clawhdf5-format/tests/writer_meta_tests.rs b/crates/clawhdf5-format/tests/writer_meta_tests.rs index 1175ab4..22db1c5 100644 --- a/crates/clawhdf5-format/tests/writer_meta_tests.rs +++ b/crates/clawhdf5-format/tests/writer_meta_tests.rs @@ -6,6 +6,7 @@ //! (`CLAWHDF5_PYTHON`, as in `writer_h5py_tests.rs`) and run `h5dump` over it. use clawhdf5_format::data_layout::DataLayout; +use clawhdf5_format::datatype::{Datatype, DatatypeByteOrder, ReferenceType}; use clawhdf5_format::file_writer::{AttrValue, FileWriter}; use clawhdf5_format::group_v2::resolve_path_any; use clawhdf5_format::message_type::MessageType; @@ -151,3 +152,168 @@ fn h5py_reads_compact_datasets_at_the_limit() { 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); +}