diff --git a/crates/clawhdf5-agent/src/schema.rs b/crates/clawhdf5-agent/src/schema.rs index 432b2d8..9f33a0a 100644 --- a/crates/clawhdf5-agent/src/schema.rs +++ b/crates/clawhdf5-agent/src/schema.rs @@ -477,10 +477,34 @@ fn write_string_dataset( } } +/// `/meta`'s attributes, failing if any of them cannot be read. +/// +/// `Group::attrs` leaves out an attribute it cannot decode. For the store's +/// settings that would silently fall back to defaults (e.g. `float16`, the +/// WAL mark), so an unreadable attribute is an error here, as it was before +/// `attrs` became tolerant. +fn meta_attrs( + file: &clawhdf5::File, +) -> Result, MemoryError> { + let meta = file + .group("meta") + .map_err(|e| MemoryError::Schema(format!("missing /meta group: {e}")))?; + let (attrs, errors) = meta + .attrs_with_errors() + .map_err(|e| MemoryError::Schema(format!("cannot read /meta attrs: {e}")))?; + if let Some(e) = errors.first() { + return Err(MemoryError::Schema(format!( + "cannot read /meta attrs: {} unreadable, first: {e}", + errors.len() + ))); + } + Ok(attrs) +} + /// Validate an HDF5 file has the correct schema and load all data. /// Read the checkpoint's [`WalMark`] from `/meta`, if it has one. pub fn read_wal_mark(file: &clawhdf5::File) -> Option { - let attrs = file.group("meta").ok()?.attrs().ok()?; + let attrs = meta_attrs(file).ok()?; let len = match attrs.get(WAL_APPLIED_LEN_ATTR)? { AttrValue::I64(v) => u64::try_from(*v).ok()?, _ => return None, @@ -498,10 +522,7 @@ pub fn read_signature( file: &clawhdf5::File, ) -> Result, MemoryError> { use crate::signing::{Manifest, StoredSignature, from_hex}; - let attrs = file - .group("meta") - .and_then(|g| g.attrs()) - .map_err(|e| MemoryError::Schema(format!("cannot read /meta attrs: {e}")))?; + let attrs = meta_attrs(file)?; let version = match attrs.get(SIG_VERSION_ATTR) { None => return Ok(None), Some(AttrValue::I64(v)) => *v, @@ -552,18 +573,14 @@ pub fn read_signature( /// Read the checkpoint bookkeeping from `/meta`. pub fn read_checkpoint_meta(file: &clawhdf5::File) -> CheckpointMeta { - let ann_generation = file - .group("meta") - .ok() - .and_then(|g| g.attrs().ok()) - .and_then(|attrs| match attrs.get(ANN_GENERATION_ATTR) { - Some(AttrValue::I64(v)) => Some(*v as u64), - _ => None, - }); - let signed = file - .group("meta") - .and_then(|g| g.attrs()) - .is_ok_and(|attrs| attrs.contains_key(SIG_VERSION_ATTR)); + let ann_generation = + meta_attrs(file) + .ok() + .and_then(|attrs| match attrs.get(ANN_GENERATION_ATTR) { + Some(AttrValue::I64(v)) => Some(*v as u64), + _ => None, + }); + let signed = meta_attrs(file).is_ok_and(|attrs| attrs.contains_key(SIG_VERSION_ATTR)); CheckpointMeta { wal_applied: read_wal_mark(file), ann_generation, @@ -575,12 +592,7 @@ pub fn validate_and_load( file: &clawhdf5::File, ) -> Result<(MemoryConfig, MemoryCache, SessionCache, KnowledgeCache), MemoryError> { // Read /meta group attributes - let meta = file - .group("meta") - .map_err(|e| MemoryError::Schema(format!("missing /meta group: {e}")))?; - let attrs = meta - .attrs() - .map_err(|e| MemoryError::Schema(format!("cannot read /meta attrs: {e}")))?; + let attrs = meta_attrs(file)?; let schema_version = match attrs.get("schema_version") { Some(AttrValue::String(s)) => s.clone(), diff --git a/crates/clawhdf5-agent/tests/float16_store.rs b/crates/clawhdf5-agent/tests/float16_store.rs index 0c20d62..9bc3168 100644 --- a/crates/clawhdf5-agent/tests/float16_store.rs +++ b/crates/clawhdf5-agent/tests/float16_store.rs @@ -258,3 +258,43 @@ fn an_existing_f32_store_stays_f32() { assert_eq!(&values[..before.1.len()], before.1.as_slice()); assert_eq!(&values[before.1.len()..], odd.as_slice()); } + +/// `Group::attrs` leaves out an attribute it cannot decode. A store whose +/// `float16` setting is unreadable must not open as `float16 = false` (or with +/// any other default in place of a setting it has): it is an error. +#[test] +fn unreadable_meta_attribute_fails_open_instead_of_defaulting() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("store.h5"); + { + let mut m = HDF5Memory::create(config(&dir, "store.h5", true)).unwrap(); + m.save(entry(1)).unwrap(); + m.flush_wal().unwrap(); + } + assert!(HDF5Memory::open_read_only(&path).is_ok()); + + // Give the `float16` attribute message an unknown version (the name is + // at +8 in a version-1 message and +9 in a version-3 one). + let mut bytes = std::fs::read(&path).unwrap(); + let name = b"float16\0"; + let mut hit = false; + let positions: Vec = (9..bytes.len() - name.len()) + .filter(|&p| &bytes[p..p + name.len()] == name) + .collect(); + for pos in positions { + for (back, version) in [(8, 1u8), (9, 3u8)] { + if bytes[pos - back] == version { + bytes[pos - back] = 0x7f; + hit = true; + } + } + } + assert!(hit, "float16 attribute message not found"); + std::fs::write(&path, &bytes).unwrap(); + + match HDF5Memory::open_read_only(&path) { + Err(MemoryError::Schema(msg)) => assert!(msg.contains("/meta"), "{msg}"), + Err(e) => panic!("unexpected error: {e}"), + Ok(_) => panic!("store opened with an unreadable float16 setting"), + } +}