diff --git a/crates/clawhdf5-format/src/attribute.rs b/crates/clawhdf5-format/src/attribute.rs index 2c8eaca..bb96bd5 100644 --- a/crates/clawhdf5-format/src/attribute.rs +++ b/crates/clawhdf5-format/src/attribute.rs @@ -1,7 +1,9 @@ //! HDF5 Attribute message parsing (message type 0x000C). #[cfg(not(feature = "std"))] -use alloc::{string::String, vec::Vec}; +use alloc::{borrow::Cow, string::String, vec::Vec}; +#[cfg(feature = "std")] +use std::borrow::Cow; use crate::attribute_info::AttributeInfoMessage; use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records}; @@ -48,17 +50,64 @@ impl AttributeMessage { /// /// `length_size` is needed for dataspace dimension parsing. pub fn parse(data: &[u8], length_size: u8) -> Result { + Self::parse_impl(data, length_size, None) + } + + /// [`AttributeMessage::parse`] with access to the rest of the file, which + /// is needed when the attribute's datatype or dataspace is *shared* (v2/v3 + /// flag bits 0/1) — e.g. an attribute created with a committed datatype. + /// In that case the embedded bytes are a reference to the real message, + /// not the message. Without file access such an attribute is an error + /// rather than a garbage datatype. + pub fn parse_in_file( + data: &[u8], + file_data: &[u8], + offset_size: u8, + length_size: u8, + ) -> Result { + Self::parse_impl(data, length_size, Some((file_data, offset_size))) + } + + fn parse_impl( + data: &[u8], + length_size: u8, + file: Option<(&[u8], u8)>, + ) -> Result { ensure_len(data, 0, 2)?; let version = data[0]; match version { 1 => Self::parse_v1(data, length_size), - 2 => Self::parse_v2(data, length_size), - 3 => Self::parse_v3(data, length_size), + 2 => Self::parse_v2(data, length_size, file), + 3 => Self::parse_v3(data, length_size, file), _ => Err(FormatError::InvalidAttributeVersion(version)), } } + /// The bytes of an embedded datatype/dataspace message, following the + /// shared-message reference when `shared` is set. + fn embedded_message<'a>( + bytes: &'a [u8], + shared: bool, + msg_type: MessageType, + length_size: u8, + file: Option<(&[u8], u8)>, + ) -> Result, FormatError> { + if !shared { + return Ok(Cow::Borrowed(bytes)); + } + let (file_data, offset_size) = file.ok_or(FormatError::UnresolvedSharedMessage)?; + let shared_ref = shared_message::parse_shared_ref(bytes, offset_size)?; + shared_message::resolve_shared_message( + file_data, + &shared_ref, + msg_type, + offset_size, + length_size, + ) + .map(Cow::Owned) + } + fn parse_v1(data: &[u8], length_size: u8) -> Result { // version(1) + reserved(1) + name_size(2) + datatype_size(2) + dataspace_size(2) = 8 ensure_len(data, 0, 8)?; @@ -94,7 +143,13 @@ impl AttributeMessage { }) } - fn parse_v2(data: &[u8], length_size: u8) -> Result { + fn parse_v2( + data: &[u8], + length_size: u8, + file: Option<(&[u8], u8)>, + ) -> Result { + // Flags: bit 0 = datatype is shared, bit 1 = dataspace is shared. + let flags = data.get(1).copied().unwrap_or(0); // version(1) + flags(1) + name_size(2) + datatype_size(2) + dataspace_size(2) = 8 ensure_len(data, 0, 8)?; let name_size = u16::from_le_bytes([data[2], data[3]]) as usize; @@ -110,12 +165,26 @@ impl AttributeMessage { // Datatype (NO padding) ensure_len(data, pos, datatype_size)?; - let (datatype, _) = Datatype::parse(&data[pos..pos + datatype_size])?; + let dt_bytes = Self::embedded_message( + &data[pos..pos + datatype_size], + flags & 0x01 != 0, + MessageType::Datatype, + length_size, + file, + )?; + let (datatype, _) = Datatype::parse(&dt_bytes)?; pos += datatype_size; // Dataspace (NO padding) ensure_len(data, pos, dataspace_size)?; - let dataspace = Dataspace::parse(&data[pos..pos + dataspace_size], length_size)?; + let ds_bytes = Self::embedded_message( + &data[pos..pos + dataspace_size], + flags & 0x02 != 0, + MessageType::Dataspace, + length_size, + file, + )?; + let dataspace = Dataspace::parse(&ds_bytes, length_size)?; pos += dataspace_size; let raw_data = compute_raw_data(data, pos, &dataspace, &datatype); @@ -128,7 +197,13 @@ impl AttributeMessage { }) } - fn parse_v3(data: &[u8], length_size: u8) -> Result { + fn parse_v3( + data: &[u8], + length_size: u8, + file: Option<(&[u8], u8)>, + ) -> Result { + // Flags: bit 0 = datatype is shared, bit 1 = dataspace is shared. + let flags = data.get(1).copied().unwrap_or(0); // version(1) + flags(1) + name_size(2) + datatype_size(2) + dataspace_size(2) + encoding(1) = 9 ensure_len(data, 0, 9)?; let name_size = u16::from_le_bytes([data[2], data[3]]) as usize; @@ -145,12 +220,26 @@ impl AttributeMessage { // Datatype (NO padding) ensure_len(data, pos, datatype_size)?; - let (datatype, _) = Datatype::parse(&data[pos..pos + datatype_size])?; + let dt_bytes = Self::embedded_message( + &data[pos..pos + datatype_size], + flags & 0x01 != 0, + MessageType::Datatype, + length_size, + file, + )?; + let (datatype, _) = Datatype::parse(&dt_bytes)?; pos += datatype_size; // Dataspace (NO padding) ensure_len(data, pos, dataspace_size)?; - let dataspace = Dataspace::parse(&data[pos..pos + dataspace_size], length_size)?; + let ds_bytes = Self::embedded_message( + &data[pos..pos + dataspace_size], + flags & 0x02 != 0, + MessageType::Dataspace, + length_size, + file, + )?; + let dataspace = Dataspace::parse(&ds_bytes, length_size)?; pos += dataspace_size; let raw_data = compute_raw_data(data, pos, &dataspace, &datatype); @@ -326,10 +415,20 @@ pub fn extract_attributes_full( offset_size, length_size, )?; - let attr = AttributeMessage::parse(&resolved_data, length_size)?; + let attr = AttributeMessage::parse_in_file( + &resolved_data, + file_data, + offset_size, + length_size, + )?; attrs.push(attr); } else { - let attr = AttributeMessage::parse(&msg.data, length_size)?; + let attr = AttributeMessage::parse_in_file( + &msg.data, + file_data, + offset_size, + length_size, + )?; attrs.push(attr); } } @@ -399,7 +498,8 @@ fn extract_dense_attributes( let attr_data = fh.read_managed_object(file_data, id_bytes, offset_size)?; // The data in the heap is a complete attribute message - let attr = AttributeMessage::parse(&attr_data, length_size)?; + let attr = + AttributeMessage::parse_in_file(&attr_data, file_data, offset_size, length_size)?; attrs.push(attr); } diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index 183eb12..43a1cae 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -114,6 +114,9 @@ pub enum FormatError { InvalidAttributeInfoVersion(u8), /// Invalid shared message version. InvalidSharedMessageVersion(u8), + /// A message is marked shared but was parsed without access to the file, + /// so the reference to the real message could not be followed. + UnresolvedSharedMessage, /// Invalid SOHM table version. InvalidSohmTableVersion(u8), /// Invalid SOHM table signature (expected "SMTB"). @@ -307,6 +310,10 @@ impl fmt::Display for FormatError { FormatError::InvalidSharedMessageVersion(v) => { write!(f, "invalid shared message version: {v}") } + FormatError::UnresolvedSharedMessage => write!( + f, + "message is shared but no file data was available to resolve it" + ), FormatError::InvalidSohmTableVersion(v) => { write!(f, "invalid SOHM table version: {v}") } diff --git a/crates/clawhdf5-format/src/shared_message.rs b/crates/clawhdf5-format/src/shared_message.rs index 6ba121d..954618d 100644 --- a/crates/clawhdf5-format/src/shared_message.rs +++ b/crates/clawhdf5-format/src/shared_message.rs @@ -16,8 +16,12 @@ //! - SMLI list structure: simple list of shared message entries //! - B-tree v2 type 7: indexed shared message entries +#[cfg(not(feature = "std"))] +use alloc::borrow::Cow; #[cfg(not(feature = "std"))] use alloc::vec::Vec; +#[cfg(feature = "std")] +use std::borrow::Cow; use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records}; use crate::error::FormatError; @@ -28,6 +32,14 @@ use crate::object_header::ObjectHeader; /// Fractal heap ID length for SOHM entries (fixed at 8 bytes). const FHEAP_ID_LEN: usize = 8; +/// Shared-message `type` values (version 3 encoding). +/// The message is in the file's shared-message (SOHM) fractal heap. +const SHARE_TYPE_SOHM: u8 = 1; +/// The message is in another object's header (a committed/named datatype). +const SHARE_TYPE_COMMITTED: u8 = 2; +/// The message is stored here but is sharable. +const SHARE_TYPE_HERE: u8 = 3; + /// A resolved shared message reference. #[derive(Debug, Clone)] pub struct SharedMessageRef { @@ -35,9 +47,10 @@ pub struct SharedMessageRef { pub ref_type: u8, /// Version of the shared message encoding. pub version: u8, - /// Address of the object header containing the shared message (type 1, 3). + /// Address of the object header holding the message (committed). Set for + /// every v1/v2 reference and for v3 types 2 and 3. pub object_header_address: Option, - /// Fractal heap ID for type 2 (SOHM) references. + /// Fractal heap ID for a v3 SOHM (type 1) reference. pub heap_id: Option<[u8; FHEAP_ID_LEN]>, } @@ -146,48 +159,39 @@ pub fn parse_shared_ref(data: &[u8], offset_size: u8) -> Result` + // for a dataset using a committed datatype under both default and + // `latest` libver bounds. + let address_at = |pos: usize| -> Result { + ensure_len(data, pos, offset_size as usize)?; + Ok(SharedMessageRef { + ref_type, + version, + object_header_address: Some(read_offset(data, pos, offset_size)?), + heap_id: None, + }) + }; match version { - 1 | 2 => { - // v1/v2: reserved(6) + address(offset_size) - let pos = 2 + 6; // skip reserved bytes - ensure_len(data, pos, offset_size as usize)?; - let addr = read_offset(data, pos, offset_size)?; + 1 => address_at(2 + 6), + 2 => address_at(2), + 3 if ref_type == SHARE_TYPE_SOHM => { + ensure_len(data, 2, FHEAP_ID_LEN)?; + let mut id = [0u8; FHEAP_ID_LEN]; + id.copy_from_slice(&data[2..2 + FHEAP_ID_LEN]); Ok(SharedMessageRef { ref_type, version, - object_header_address: Some(addr), - heap_id: None, + object_header_address: None, + heap_id: Some(id), }) } - 3 => { - match ref_type { - 1 | 3 => { - // type 1/3: message in another object header - // v3 layout: version(1) + type(1) + address(offset_size) - ensure_len(data, 2, offset_size as usize)?; - let addr = read_offset(data, 2, offset_size)?; - Ok(SharedMessageRef { - ref_type, - version, - object_header_address: Some(addr), - heap_id: None, - }) - } - 2 => { - // type 2: SOHM table (fractal heap ID) - ensure_len(data, 2, FHEAP_ID_LEN)?; - let mut id = [0u8; FHEAP_ID_LEN]; - id.copy_from_slice(&data[2..2 + FHEAP_ID_LEN]); - Ok(SharedMessageRef { - ref_type, - version, - object_header_address: None, - heap_id: Some(id), - }) - } - _ => Err(FormatError::InvalidSharedMessageVersion(ref_type)), - } - } + 3 if ref_type == SHARE_TYPE_COMMITTED || ref_type == SHARE_TYPE_HERE => address_at(2), + 3 => Err(FormatError::InvalidSharedMessageVersion(ref_type)), _ => Err(FormatError::InvalidSharedMessageVersion(version)), } } @@ -422,6 +426,35 @@ pub fn resolve_sohm_message( fh_header.read_managed_object(file_data, heap_id, offset_size) } +/// The payload of an object-header message, following the indirection if the +/// message is *shared* (header flag bit 1). +/// +/// A shared message's bytes are not the message itself but a reference to +/// where it lives — e.g. a dataset created with a committed (named) datatype +/// stores only a pointer to that datatype's object header. Every reader of a +/// message that may be shared (datatype, dataspace, fill value, filter +/// pipeline, attribute) must go through this; parsing the reference bytes as +/// the message yields garbage rather than an error. +pub fn message_data<'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)?; + resolve_shared_message( + file_data, + &shared_ref, + msg.msg_type, + offset_size, + length_size, + ) + .map(Cow::Owned) +} + /// Resolve a shared message to its actual message data. /// /// For type 1/3 (shared in another object header), reads the target object header @@ -453,14 +486,14 @@ pub fn resolve_shared_message_with_sohm( length_size: u8, sohm_table: Option<&SohmTable>, ) -> Result, FormatError> { - match shared_ref.ref_type { - 1 | 3 => { - let addr = shared_ref - .object_header_address - .ok_or(FormatError::UnexpectedEof { - expected: 1, - available: 0, - })?; + // Dispatch on what the reference carries rather than on `ref_type`: v1/v2 + // references are always an object-header address whatever their type + // byte says. + match ( + shared_ref.object_header_address, + shared_ref.heap_id.as_ref(), + ) { + (Some(addr), _) => { let target_header = ObjectHeader::parse(file_data, addr as usize, offset_size, length_size)?; for msg in &target_header.messages { @@ -487,11 +520,7 @@ pub fn resolve_shared_message_with_sohm( available: 0, }) } - 2 => { - let heap_id = shared_ref - .heap_id - .as_ref() - .ok_or(FormatError::InvalidSharedMessageVersion(2))?; + (None, Some(heap_id)) => { let table = sohm_table.ok_or(FormatError::InvalidSharedMessageVersion(2))?; resolve_sohm_message( file_data, @@ -502,7 +531,7 @@ pub fn resolve_shared_message_with_sohm( length_size, ) } - _ => Err(FormatError::InvalidSharedMessageVersion( + (None, None) => Err(FormatError::InvalidSharedMessageVersion( shared_ref.ref_type, )), } @@ -522,15 +551,15 @@ mod tests { } #[test] - fn parse_v3_type1_ref() { + fn parse_v3_committed_ref() { let mut data = Vec::new(); data.push(3); // version - data.push(1); // type 1 = shared in another OH + data.push(SHARE_TYPE_COMMITTED); // message lives in another object header data.extend_from_slice(&0x1234u64.to_le_bytes()); // address let shared = parse_shared_ref(&data, 8).unwrap(); assert_eq!(shared.version, 3); - assert_eq!(shared.ref_type, 1); + assert_eq!(shared.ref_type, SHARE_TYPE_COMMITTED); assert_eq!(shared.object_header_address, Some(0x1234)); assert!(shared.heap_id.is_none()); } @@ -539,7 +568,7 @@ mod tests { fn parse_v3_type3_ref() { let mut data = Vec::new(); data.push(3); // version - data.push(3); // type 3 = shared in another OH (v3 encoding) + data.push(SHARE_TYPE_HERE); // stored here but sharable: an address data.extend_from_slice(&0xABCDu64.to_le_bytes()); let shared = parse_shared_ref(&data, 8).unwrap(); @@ -563,10 +592,10 @@ mod tests { #[test] fn parse_v2_ref() { + // v2 dropped v1's six reserved bytes: the address follows the type. let mut data = Vec::new(); data.push(2); // version - data.push(0); // type - data.extend_from_slice(&[0u8; 6]); // reserved + data.push(SHARE_TYPE_COMMITTED); data.extend_from_slice(&0x9000u32.to_le_bytes()); let shared = parse_shared_ref(&data, 4).unwrap(); @@ -575,15 +604,26 @@ mod tests { } #[test] - fn parse_v3_type2_sohm() { + fn parse_v2_ref_from_hdf5_2_0() { + // Datatype message of a dataset created with a committed datatype, + // as written by h5py 3.16 / HDF5 2.0 (libver='latest'): header flags + // 0x03 (shared), payload `02 02 <8-byte object header address>`. + let data = [0x02, 0x02, 0xb3, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; + let shared = parse_shared_ref(&data, 8).unwrap(); + assert_eq!(shared.object_header_address, Some(0xb3)); + assert!(shared.heap_id.is_none()); + } + + #[test] + fn parse_v3_sohm_ref() { let mut data = Vec::new(); data.push(3); // version - data.push(2); // type 2 = SOHM heap + data.push(SHARE_TYPE_SOHM); // message lives in the SOHM fractal heap data.extend_from_slice(&[0xAA, 0xBB, 0xCC, 0xDD, 0x11, 0x22, 0x33, 0x44]); let shared = parse_shared_ref(&data, 8).unwrap(); assert_eq!(shared.version, 3); - assert_eq!(shared.ref_type, 2); + assert_eq!(shared.ref_type, SHARE_TYPE_SOHM); assert_eq!(shared.object_header_address, None); assert_eq!( shared.heap_id, @@ -592,10 +632,10 @@ mod tests { } #[test] - fn parse_v3_type2_too_short() { + fn parse_v3_sohm_too_short() { let mut data = Vec::new(); data.push(3); // version - data.push(2); // type 2 = SOHM heap + data.push(SHARE_TYPE_SOHM); data.extend_from_slice(&[0xAA, 0xBB]); // only 2 bytes, need 8 let err = parse_shared_ref(&data, 8).unwrap_err(); @@ -620,7 +660,7 @@ mod tests { fn parse_four_byte_offsets() { let mut data = Vec::new(); data.push(3); // version - data.push(1); // type 1 + data.push(SHARE_TYPE_COMMITTED); data.extend_from_slice(&0x1000u32.to_le_bytes()); let shared = parse_shared_ref(&data, 4).unwrap(); diff --git a/crates/clawhdf5/src/lazy.rs b/crates/clawhdf5/src/lazy.rs index 192a05c..4484a4c 100644 --- a/crates/clawhdf5/src/lazy.rs +++ b/crates/clawhdf5/src/lazy.rs @@ -416,15 +416,43 @@ impl<'f, R: HDF5Read> LazyDataset<'f, R> { )) } + /// A header message's payload, resolved through the shared-message + /// indirection when needed (e.g. a committed datatype). See + /// [`clawhdf5_format::shared_message::message_data`]. + fn message_payload( + &self, + msg_type: MessageType, + ) -> Result>, Error> { + self.header + .messages + .iter() + .find(|m| m.msg_type == msg_type) + .map(|msg| { + clawhdf5_format::shared_message::message_data( + self.file.as_bytes(), + msg, + self.file.offset_size(), + self.file.length_size(), + ) + .map_err(Error::Format) + }) + .transpose() + } + + fn required_payload(&self, msg_type: MessageType) -> Result, Error> { + self.message_payload(msg_type)? + .ok_or(Error::MissingMessage(msg_type)) + } + fn datatype(&self) -> Result { - let msg = find_message(&self.header, MessageType::Datatype)?; - let (dt, _) = Datatype::parse(&msg.data)?; + let data = self.required_payload(MessageType::Datatype)?; + let (dt, _) = Datatype::parse(&data)?; Ok(dt) } fn dataspace(&self) -> Result { - let msg = find_message(&self.header, MessageType::Dataspace)?; - Ok(Dataspace::parse(&msg.data, self.file.length_size())?) + let data = self.required_payload(MessageType::Dataspace)?; + Ok(Dataspace::parse(&data, self.file.length_size())?) } fn data_layout(&self) -> Result { @@ -441,11 +469,8 @@ impl<'f, R: HDF5Read> LazyDataset<'f, R> { /// filters" would hand the caller the still-compressed bytes as if they /// were the data. fn filter_pipeline(&self) -> Result, Error> { - self.header - .messages - .iter() - .find(|m| m.msg_type == MessageType::FilterPipeline) - .map(|msg| FilterPipeline::parse(&msg.data).map_err(Error::Format)) + self.message_payload(MessageType::FilterPipeline)? + .map(|data| FilterPipeline::parse(&data).map_err(Error::Format)) .transpose() } diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index 1573780..c1886a4 100644 --- a/crates/clawhdf5/src/mmap_file.rs +++ b/crates/clawhdf5/src/mmap_file.rs @@ -357,15 +357,43 @@ impl<'f> MmapDataset<'f> { )) } + /// A header message's payload, resolved through the shared-message + /// indirection when needed (e.g. a committed datatype). See + /// [`clawhdf5_format::shared_message::message_data`]. + fn message_payload( + &self, + msg_type: MessageType, + ) -> Result>, Error> { + self.header + .messages + .iter() + .find(|m| m.msg_type == msg_type) + .map(|msg| { + clawhdf5_format::shared_message::message_data( + self.file.as_bytes(), + msg, + self.file.offset_size(), + self.file.length_size(), + ) + .map_err(Error::Format) + }) + .transpose() + } + + fn required_payload(&self, msg_type: MessageType) -> Result, Error> { + self.message_payload(msg_type)? + .ok_or(Error::MissingMessage(msg_type)) + } + fn datatype(&self) -> Result { - let msg = find_message(&self.header, MessageType::Datatype)?; - let (dt, _) = Datatype::parse(&msg.data)?; + let data = self.required_payload(MessageType::Datatype)?; + let (dt, _) = Datatype::parse(&data)?; Ok(dt) } fn dataspace(&self) -> Result { - let msg = find_message(&self.header, MessageType::Dataspace)?; - Ok(Dataspace::parse(&msg.data, self.file.length_size())?) + let data = self.required_payload(MessageType::Dataspace)?; + Ok(Dataspace::parse(&data, self.file.length_size())?) } fn data_layout(&self) -> Result { @@ -382,11 +410,8 @@ impl<'f> MmapDataset<'f> { /// filters" would hand the caller the still-compressed bytes as if they /// were the data. fn filter_pipeline(&self) -> Result, Error> { - self.header - .messages - .iter() - .find(|m| m.msg_type == MessageType::FilterPipeline) - .map(|msg| FilterPipeline::parse(&msg.data).map_err(Error::Format)) + self.message_payload(MessageType::FilterPipeline)? + .map(|data| FilterPipeline::parse(&data).map_err(Error::Format)) .transpose() } diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index fee9594..4bbe0cd 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -723,15 +723,43 @@ impl<'f> Dataset<'f> { )?) } + /// A header message's payload, resolved through the shared-message + /// indirection when needed (e.g. a committed datatype). See + /// [`clawhdf5_format::shared_message::message_data`]. + fn message_payload( + &self, + msg_type: MessageType, + ) -> Result>, Error> { + self.header + .messages + .iter() + .find(|m| m.msg_type == msg_type) + .map(|msg| { + clawhdf5_format::shared_message::message_data( + self.file.as_bytes(), + msg, + self.file.offset_size(), + self.file.length_size(), + ) + .map_err(Error::Format) + }) + .transpose() + } + + fn required_payload(&self, msg_type: MessageType) -> Result, Error> { + self.message_payload(msg_type)? + .ok_or(Error::MissingMessage(msg_type)) + } + fn datatype(&self) -> Result { - let msg = find_message(&self.header, MessageType::Datatype)?; - let (dt, _) = Datatype::parse(&msg.data)?; + let data = self.required_payload(MessageType::Datatype)?; + let (dt, _) = Datatype::parse(&data)?; Ok(dt) } fn dataspace(&self) -> Result { - let msg = find_message(&self.header, MessageType::Dataspace)?; - Ok(Dataspace::parse(&msg.data, self.file.length_size())?) + let data = self.required_payload(MessageType::Dataspace)?; + Ok(Dataspace::parse(&data, self.file.length_size())?) } fn data_layout(&self) -> Result { @@ -748,11 +776,8 @@ impl<'f> Dataset<'f> { /// filters" would hand the caller the still-compressed bytes as if they /// were the data. fn filter_pipeline(&self) -> Result, Error> { - self.header - .messages - .iter() - .find(|m| m.msg_type == MessageType::FilterPipeline) - .map(|msg| FilterPipeline::parse(&msg.data).map_err(Error::Format)) + self.message_payload(MessageType::FilterPipeline)? + .map(|data| FilterPipeline::parse(&data).map_err(Error::Format)) .transpose() } diff --git a/crates/clawhdf5/tests/h5py_interop_tests.rs b/crates/clawhdf5/tests/h5py_interop_tests.rs index dc6121a..a7154fd 100644 --- a/crates/clawhdf5/tests/h5py_interop_tests.rs +++ b/crates/clawhdf5/tests/h5py_interop_tests.rs @@ -565,3 +565,52 @@ print("OK") ); assert_eq!(run_python_output(&script), "OK"); } + +// --------------------------------------------------------------------------- +// h5py uses committed (named) datatypes -> clawhdf5 reads +// --------------------------------------------------------------------------- + +/// A dataset or attribute created from a committed datatype stores only a +/// *shared message* reference to it. These used to be parsed as the datatype +/// itself (yielding `Time { size: 0 }` and unreadable data) and the attribute +/// was silently dropped. +#[test] +fn h5py_committed_datatypes_clawhdf5_reads() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + for (tag, kwargs) in [("default", ""), ("latest", ", libver='latest'")] { + let path = dir.path().join(format!("committed_{tag}.h5")); + let path_str = path.display().to_string(); + let script = format!( + r#" +import h5py, numpy as np +with h5py.File("{path_str}", "w"{kwargs}) as f: + f["f8type"] = np.dtype("