fix(format): honour unknown-message flags the way libhdf5 does
The object header parser failed on an unknown message with flag bit 3 set and ignored bit 7. Per the spec, bit 3 means "fail if unknown and the file is opened for writing" and bit 7 "fail if unknown, always". The parser only reads, so it now ignores bit 3 (as libhdf5 does for a read-only open) and refuses bit 7, in v1 headers, v2 headers and their continuation chunks. On libhdf5's conformance file tbogus.h5 (added as a fixture) we used to refuse Dataset2 and open Dataset3; we now match libhdf5: Dataset1, 2, 4 and 5 open, Dataset3 is refused. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -146,12 +146,7 @@ impl ObjectHeader {
|
|||||||
ensure_len(data, pos, msg_data_size)?;
|
ensure_len(data, pos, msg_data_size)?;
|
||||||
let msg_type = MessageType::from_u16(msg_type_raw);
|
let msg_type = MessageType::from_u16(msg_type_raw);
|
||||||
|
|
||||||
// Check if unknown + must-understand (bit 3 of msg_flags)
|
check_unknown_message(msg_type, msg_flags)?;
|
||||||
if let MessageType::Unknown(id) = msg_type
|
|
||||||
&& msg_flags & 0x08 != 0
|
|
||||||
{
|
|
||||||
return Err(FormatError::UnsupportedMessage(id));
|
|
||||||
}
|
|
||||||
|
|
||||||
if msg_type != MessageType::Nil {
|
if msg_type != MessageType::Nil {
|
||||||
messages.push(HeaderMessage {
|
messages.push(HeaderMessage {
|
||||||
@@ -229,11 +224,7 @@ impl ObjectHeader {
|
|||||||
|
|
||||||
let msg_type = MessageType::from_u16(msg_type_raw);
|
let msg_type = MessageType::from_u16(msg_type_raw);
|
||||||
|
|
||||||
if let MessageType::Unknown(id) = msg_type
|
check_unknown_message(msg_type, msg_flags)?;
|
||||||
&& msg_flags & 0x08 != 0
|
|
||||||
{
|
|
||||||
return Err(FormatError::UnsupportedMessage(id));
|
|
||||||
}
|
|
||||||
|
|
||||||
if msg_type != MessageType::Nil {
|
if msg_type != MessageType::Nil {
|
||||||
messages.push(HeaderMessage {
|
messages.push(HeaderMessage {
|
||||||
@@ -424,11 +415,7 @@ impl ObjectHeader {
|
|||||||
|
|
||||||
let msg_type = MessageType::from_u16(msg_type_raw);
|
let msg_type = MessageType::from_u16(msg_type_raw);
|
||||||
|
|
||||||
if let MessageType::Unknown(id) = msg_type
|
check_unknown_message(msg_type, msg_flags)?;
|
||||||
&& msg_flags & 0x08 != 0
|
|
||||||
{
|
|
||||||
return Err(FormatError::UnsupportedMessage(id));
|
|
||||||
}
|
|
||||||
|
|
||||||
let msg_data = data[pos..pos + msg_data_size].to_vec();
|
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)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
@@ -632,14 +637,38 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn parse_v1_unknown_must_understand_errors() {
|
fn parse_v1_unknown_fail_always_errors() {
|
||||||
// Bit 3 of msg_flags = must understand
|
// Bit 7 of msg_flags = fail if unknown, whatever the access mode.
|
||||||
let messages = [(0x00FFu16, &[0xAA][..], 0x08u8)];
|
let messages = [(0x00FFu16, &[0xAA][..], 0x80u8)];
|
||||||
let data = build_v1_header(&messages, 8, 8);
|
let data = build_v1_header(&messages, 8, 8);
|
||||||
let err = ObjectHeader::parse(&data, 0, 8, 8).unwrap_err();
|
let err = ObjectHeader::parse(&data, 0, 8, 8).unwrap_err();
|
||||||
assert_eq!(err, FormatError::UnsupportedMessage(0x00FF));
|
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]
|
#[test]
|
||||||
fn parse_v2_no_timestamps_one_message() {
|
fn parse_v2_no_timestamps_one_message() {
|
||||||
let data = build_v2_header(0x00, &[(0x01, &[10, 20], 0)], None);
|
let data = build_v2_header(0x00, &[(0x01, &[10, 20], 0)], None);
|
||||||
|
|||||||
BIN
Binary file not shown.
@@ -563,3 +563,37 @@ fn slash_in_a_group_or_dataset_name_is_an_error() {
|
|||||||
let bytes = fw.finish().unwrap();
|
let bytes = fw.finish().unwrap();
|
||||||
header_at(&bytes, "g/c");
|
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(_)
|
||||||
|
));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user