CHANGELOG.md

@@ -333,6 +333,13 @@
     a file shorter than the end of file its superblock records is refused
     ("truncated file"), and nothing past that end is read. `File`,
     `LazyFile` and `MmapFile` do this.
+  - the writer: `FileWriter::finish()` / `FileBuilder::finish()` refuse a
+    datatype the reader would refuse (`FormatError::SerializationError`,
+    "datatype cannot be written: ..."), such as a compound with a repeated
+    field name or no fields, or an enum member with an empty name
+    (`CompoundTypeBuilder` and `EnumTypeBuilder` build them without
+    complaint). These were never valid HDF5 — h5py refuses them — and
+    clawhdf5 wrote them, which made files it could not read back.
 
   Checks newer libhdf5 releases make but HDF5 2.0 does not (bit-field
   offsets, the variable-length kind, array sizes) are left out, so files

crates/clawhdf5-format/src/datatype.rs

@@ -1141,9 +1141,23 @@ impl Datatype {
     }
 
     /// Check that this datatype can be written: every part of it has an
-    /// on-disk encoding. [`Self::serialize`] cannot report errors, so the
+    /// on-disk encoding, and the encoding is one the reader (and libhdf5)
-    /// writer calls this first.
+    /// accepts. [`Self::serialize`] cannot report errors, so the writer calls
+    /// this first. A compound with no fields or a repeated field name, or an
+    /// enum member with an empty name, is refused here: libhdf5 and h5py
+    /// refuse such types, and so does [`Self::parse`], so writing one made a
+    /// file that could not be read back.
     pub fn check_encodable(&self) -> Result<(), FormatError> {
+        self.check_encodable_parts()?;
+        Self::parse(&self.serialize()).map_err(|e| {
+            FormatError::SerializationError(format!(
+                "datatype cannot be written: HDF5 readers refuse it ({e})"
+            ))
+        })?;
+        Ok(())
+    }
+
+    fn check_encodable_parts(&self) -> Result<(), FormatError> {
         match self {
             Datatype::Opaque { tag, .. } if opaque_tag_text(tag).len() > MAX_OPAQUE_TAG_LEN => {
                 Err(FormatError::SerializationError(format!(
@@ -1156,10 +1170,10 @@ impl Datatype {
             )),
             Datatype::Compound { members, .. } => members
                 .iter()
-                .try_for_each(|m| m.datatype.check_encodable()),
+                .try_for_each(|m| m.datatype.check_encodable_parts()),
             Datatype::Enumeration { base_type, .. }
             | Datatype::VariableLength { base_type, .. }
-            | Datatype::Array { base_type, .. } => base_type.check_encodable(),
+            | Datatype::Array { base_type, .. } => base_type.check_encodable_parts(),
             _ => Ok(()),
         }
     }

crates/clawhdf5/tests/legacy_writer_files.rs

@@ -81,3 +81,61 @@ fn every_object_of_a_v2_7_0_file_reads() {
     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]);
 }
+
+/// Datatypes libhdf5 (and this reader) refuse — a compound with a repeated
+/// field name or no fields, an enum member with an empty name — must not be
+/// written: they made files that did not read back. Valid neighbours of each
+/// still write and read back.
+#[test]
+fn datatypes_the_reader_refuses_are_not_written() {
+    use clawhdf5::{CompoundTypeBuilder, EnumTypeBuilder, FileBuilder};
+
+    let write_compound = |dt, raw: Vec<u8>| {
+        let mut b = FileBuilder::new();
+        b.create_dataset("d").with_compound_data(dt, raw, 1);
+        b.finish()
+    };
+    let write_enum = |dt| {
+        let mut b = FileBuilder::new();
+        b.create_dataset("d").with_enum_u8_data(dt, &[0, 1]);
+        b.finish()
+    };
+    let refused = |r: Result<Vec<u8>, clawhdf5::Error>, what: &str| {
+        let err = r.expect_err("written");
+        let msg = err.to_string();
+        assert!(msg.contains("datatype cannot be written"), "{msg}");
+        assert!(msg.contains(what), "{msg}");
+    };
+
+    let dup = CompoundTypeBuilder::new()
+        .f64_field("x")
+        .f64_field("x")
+        .build();
+    refused(
+        write_compound(dup, vec![0; 16]),
+        "duplicated compound field name 'x'",
+    );
+    refused(
+        write_compound(CompoundTypeBuilder::new().build(), vec![]),
+        "invalid",
+    );
+    let empty_name = EnumTypeBuilder::u8_based()
+        .u8_value("A", 0)
+        .u8_value("", 1)
+        .build();
+    refused(write_enum(empty_name), "0 length enum name");
+
+    let ok = CompoundTypeBuilder::new()
+        .f64_field("x")
+        .f64_field("y")
+        .build();
+    let bytes = write_compound(ok, vec![0; 16]).unwrap();
+    let file = File::from_bytes(bytes).unwrap();
+    file.dataset("d").unwrap().dtype().unwrap();
+    let ok = EnumTypeBuilder::u8_based()
+        .u8_value("A", 0)
+        .u8_value("B", 1)
+        .build();
+    let file = File::from_bytes(write_enum(ok).unwrap()).unwrap();
+    file.dataset("d").unwrap().dtype().unwrap();
+}