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/tests/fixtures/tbogus.h5 b/crates/clawhdf5-format/tests/fixtures/tbogus.h5 new file mode 100644 index 0000000..f64229e Binary files /dev/null and b/crates/clawhdf5-format/tests/fixtures/tbogus.h5 differ diff --git a/crates/clawhdf5-format/tests/writer_meta_tests.rs b/crates/clawhdf5-format/tests/writer_meta_tests.rs index 587f65f..88a08a1 100644 --- a/crates/clawhdf5-format/tests/writer_meta_tests.rs +++ b/crates/clawhdf5-format/tests/writer_meta_tests.rs @@ -563,3 +563,37 @@ fn slash_in_a_group_or_dataset_name_is_an_error() { 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(_) + )); + } + } + } +}