Fix silent wrong data and libhdf5 interop found by the HDF5 audit #11
@@ -842,6 +842,9 @@ impl Datatype {
|
|||||||
opaque_tag_text(tag).len()
|
opaque_tag_text(tag).len()
|
||||||
)))
|
)))
|
||||||
}
|
}
|
||||||
|
Datatype::String { size: 0, .. } => Err(FormatError::SerializationError(
|
||||||
|
"fixed-length string datatype of size 0 (libhdf5 requires at least 1 byte)".into(),
|
||||||
|
)),
|
||||||
Datatype::Compound { members, .. } => members
|
Datatype::Compound { members, .. } => members
|
||||||
.iter()
|
.iter()
|
||||||
.try_for_each(|m| m.datatype.check_encodable()),
|
.try_for_each(|m| m.datatype.check_encodable()),
|
||||||
|
|||||||
@@ -384,7 +384,11 @@ pub(crate) fn build_attr_message(name: &str, value: &AttrValue) -> AttributeMess
|
|||||||
raw_data: data.clone(),
|
raw_data: data.clone(),
|
||||||
},
|
},
|
||||||
AttrValue::String(s) => {
|
AttrValue::String(s) => {
|
||||||
let bytes = s.as_bytes();
|
// A fixed-length string type must be at least 1 byte: libhdf5
|
||||||
|
// rejects size 0 ("invalid datatype size") and with it every
|
||||||
|
// attribute on the object. h5py stores "" as one NUL byte.
|
||||||
|
let mut bytes = s.as_bytes().to_vec();
|
||||||
|
bytes.resize(bytes.len().max(1), 0);
|
||||||
AttributeMessage {
|
AttributeMessage {
|
||||||
name: name.to_string(),
|
name: name.to_string(),
|
||||||
datatype: Datatype::String {
|
datatype: Datatype::String {
|
||||||
@@ -393,11 +397,12 @@ pub(crate) fn build_attr_message(name: &str, value: &AttrValue) -> AttributeMess
|
|||||||
charset: CharacterSet::Utf8,
|
charset: CharacterSet::Utf8,
|
||||||
},
|
},
|
||||||
dataspace: scalar_ds(),
|
dataspace: scalar_ds(),
|
||||||
raw_data: bytes.to_vec(),
|
raw_data: bytes,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
AttrValue::StringArray(arr) => {
|
AttrValue::StringArray(arr) => {
|
||||||
let max_len = arr.iter().map(|s| s.len()).max().unwrap_or(0);
|
// At least 1 byte per element, as for a single string.
|
||||||
|
let max_len = arr.iter().map(|s| s.len()).max().unwrap_or(0).max(1);
|
||||||
let mut raw = Vec::new();
|
let mut raw = Vec::new();
|
||||||
for s in arr {
|
for s in arr {
|
||||||
let mut b = s.as_bytes().to_vec();
|
let mut b = s.as_bytes().to_vec();
|
||||||
|
|||||||
@@ -464,3 +464,63 @@ fn h5py_sees_our_fill_time_and_fill_value() {
|
|||||||
);
|
);
|
||||||
h5dump_ok(&path);
|
h5dump_ok(&path);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ---- 5. empty string attributes ----
|
||||||
|
|
||||||
|
fn empty_string_file() -> Vec<u8> {
|
||||||
|
let mut fw = FileWriter::new();
|
||||||
|
fw.set_root_attr("empty", AttrValue::String(String::new()));
|
||||||
|
fw.set_root_attr("x", AttrValue::String("héllo".into()));
|
||||||
|
fw.set_root_attr(
|
||||||
|
"empties",
|
||||||
|
AttrValue::StringArray(vec![String::new(), String::new()]),
|
||||||
|
);
|
||||||
|
fw.set_root_attr("n", AttrValue::I64(3));
|
||||||
|
fw.finish().unwrap()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn empty_string_attribute_has_a_one_byte_type() {
|
||||||
|
// Measured: "" got a size-0 string type, and libhdf5 then refused every
|
||||||
|
// attribute on the object ("invalid datatype size").
|
||||||
|
let bytes = empty_string_file();
|
||||||
|
let (sb, root) = header_at(&bytes, "/");
|
||||||
|
let attrs = clawhdf5_format::attribute::extract_attributes(&root, sb.length_size).unwrap();
|
||||||
|
for name in ["empty", "empties"] {
|
||||||
|
let a = attrs.iter().find(|a| a.name == name).unwrap();
|
||||||
|
assert_eq!(a.datatype.type_size(), 1, "{name}");
|
||||||
|
let strings = a.read_as_strings().unwrap();
|
||||||
|
assert!(strings.iter().all(String::is_empty), "{name}: {strings:?}");
|
||||||
|
}
|
||||||
|
|
||||||
|
// A size-0 string type handed in directly is refused, not written.
|
||||||
|
let mut fw = FileWriter::new();
|
||||||
|
fw.set_root_attr(
|
||||||
|
"raw",
|
||||||
|
AttrValue::Raw {
|
||||||
|
datatype: Datatype::String {
|
||||||
|
size: 0,
|
||||||
|
padding: clawhdf5_format::datatype::StringPadding::NullPad,
|
||||||
|
charset: clawhdf5_format::datatype::CharacterSet::Ascii,
|
||||||
|
},
|
||||||
|
shape: vec![],
|
||||||
|
data: vec![],
|
||||||
|
},
|
||||||
|
);
|
||||||
|
assert!(fw.finish().is_err());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
#[ignore = "requires Python h5py module and h5dump"]
|
||||||
|
fn h5py_reads_all_attributes_next_to_an_empty_string() {
|
||||||
|
let path = write_tmp("empty_str", &empty_string_file());
|
||||||
|
let out = h5py(
|
||||||
|
&path,
|
||||||
|
"f = h5py.File(path, 'r')\n\
|
||||||
|
d = lambda v: v.decode() if isinstance(v, bytes) else v\n\
|
||||||
|
print(json.dumps([d(f.attrs['empty']), d(f.attrs['x']),\n\
|
||||||
|
\x20 [d(s) for s in f.attrs['empties']], int(f.attrs['n'])], ensure_ascii=False))",
|
||||||
|
);
|
||||||
|
assert_eq!(out, r#"["", "héllo", ["", ""], 3]"#);
|
||||||
|
h5dump_ok(&path);
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user