Fix silent wrong data and libhdf5 interop found by the HDF5 audit #11
@@ -4,7 +4,7 @@
|
|||||||
//! for compound, enumeration, variable-length, and array types.
|
//! for compound, enumeration, variable-length, and array types.
|
||||||
|
|
||||||
#[cfg(not(feature = "std"))]
|
#[cfg(not(feature = "std"))]
|
||||||
use alloc::{boxed::Box, string::String, vec, vec::Vec};
|
use alloc::{boxed::Box, format, string::String, vec, vec::Vec};
|
||||||
|
|
||||||
use byteorder::{ByteOrder, LittleEndian};
|
use byteorder::{ByteOrder, LittleEndian};
|
||||||
|
|
||||||
@@ -137,6 +137,17 @@ pub enum Datatype {
|
|||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Longest opaque tag that can be stored: its NUL-padded length must fit
|
||||||
|
/// the 8-bit length in the datatype's class bits.
|
||||||
|
pub const MAX_OPAQUE_TAG_LEN: usize = 248;
|
||||||
|
|
||||||
|
/// An opaque tag up to (not including) its first NUL.
|
||||||
|
fn opaque_tag_text(tag: &[u8]) -> &[u8] {
|
||||||
|
tag.iter()
|
||||||
|
.position(|&b| b == 0)
|
||||||
|
.map_or(tag, |end| &tag[..end])
|
||||||
|
}
|
||||||
|
|
||||||
fn ensure_len(data: &[u8], offset: usize, needed: usize) -> Result<(), FormatError> {
|
fn ensure_len(data: &[u8], offset: usize, needed: usize) -> Result<(), FormatError> {
|
||||||
match offset.checked_add(needed) {
|
match offset.checked_add(needed) {
|
||||||
Some(end) if end <= data.len() => Ok(()),
|
Some(end) if end <= data.len() => Ok(()),
|
||||||
@@ -361,7 +372,10 @@ impl Datatype {
|
|||||||
// Opaque
|
// Opaque
|
||||||
let tag_len = bf0 as usize;
|
let tag_len = bf0 as usize;
|
||||||
ensure_len(data, pos, tag_len)?;
|
ensure_len(data, pos, tag_len)?;
|
||||||
let tag = data[pos..pos + tag_len].to_vec();
|
// The stored tag is NUL-padded to a multiple of 8 bytes; the
|
||||||
|
// tag itself ends at the first NUL (libhdf5 reads it with
|
||||||
|
// `strndup`).
|
||||||
|
let tag = opaque_tag_text(&data[pos..pos + tag_len]).to_vec();
|
||||||
// Tags are padded to multiple of 8 bytes
|
// Tags are padded to multiple of 8 bytes
|
||||||
let padded = (tag_len + 7) & !7;
|
let padded = (tag_len + 7) & !7;
|
||||||
let pos = 8 + padded; // from start of properties
|
let pos = 8 + padded; // from start of properties
|
||||||
@@ -767,7 +781,74 @@ impl Datatype {
|
|||||||
buf.extend_from_slice(&base_type.serialize());
|
buf.extend_from_slice(&base_type.serialize());
|
||||||
buf
|
buf
|
||||||
}
|
}
|
||||||
_ => Vec::new(),
|
Datatype::Time {
|
||||||
|
size,
|
||||||
|
bit_precision,
|
||||||
|
} => {
|
||||||
|
// Byte order is not modelled for time types; write little-endian.
|
||||||
|
let mut buf = Self::build_header(2, 1, [0, 0, 0], *size);
|
||||||
|
buf.extend_from_slice(&bit_precision.to_le_bytes());
|
||||||
|
buf
|
||||||
|
}
|
||||||
|
Datatype::BitField {
|
||||||
|
size,
|
||||||
|
byte_order,
|
||||||
|
bit_offset,
|
||||||
|
bit_precision,
|
||||||
|
} => {
|
||||||
|
let bf0 = u8::from(matches!(byte_order, DatatypeByteOrder::BigEndian));
|
||||||
|
let mut buf = Self::build_header(4, 1, [bf0, 0, 0], *size);
|
||||||
|
buf.extend_from_slice(&bit_offset.to_le_bytes());
|
||||||
|
buf.extend_from_slice(&bit_precision.to_le_bytes());
|
||||||
|
buf
|
||||||
|
}
|
||||||
|
Datatype::Opaque { size, tag } => {
|
||||||
|
// The tag is stored NUL-padded to a multiple of 8 bytes and the
|
||||||
|
// padded length goes in the class bits, as libhdf5 writes it.
|
||||||
|
// A tag longer than MAX_OPAQUE_TAG_LEN cannot be encoded;
|
||||||
|
// `check_encodable` rejects it before a file is written.
|
||||||
|
let tag = opaque_tag_text(tag);
|
||||||
|
let tag = &tag[..tag.len().min(MAX_OPAQUE_TAG_LEN)];
|
||||||
|
let padded = tag.len().div_ceil(8) * 8;
|
||||||
|
let mut buf = Self::build_header(5, 1, [padded as u8, 0, 0], *size);
|
||||||
|
buf.extend_from_slice(tag);
|
||||||
|
buf.resize(8 + padded, 0);
|
||||||
|
buf
|
||||||
|
}
|
||||||
|
Datatype::Reference { size, ref_type } => {
|
||||||
|
// Legacy references are datatype version 1; the H5T_STD_REF
|
||||||
|
// kinds only exist from version 4, which also carries their
|
||||||
|
// encoding version (1) in the high nibble.
|
||||||
|
let (version, bf0) = match ref_type {
|
||||||
|
ReferenceType::Object => (1, 0),
|
||||||
|
ReferenceType::DatasetRegion => (1, 1),
|
||||||
|
ReferenceType::Object2 => (4, 0x12),
|
||||||
|
ReferenceType::DatasetRegion2 => (4, 0x13),
|
||||||
|
ReferenceType::Attribute => (4, 0x14),
|
||||||
|
};
|
||||||
|
Self::build_header(7, version, [bf0, 0, 0], *size)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Check that this datatype can be written: every part of it has an
|
||||||
|
/// on-disk encoding. [`Self::serialize`] cannot report errors, so the
|
||||||
|
/// writer calls this first.
|
||||||
|
pub fn check_encodable(&self) -> Result<(), FormatError> {
|
||||||
|
match self {
|
||||||
|
Datatype::Opaque { tag, .. } if opaque_tag_text(tag).len() > MAX_OPAQUE_TAG_LEN => {
|
||||||
|
Err(FormatError::SerializationError(format!(
|
||||||
|
"opaque tag is {} bytes; at most {MAX_OPAQUE_TAG_LEN} can be stored",
|
||||||
|
opaque_tag_text(tag).len()
|
||||||
|
)))
|
||||||
|
}
|
||||||
|
Datatype::Compound { members, .. } => members
|
||||||
|
.iter()
|
||||||
|
.try_for_each(|m| m.datatype.check_encodable()),
|
||||||
|
Datatype::Enumeration { base_type, .. }
|
||||||
|
| Datatype::VariableLength { base_type, .. }
|
||||||
|
| Datatype::Array { base_type, .. } => base_type.check_encodable(),
|
||||||
|
_ => Ok(()),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1625,6 +1706,122 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn hex(s: &str) -> Vec<u8> {
|
||||||
|
(0..s.len())
|
||||||
|
.step_by(2)
|
||||||
|
.map(|i| u8::from_str_radix(&s[i..i + 2], 16).unwrap())
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// `serialize` used to return an empty message for these four classes,
|
||||||
|
/// which libhdf5 rejects ("ran off end of input buffer while decoding").
|
||||||
|
/// Expected bytes are libhdf5's own encoding (HDF5 2.0 `H5Tencode`, or the
|
||||||
|
/// datatype message of an HDF5 2.0 file for `H5T_STD_REF`).
|
||||||
|
#[test]
|
||||||
|
fn serialize_matches_libhdf5_for_time_bitfield_opaque_reference() {
|
||||||
|
let cases = [
|
||||||
|
(
|
||||||
|
Datatype::Reference {
|
||||||
|
size: 8,
|
||||||
|
ref_type: ReferenceType::Object,
|
||||||
|
},
|
||||||
|
"1700000008000000",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
Datatype::Reference {
|
||||||
|
size: 12,
|
||||||
|
ref_type: ReferenceType::DatasetRegion,
|
||||||
|
},
|
||||||
|
"170100000c000000",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
Datatype::Reference {
|
||||||
|
size: 18,
|
||||||
|
ref_type: ReferenceType::Object2,
|
||||||
|
},
|
||||||
|
"4712000012000000",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
Datatype::BitField {
|
||||||
|
size: 1,
|
||||||
|
byte_order: DatatypeByteOrder::LittleEndian,
|
||||||
|
bit_offset: 0,
|
||||||
|
bit_precision: 8,
|
||||||
|
},
|
||||||
|
"140000000100000000000800",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
Datatype::BitField {
|
||||||
|
size: 2,
|
||||||
|
byte_order: DatatypeByteOrder::BigEndian,
|
||||||
|
bit_offset: 0,
|
||||||
|
bit_precision: 16,
|
||||||
|
},
|
||||||
|
"140100000200000000001000",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
Datatype::Opaque {
|
||||||
|
size: 4,
|
||||||
|
tag: b"mytag".to_vec(),
|
||||||
|
},
|
||||||
|
"15080000040000006d79746167000000",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
Datatype::Opaque {
|
||||||
|
size: 4,
|
||||||
|
tag: b"12345678".to_vec(),
|
||||||
|
},
|
||||||
|
"15080000040000003132333435363738",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
Datatype::Opaque {
|
||||||
|
size: 4,
|
||||||
|
tag: vec![],
|
||||||
|
},
|
||||||
|
"1500000004000000",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
Datatype::Time {
|
||||||
|
size: 4,
|
||||||
|
bit_precision: 32,
|
||||||
|
},
|
||||||
|
"12000000040000002000",
|
||||||
|
),
|
||||||
|
];
|
||||||
|
for (dt, expected) in cases {
|
||||||
|
let bytes = dt.serialize();
|
||||||
|
assert_eq!(bytes, hex(expected), "{dt:?}");
|
||||||
|
let (parsed, consumed) = Datatype::parse(&bytes).unwrap();
|
||||||
|
assert_eq!(parsed, dt);
|
||||||
|
assert_eq!(consumed, bytes.len());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn opaque_tag_padding_is_not_part_of_the_tag() {
|
||||||
|
// libhdf5 pads "mytag" to 8 bytes; parsing must not return the NULs,
|
||||||
|
// or copying the type would grow the tag.
|
||||||
|
let (dt, _) = Datatype::parse(&hex("15080000040000006d79746167000000")).unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
dt,
|
||||||
|
Datatype::Opaque {
|
||||||
|
size: 4,
|
||||||
|
tag: b"mytag".to_vec()
|
||||||
|
}
|
||||||
|
);
|
||||||
|
let long = Datatype::Opaque {
|
||||||
|
size: 1,
|
||||||
|
tag: vec![b'x'; MAX_OPAQUE_TAG_LEN + 1],
|
||||||
|
};
|
||||||
|
assert!(long.check_encodable().is_err());
|
||||||
|
let ok = Datatype::Opaque {
|
||||||
|
size: 1,
|
||||||
|
tag: vec![b'x'; MAX_OPAQUE_TAG_LEN],
|
||||||
|
};
|
||||||
|
assert!(ok.check_encodable().is_ok());
|
||||||
|
assert_eq!(Datatype::parse(&ok.serialize()).unwrap().0, ok);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_error_invalid_reference_type() {
|
fn test_error_invalid_reference_type() {
|
||||||
let buf = build_dt_header(7, 1, [5, 0, 0], 8);
|
let buf = build_dt_header(7, 1, [5, 0, 0], 8);
|
||||||
|
|||||||
@@ -1125,6 +1125,17 @@ impl FileWriter {
|
|||||||
root_attrs.push(build_attr_message(n, v));
|
root_attrs.push(build_attr_message(n, v));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Every datatype must have an on-disk encoding before anything is laid
|
||||||
|
// out: `Datatype::serialize` itself cannot report a failure.
|
||||||
|
let group_attrs = groups.iter().flat_map(|g| &g.attrs);
|
||||||
|
let ds_attrs = all_ds.iter().flat_map(|d| &d.attrs);
|
||||||
|
for a in root_attrs.iter().chain(group_attrs).chain(ds_attrs) {
|
||||||
|
a.datatype.check_encodable()?;
|
||||||
|
}
|
||||||
|
for d in &all_ds {
|
||||||
|
d.dt.check_encodable()?;
|
||||||
|
}
|
||||||
|
|
||||||
let is_vds: Vec<bool> = all_ds.iter().map(|d| d.virtual_sources.is_some()).collect();
|
let is_vds: Vec<bool> = all_ds.iter().map(|d| d.virtual_sources.is_some()).collect();
|
||||||
let is_chunked: Vec<bool> = all_ds
|
let is_chunked: Vec<bool> = all_ds
|
||||||
.iter()
|
.iter()
|
||||||
|
|||||||
@@ -6,6 +6,7 @@
|
|||||||
//! (`CLAWHDF5_PYTHON`, as in `writer_h5py_tests.rs`) and run `h5dump` over it.
|
//! (`CLAWHDF5_PYTHON`, as in `writer_h5py_tests.rs`) and run `h5dump` over it.
|
||||||
|
|
||||||
use clawhdf5_format::data_layout::DataLayout;
|
use clawhdf5_format::data_layout::DataLayout;
|
||||||
|
use clawhdf5_format::datatype::{Datatype, DatatypeByteOrder, ReferenceType};
|
||||||
use clawhdf5_format::file_writer::{AttrValue, FileWriter};
|
use clawhdf5_format::file_writer::{AttrValue, FileWriter};
|
||||||
use clawhdf5_format::group_v2::resolve_path_any;
|
use clawhdf5_format::group_v2::resolve_path_any;
|
||||||
use clawhdf5_format::message_type::MessageType;
|
use clawhdf5_format::message_type::MessageType;
|
||||||
@@ -151,3 +152,168 @@ fn h5py_reads_compact_datasets_at_the_limit() {
|
|||||||
h5dump_ok(&path);
|
h5dump_ok(&path);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ---- 2. Time / BitField / Opaque / Reference datatypes ----
|
||||||
|
|
||||||
|
fn exotic_types() -> Vec<(&'static str, Datatype, Vec<u8>)> {
|
||||||
|
// Four elements each. The object references point at the root group,
|
||||||
|
// which a v3-superblock file without an extension puts at address 48.
|
||||||
|
let refs: Vec<u8> = (0..4).flat_map(|_| 48u64.to_le_bytes()).collect();
|
||||||
|
vec![
|
||||||
|
(
|
||||||
|
"bits",
|
||||||
|
Datatype::BitField {
|
||||||
|
size: 1,
|
||||||
|
byte_order: DatatypeByteOrder::LittleEndian,
|
||||||
|
bit_offset: 0,
|
||||||
|
bit_precision: 8,
|
||||||
|
},
|
||||||
|
vec![1, 2, 4, 8],
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"opaque",
|
||||||
|
Datatype::Opaque {
|
||||||
|
size: 4,
|
||||||
|
tag: b"mytag".to_vec(),
|
||||||
|
},
|
||||||
|
(0..16).collect(),
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"ref",
|
||||||
|
Datatype::Reference {
|
||||||
|
size: 8,
|
||||||
|
ref_type: ReferenceType::Object,
|
||||||
|
},
|
||||||
|
refs,
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"time",
|
||||||
|
Datatype::Time {
|
||||||
|
size: 4,
|
||||||
|
bit_precision: 32,
|
||||||
|
},
|
||||||
|
(0..16).collect(),
|
||||||
|
),
|
||||||
|
]
|
||||||
|
}
|
||||||
|
|
||||||
|
fn exotic_file() -> Vec<u8> {
|
||||||
|
let mut fw = FileWriter::new();
|
||||||
|
for (name, dt, raw) in exotic_types() {
|
||||||
|
fw.create_dataset(name)
|
||||||
|
.with_compound_data(dt.clone(), raw.clone(), 4);
|
||||||
|
fw.set_root_attr(
|
||||||
|
name,
|
||||||
|
AttrValue::Raw {
|
||||||
|
datatype: dt,
|
||||||
|
shape: vec![4],
|
||||||
|
data: raw,
|
||||||
|
},
|
||||||
|
);
|
||||||
|
}
|
||||||
|
fw.finish().unwrap()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn exotic_datatypes_are_written_not_emptied() {
|
||||||
|
let bytes = exotic_file();
|
||||||
|
let (sb, root) = header_at(&bytes, "/");
|
||||||
|
assert_eq!(sb.root_group_address, 48);
|
||||||
|
let attrs = clawhdf5_format::attribute::extract_attributes(&root, sb.length_size).unwrap();
|
||||||
|
for (name, dt, raw) in exotic_types() {
|
||||||
|
let (_, oh) = header_at(&bytes, name);
|
||||||
|
let msg = oh
|
||||||
|
.messages
|
||||||
|
.iter()
|
||||||
|
.find(|m| m.msg_type == MessageType::Datatype)
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(msg.data, dt.serialize(), "{name}");
|
||||||
|
assert_eq!(Datatype::parse(&msg.data).unwrap().0, dt, "{name}");
|
||||||
|
let attr = attrs.iter().find(|a| a.name == name).unwrap();
|
||||||
|
assert_eq!(attr.datatype, dt, "{name}");
|
||||||
|
assert_eq!(attr.raw_data, raw, "{name}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
#[ignore = "requires Python h5py module and h5dump"]
|
||||||
|
fn h5py_reads_exotic_datatypes() {
|
||||||
|
let path = write_tmp("exotic", &exotic_file());
|
||||||
|
let out = h5py(
|
||||||
|
&path,
|
||||||
|
"from h5py import h5t, h5s\n\
|
||||||
|
f = h5py.File(path, 'r')\n\
|
||||||
|
r = {}\n\
|
||||||
|
buf = np.zeros(4, dtype='V4')\n\
|
||||||
|
f['opaque'].id.read(h5s.ALL, h5s.ALL, buf, mtype=f['opaque'].id.get_type())\n\
|
||||||
|
r['bits'] = f['bits'][()].tolist(), f.attrs['bits'].tolist()\n\
|
||||||
|
r['opaque'] = (f['opaque'].id.get_type().get_tag().decode(),\n\
|
||||||
|
\x20 f.attrs.get_id('opaque').get_type().get_tag().decode(),\n\
|
||||||
|
\x20 buf.tobytes().hex())\n\
|
||||||
|
r['ref'] = [f[x].name for x in f['ref'][()]] + [f[x].name for x in f.attrs['ref']]\n\
|
||||||
|
r['time'] = (f['time'].id.get_type().get_class() == h5t.TIME,\n\
|
||||||
|
\x20 f.attrs.get_id('time').get_type().get_class() == h5t.TIME)\n\
|
||||||
|
print(json.dumps(r))",
|
||||||
|
);
|
||||||
|
let v: serde_json::Value = serde_json::from_str(&out).unwrap();
|
||||||
|
assert_eq!(v["bits"], serde_json::json!([[1, 2, 4, 8], [1, 2, 4, 8]]));
|
||||||
|
assert_eq!(
|
||||||
|
v["opaque"],
|
||||||
|
serde_json::json!(["mytag", "mytag", "000102030405060708090a0b0c0d0e0f"])
|
||||||
|
);
|
||||||
|
assert_eq!(v["ref"], serde_json::json!(vec!["/"; 8]));
|
||||||
|
assert_eq!(v["time"], serde_json::json!([true, true]));
|
||||||
|
h5dump_ok(&path);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
#[ignore = "requires Python h5py module and h5dump"]
|
||||||
|
fn raw_attributes_copied_from_h5py_survive_a_rewrite() {
|
||||||
|
// Read Raw attributes of the exotic classes out of an h5py file and write
|
||||||
|
// them back: this used to emit empty datatype messages.
|
||||||
|
let src = std::env::temp_dir().join("clawhdf5_writer_meta_exotic_src.h5");
|
||||||
|
h5py(
|
||||||
|
&src,
|
||||||
|
"from h5py import h5t, h5s, h5a\n\
|
||||||
|
f = h5py.File(path, 'w')\n\
|
||||||
|
f.attrs['ref'] = np.array([f.ref, f.ref], dtype=h5py.ref_dtype)\n\
|
||||||
|
f.attrs.create('opaque', np.frombuffer(b'abcdefgh', dtype='V4'))\n\
|
||||||
|
t = h5t.STD_B16BE.copy()\n\
|
||||||
|
a = h5a.create(f.id, b'bits', t, h5s.create_simple((2,)))\n\
|
||||||
|
a.write(np.array([0x0102, 0x0304], dtype='>u2'), mtype=t)\n\
|
||||||
|
a.close()\n\
|
||||||
|
f.close()",
|
||||||
|
);
|
||||||
|
let src_bytes = std::fs::read(&src).unwrap();
|
||||||
|
let (sb, root) = header_at(&src_bytes, "/");
|
||||||
|
let attrs = clawhdf5_format::attribute::extract_attributes(&root, sb.length_size).unwrap();
|
||||||
|
assert_eq!(attrs.len(), 3);
|
||||||
|
let mut fw = FileWriter::new();
|
||||||
|
for a in &attrs {
|
||||||
|
let data = if a.name == "ref" {
|
||||||
|
// Re-target the references at our root group.
|
||||||
|
48u64.to_le_bytes().repeat(2)
|
||||||
|
} else {
|
||||||
|
a.raw_data.clone()
|
||||||
|
};
|
||||||
|
fw.set_root_attr(
|
||||||
|
&a.name,
|
||||||
|
AttrValue::Raw {
|
||||||
|
datatype: a.datatype.clone(),
|
||||||
|
shape: a.dataspace.dimensions.clone(),
|
||||||
|
data,
|
||||||
|
},
|
||||||
|
);
|
||||||
|
}
|
||||||
|
let path = write_tmp("exotic_copy", &fw.finish().unwrap());
|
||||||
|
let out = h5py(
|
||||||
|
&path,
|
||||||
|
"f = h5py.File(path, 'r')\n\
|
||||||
|
print(json.dumps([[f[x].name for x in f.attrs['ref']],\n\
|
||||||
|
\x20 f.attrs['opaque'].tobytes().decode(),\n\
|
||||||
|
\x20 f.attrs.get_id('bits').get_type().get_order(),\n\
|
||||||
|
\x20 f.attrs['bits'].tolist()]))",
|
||||||
|
);
|
||||||
|
assert_eq!(out, r#"[["/", "/"], "abcdefgh", 1, [258, 772]]"#);
|
||||||
|
h5dump_ok(&path);
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user