fix(format): keep reading the floats and empty strings clawhdf5 v2.7.0 wrote

Two of the datatype checks added on this branch refused files clawhdf5
itself wrote up to v2.7.0: it put the sign bit of every float at
position 63 (so every f32 it wrote failed "sign bit position out of
bounds", including every agent store's embeddings), and wrote an
empty-string attribute with a size-0 string type ("invalid datatype
size", failing every attribute of the object). libhdf5 refuses both, but
neither decodes to wrong values (an IEEE float's sign position is not
used; a size-0 string is empty), so this reader keeps accepting them.

New fixtures written by clawhdf5 v2.7.0 (FileBuilder with every datatype,
layout and attribute kind it could write, and a FileWriter paged file)
and legacy_writer_files.rs, which reads every object of them. The agent's
v2.5.0 store fixture (float16_store) passes again.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

crates/clawhdf5-format/src/datatype.rs

@@ -289,9 +289,15 @@ fn ranges_overlap(a0: u64, a1: u64, b0: u64, b1: u64) -> bool {
     a0 <= b1 && b0 <= a1
 }
 
-/// libhdf5's checks on a floating-point type's fields: sign, exponent and
+/// libhdf5's checks on a floating-point type's fields: exponent and mantissa
-/// mantissa must lie inside the type, be non-empty, and not overlap.
+/// must lie inside the type, be non-empty, and not overlap each other or the
-/// (libhdf5 does not check a float's bit offset and precision.)
+/// sign bit. (libhdf5 does not check a float's bit offset and precision.)
+///
+/// One libhdf5 check is left out on purpose: a sign bit position outside the
+/// type ("sign bit position out of bounds"). clawhdf5 up to v2.7.0 wrote 63
+/// there for every float, so every `f32` it wrote (every agent store's
+/// embeddings) would stop opening. The position is not used to decode an
+/// IEEE float, so reading such a type returns the right values.
 fn check_float_fields(
     size: u32,
     sign: u8,
@@ -308,9 +314,6 @@ fn check_float_fields(
         u64::from(mpos),
         u64::from(msize),
     );
-    if sign >= bits {
-        return Err(invalid("sign bit position out of bounds"));
-    }
     if esize == 0 {
         return Err(invalid("exponent size can't be zero"));
     }
@@ -380,7 +383,12 @@ impl Datatype {
 
         let size = LittleEndian::read_u32(&data[4..8]);
         let mut pos = 8;
-        if size == 0 {
+        // libhdf5 refuses size 0 for every class. A fixed-length string is
+        // exempt: clawhdf5 up to v2.7.0 wrote an empty-string attribute
+        // with a size-0 string type, and refusing it would fail every
+        // attribute of such objects, while reading it (an empty string) is
+        // harmless.
+        if size == 0 && class_id != 3 {
             return Err(invalid("invalid datatype size"));
         }
 
@@ -2335,10 +2343,9 @@ mod tests {
         let mut data = build_dt_header(9, 1, [1, 0, 0], 0);
         data.extend_from_slice(&build_fixed_point(1, false, false, 0, 8));
         assert_eq!(invalid_reason(&data), "invalid datatype size");
-        assert_eq!(
+        // Except a fixed-length string, which clawhdf5 <= v2.7.0 wrote for an
-            invalid_reason(&build_dt_header(3, 1, [0, 0, 0], 0)),
+        // empty-string attribute.
-            "invalid datatype size"
+        assert!(Datatype::parse(&build_dt_header(3, 1, [0, 0, 0], 0)).is_ok());
-        );
         assert_eq!(
             invalid_reason(&build_fixed_point(0, false, false, 0, 0)),
             "invalid datatype size"
@@ -2376,7 +2383,6 @@ mod tests {
         };
         assert!(Datatype::parse(&f32_with(31, 23, 8, 0, 23)).is_ok());
         for (fields, why) in [
-            ((32, 23, 8, 0, 23), "sign bit position out of bounds"),
             ((31, 23, 0, 0, 23), "exponent size can't be zero"),
             (
                 (31, 32, 8, 0, 23),
@@ -2447,6 +2453,18 @@ mod tests {
         assert!(Datatype::parse_in_header(&data, 1).is_err());
     }
 
+    #[test]
+    fn f32_written_by_clawhdf5_up_to_2_7_0_still_parses() {
+        // Those versions put the sign bit at 63 whatever the float's size;
+        // libhdf5 refuses it ("sign bit position out of bounds").
+        let mut data = build_dt_header(1, 1, [0x20, 63, 0], 4);
+        data.extend_from_slice(&0u16.to_le_bytes());
+        data.extend_from_slice(&32u16.to_le_bytes());
+        data.extend_from_slice(&[23, 8, 0, 23]);
+        data.extend_from_slice(&127u32.to_le_bytes());
+        assert!(Datatype::parse(&data).is_ok());
+    }
+
     #[test]
     fn float_bit_6_is_vax_order_only_from_version_3() {
         // h5py opens a v1 float with bit 6 set as an ordinary little-endian

crates/clawhdf5/tests/legacy_writer_files.rs

@@ -0,0 +1,83 @@
+//! Files written by older clawhdf5 releases must keep opening, even where
+//! libhdf5 refuses them: stricter validation of corrupt files must not lock
+//! users out of their own data.
+//!
+//! `fixtures/written_by_v2_7_0.h5` and `written_by_v2_7_0_paged.h5` were
+//! written by clawhdf5 v2.7.0 (`FileBuilder` / `FileWriter` with every
+//! datatype, layout and attribute kind it could write). v2.7.0 wrote the sign
+//! bit of every float at position 63 and a size-0 string type for an empty
+//! string attribute; libhdf5 refuses both, this reader must not.
+
+use std::path::PathBuf;
+
+use clawhdf5::{AttrValue, File};
+
+fn fixture(name: &str) -> PathBuf {
+    PathBuf::from(env!("CARGO_MANIFEST_DIR"))
+        .join("tests/fixtures")
+        .join(name)
+}
+
+#[test]
+fn every_object_of_a_v2_7_0_file_reads() {
+    let file = File::open(fixture("written_by_v2_7_0.h5")).unwrap();
+
+    let (attrs, errors) = file.root().attrs_with_errors().unwrap();
+    assert!(errors.is_empty(), "{errors:?}");
+    // (It reads as no strings, as it did before.)
+    assert!(
+        matches!(&attrs["empty"], AttrValue::StringArray(v) if v.iter().all(String::is_empty)),
+        "{:?}",
+        attrs["empty"]
+    );
+    assert!(matches!(&attrs["title"], AttrValue::String(s) if s == "old"));
+
+    let f32s = |name: &str| file.dataset(name).unwrap().read_f32().unwrap();
+    assert_eq!(f32s("f32"), [1.0, 2.0, 3.0]);
+    assert_eq!(
+        f32s("f32_2d"),
+        (0..60).map(|x| x as f32).collect::<Vec<_>>()
+    );
+    assert_eq!(
+        f32s("chunked"),
+        (0..1000).map(|x| x as f32).collect::<Vec<_>>()
+    );
+    assert!(f32s("empty").is_empty());
+    assert_eq!(
+        file.dataset("f64").unwrap().read_f64().unwrap(),
+        [1.0, 2.0, 3.0]
+    );
+    assert_eq!(file.dataset("i32").unwrap().read_i32().unwrap(), [1, -2, 3]);
+    assert_eq!(file.dataset("i64").unwrap().read_i64().unwrap(), [1, -2, 3]);
+    assert_eq!(file.dataset("u64").unwrap().read_u64().unwrap(), [1, 2, 3]);
+    assert_eq!(
+        file.dataset("chunked_2d").unwrap().read_i32().unwrap(),
+        (0..600).collect::<Vec<_>>()
+    );
+    for name in ["unlimited", "maxshape"] {
+        assert_eq!(
+            file.dataset(name).unwrap().read_f64().unwrap(),
+            (0..100).map(|x| x as f64).collect::<Vec<_>>(),
+            "{name}"
+        );
+    }
+    assert_eq!(
+        file.dataset("compact").unwrap().read_i32().unwrap(),
+        [7, 8, 9]
+    );
+    for name in ["u8", "compound", "enum", "enum8"] {
+        file.dataset(name)
+            .unwrap()
+            .dtype()
+            .unwrap_or_else(|e| panic!("{name}: {e}"));
+    }
+
+    let grp = file.group("grp").unwrap();
+    let (attrs, errors) = grp.attrs_with_errors().unwrap();
+    assert!(errors.is_empty(), "{errors:?}");
+    assert_eq!(attrs.len(), 21);
+    assert_eq!(grp.dataset("d").unwrap().read_f32().unwrap(), [4.0, 5.0]);
+
+    let paged = File::open(fixture("written_by_v2_7_0_paged.h5")).unwrap();
+    assert_eq!(paged.dataset("d").unwrap().read_f32().unwrap(), [1.0, 2.0]);
+}