From 92386056610508dd0666ffc3e79e6b1446404ffb Mon Sep 17 00:00:00 2001
From: osobh <om.sobh@gmail.com>
Date: Sat, 26 Sep 2026 01:24:17 -0500
Subject: [PATCH] fix(format): measure compound members by their stored size

The compound overlap check measured each earlier member with
Datatype::type_size, which is a fixed 16 for a variable-length type. On
disk a VL member takes 4 + offset size + 4 bytes, 12 in a file with
4-byte offsets, so a member right after one was refused as "member
overlaps with previous member" (and with the type, every attribute of
the object). libhdf5 measures members by their decoded, stored size
(times a v1 member's array dimensions); so does this now.

Reading VL values in such files is a separate, older gap, now recorded
in known-issues.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---
 CHANGELOG.md                                  |  3 ++
 crates/clawhdf5-format/src/datatype.rs        | 28 ++++++----
 .../tests/header_validation_interop.rs        | 51 +++++++++++++++++++
 docs/known-issues.md                          |  6 +++
 4 files changed, 78 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e555823..c0ff1e2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -321,6 +321,9 @@
     half its bits unused (`Datatype::parse_in_header`,
     `Datatype::check_unused_bits`). A v1/v2 float's class bit 6 was read as
     VAX byte order; libhdf5 ignores it before version 3, and so does this.
+    The overlap check measures each earlier member by its stored size, as
+    libhdf5 does, so a variable-length member (4 + offset size + 4 bytes)
+    in a file with 4-byte offsets does not overlap the member after it.
   - chunked layouts (`FormatError::InvalidChunkDimensions`): a zero chunk
     dimension, a chunk rank that does not match the dataspace, a chunk of
     4 GiB or more indexed by a v1 B-tree (layout version 3 or earlier;
diff --git a/crates/clawhdf5-format/src/datatype.rs b/crates/clawhdf5-format/src/datatype.rs
index 703f44a..12e427f 100644
--- a/crates/clawhdf5-format/src/datatype.rs
+++ b/crates/clawhdf5-format/src/datatype.rs
@@ -215,11 +215,6 @@ fn stored_type_size(data: &[u8], pos: usize) -> Result<u32, FormatError> {
     Ok(LittleEndian::read_u32(&data[pos + 4..pos + 8]))
 }
 
-/// A compound member's size in the compound, as libhdf5 counts it.
-fn member_size(dt: &Datatype) -> u64 {
-    u64::from(dt.type_size())
-}
-
 /// libhdf5 refuses an array type of more than `H5S_MAX_RANK` (32)
 /// dimensions.
 fn check_array_rank(ndims: usize) -> Result<(), FormatError> {
@@ -543,11 +538,17 @@ impl Datatype {
                     return Err(invalid("invalid number of members: 0"));
                 }
                 let mut members: Vec<CompoundMember> = Vec::with_capacity(num_members as usize);
+                // Each member's size in the compound as libhdf5 decodes it:
+                // its stored size, times a v1 member's array dimensions. A
+                // variable-length member takes 4 + offset size + 4 bytes on
+                // disk, not the 16 of `Datatype::type_size`.
+                let mut member_sizes: Vec<u64> = Vec::with_capacity(num_members as usize);
                 // libhdf5 checks each member as it is decoded: it must fit in
                 // the compound (by its own stored size, before a v1 member's
                 // array dimensions are applied), and must not repeat a name
                 // or overlap an earlier member (by its final size).
                 let check_member = |members: &[CompoundMember],
+                                    member_sizes: &[u64],
                                     name: &str,
                                     byte_offset: u64,
                                     stored_size: u32,
@@ -565,9 +566,8 @@ impl Datatype {
                         )));
                     }
                     let end = byte_offset + final_size;
-                    if members.iter().any(|m| {
-                        let m_end = m.byte_offset + member_size(&m.datatype);
-                        byte_offset < m_end && m.byte_offset < end
+                    if members.iter().zip(member_sizes).any(|(m, &m_size)| {
+                        byte_offset < m.byte_offset + m_size && m.byte_offset < end
                     }) {
                         return Err(invalid("member overlaps with previous member"));
                     }
@@ -588,13 +588,16 @@ impl Datatype {
                         let (member_dt, consumed) =
                             Self::parse_with_depth(&data[pos..], depth + 1)?;
                         pos += consumed;
+                        let final_size = u64::from(stored_size);
                         check_member(
                             &members,
+                            &member_sizes,
                             &name,
                             byte_offset,
                             stored_size,
-                            member_size(&member_dt),
+                            final_size,
                         )?;
+                        member_sizes.push(final_size);
                         members.push(CompoundMember {
                             name,
                             byte_offset,
@@ -652,6 +655,9 @@ impl Datatype {
                         let (mut member_dt, consumed) =
                             Self::parse_with_depth(&data[pos..], depth + 1)?;
                         pos += consumed;
+                        let final_size = array_dims.iter().fold(u64::from(stored_size), |a, &d| {
+                            a.saturating_mul(u64::from(d))
+                        });
                         if !array_dims.is_empty() {
                             member_dt = Datatype::Array {
                                 base_type: Box::new(member_dt),
@@ -660,11 +666,13 @@ impl Datatype {
                         }
                         check_member(
                             &members,
+                            &member_sizes,
                             &name,
                             byte_offset,
                             stored_size,
-                            member_size(&member_dt),
+                            final_size,
                         )?;
+                        member_sizes.push(final_size);
                         members.push(CompoundMember {
                             name,
                             byte_offset,
diff --git a/crates/clawhdf5/tests/header_validation_interop.rs b/crates/clawhdf5/tests/header_validation_interop.rs
index daa3e19..a2cebf5 100644
--- a/crates/clawhdf5/tests/header_validation_interop.rs
+++ b/crates/clawhdf5/tests/header_validation_interop.rs
@@ -313,3 +313,54 @@ with h5py.File(os.path.join(d, "big.h5"), "r") as f:
     let values = file.dataset("d").unwrap().read_f64().unwrap();
     assert_eq!(values, (0..10).map(f64::from).collect::<Vec<_>>());
 }
+
+/// A variable-length compound member takes 4 + offset size + 4 bytes, which
+/// is 12 in a file with 4-byte offsets, and libhdf5 checks for overlapping
+/// members with that stored size. A member right after one was refused as
+/// "member overlaps with previous member" (the check took the 16 bytes of
+/// an 8-byte-offset file), and with it every attribute of the object.
+#[test]
+fn variable_length_compound_members_in_files_with_4_byte_offsets() {
+    skip_if_no_python!();
+    let dir = tempfile::tempdir().unwrap();
+    run_python(
+        dir.path(),
+        r#"
+from h5py import h5f, h5p
+dt = np.dtype([("s", h5py.string_dtype()), ("i", "<i4")])
+a = np.array([("x", 1), ("yy", 2)], dtype=dt)
+for libver in ("earliest", "latest"):
+    fcpl = h5p.create(h5p.FILE_CREATE)
+    fcpl.set_sizes(4, 4)
+    fapl = h5p.create(h5p.FILE_ACCESS)
+    low = h5f.LIBVER_EARLIEST if libver == "earliest" else h5f.LIBVER_LATEST
+    fapl.set_libver_bounds(low, h5f.LIBVER_LATEST)
+    path = os.path.join(d, f"{libver}.h5")
+    with h5py.File(h5f.create(path.encode(), h5f.ACC_TRUNC, fcpl=fcpl, fapl=fapl)) as f:
+        f.create_dataset("c", data=a)
+        f.attrs["c"] = a
+        f.attrs["note"] = "kept"
+    with h5py.File(path, "r") as f:
+        assert f["c"][1]["i"] == 2
+        assert f.attrs["note"] == "kept"
+"#,
+    );
+    for libver in ["earliest", "latest"] {
+        let file = File::open(dir.path().join(format!("{libver}.h5"))).unwrap();
+        let dtype = file.dataset("c").unwrap().dtype();
+        assert!(
+            matches!(&dtype, Ok(clawhdf5::DType::Compound(fields))
+                if fields.iter().map(|f| f.0.as_str()).eq(["s", "i"])),
+            "{libver}: {dtype:?}"
+        );
+        // (Variable-length values in files with 4-byte offsets are not
+        // decoded yet; see docs/known-issues.md. The attributes must at
+        // least be listed without errors.)
+        let (attrs, errors) = file.root().attrs_with_errors().unwrap();
+        assert!(errors.is_empty(), "{libver}: {errors:?}");
+        assert!(
+            attrs.contains_key("c") && attrs.contains_key("note"),
+            "{libver}: {attrs:?}"
+        );
+    }
+}
diff --git a/docs/known-issues.md b/docs/known-issues.md
index 07c888f..7444f15 100644
--- a/docs/known-issues.md
+++ b/docs/known-issues.md
@@ -129,6 +129,12 @@ fill-value item that did is fixed).
   failing the others.
 - **Other readers:**
   - VL-string datasets are not readable through `File`.
+  - Variable-length values inside a compound (and VL-string attributes) in
+    a file with 4-byte offsets (`sizeof_addr = 4`) fail with
+    `GlobalHeapObjectNotFound` or come back as `Raw`: these paths assume
+    the 16-byte element of an 8-byte-offset file. The datatype itself reads
+    (it was refused as "member overlaps with previous member" until
+    2026-09-26).
   - Metadata cache images are not supported.
   - x87 long double and binary128 are refused.
   - N-Bit on 64-bit scale-offset data and some N-Bit parameter layouts fail.