quantumclaw/clawhdf5
3
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Fix silent wrong data and libhdf5 interop found by the HDF5 audit #11

Merged
osobh merged 41 commits from fix/phase0-correctness into main 2026-09-26 02:42:54 +00:00
Conversation 0 Commits 41 Files Changed 50
+239 -33
4 changed files with 239 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit b36998ef01 - Show all commits
crates/clawhdf5-format/src/file_writer.rs
+26 -19
View File
@@ -33,6 +33,11 @@ pub(crate) const OFFSET_SIZE: u8 = 8;
pub(crate) const LENGTH_SIZE: u8 = 8;
const SUPERBLOCK_SIZE: usize = 48;
/// Largest raw data a compact dataset can hold: the layout message (version,
/// class, 2-byte size, data) must fit an object header message, whose size
/// field is 2 bytes. Bigger "compact" requests fall back to contiguous storage.
const MAX_COMPACT_DATA_SIZE: usize = crate::object_header_writer::MAX_MESSAGE_SIZE - 4;
/// Threshold for switching from compact (inline) to dense attribute storage.
const DENSE_ATTR_THRESHOLD: usize = 8;
@@ -51,7 +56,7 @@ pub(crate) fn build_chunked_dataset_oh(
attrs: &[AttributeMessage],
dense_blob: Option<&DenseAttrBlob>,
fill_time: FillTime,
) -> Vec<u8> {
) -> Result<Vec<u8>, FormatError> {
let mut w = ObjectHeaderWriter::new();
w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01);
w.add_message(MessageType::Dataspace, ds.serialize(LENGTH_SIZE));
@@ -78,7 +83,7 @@ pub(crate) fn build_dataset_oh(
attrs: &[AttributeMessage],
dense_blob: Option<&DenseAttrBlob>,
fill_time: FillTime,
) -> Vec<u8> {
) -> Result<Vec<u8>, FormatError> {
let mut w = ObjectHeaderWriter::new();
w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01);
w.add_message(MessageType::Dataspace, ds.serialize(LENGTH_SIZE));
@@ -113,7 +118,7 @@ pub(crate) fn build_compact_dataset_oh(
attrs: &[AttributeMessage],
dense_blob: Option<&DenseAttrBlob>,
fill_time: FillTime,
) -> Vec<u8> {
) -> Result<Vec<u8>, FormatError> {
let mut w = ObjectHeaderWriter::new();
w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01);
w.add_message(MessageType::Dataspace, ds.serialize(LENGTH_SIZE));
@@ -140,7 +145,7 @@ pub(crate) fn build_group_oh(
dense_link_info: Option<&[u8]>,
attrs: &[AttributeMessage],
dense_blob: Option<&DenseAttrBlob>,
) -> Vec<u8> {
) -> Result<Vec<u8>, FormatError> {
let mut w = ObjectHeaderWriter::new();
if let Some(li) = dense_link_info {
// Dense link storage: a LinkInfo pointing at the fractal heap + name
@@ -903,7 +908,7 @@ pub(crate) fn build_vds_dataset_oh(
attrs: &[AttributeMessage],
dense_blob: Option<&DenseAttrBlob>,
fill_time: FillTime,
) -> Vec<u8> {
) -> Result<Vec<u8>, FormatError> {
let mut w = ObjectHeaderWriter::new();
w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01);
w.add_message(MessageType::Dataspace, ds.serialize(LENGTH_SIZE));
@@ -1130,7 +1135,9 @@ impl FileWriter {
let is_compact: Vec<bool> = all_ds
.iter()
.enumerate()
.map(|(i, d)| !is_vds[i] && !is_chunked[i] && d.compact && d.raw.len() <= 65535)
.map(|(i, d)| {
!is_vds[i] && !is_chunked[i] && d.compact && d.raw.len() <= MAX_COMPACT_DATA_SIZE
})
.collect();
let root_dense = root_attrs.len() > DENSE_ATTR_THRESHOLD;
let group_dense: Vec<bool> = groups
@@ -1169,9 +1176,9 @@ impl FileWriter {
}
let attr_blob = group_dense[gi].then(|| build_dense_attrs(&g.attrs, 0));
let dl = group_links_dense[gi].then_some(dummy_link_info.as_slice());
build_group_oh(&dummy_links, dl, &g.attrs, attr_blob.as_ref()).len()
build_group_oh(&dummy_links, dl, &g.attrs, attr_blob.as_ref()).map(|oh| oh.len())
})
.collect();
.collect::<Result<_, _>>()?;
let root_dummy_links: Vec<LinkMessage> = {
let mut links = Vec::new();
@@ -1186,7 +1193,7 @@ impl FileWriter {
let root_oh_size = {
let attr_blob = root_dense.then(|| build_dense_attrs(&root_attrs, 0));
let dl = root_links_dense.then_some(dummy_link_info.as_slice());
build_group_oh(&root_dummy_links, dl, &root_attrs, attr_blob.as_ref()).len()
build_group_oh(&root_dummy_links, dl, &root_attrs, attr_blob.as_ref())?.len()
};
struct DataBlob {
@@ -1215,7 +1222,7 @@ impl FileWriter {
&d.attrs,
dense_blob.as_ref(),
d.fill_time,
);
)?;
// Global heap blob size is address-independent; compute it now
// so pass 2 can place it correctly.
let vds_mappings = d.virtual_sources.as_deref().unwrap_or(&[]);
@@ -1259,7 +1266,7 @@ impl FileWriter {
&d.attrs,
dense_blob.as_ref(),
d.fill_time,
);
)?;
dummy_blobs.push(DataBlob {
data: result.data_bytes,
oh_bytes: oh,
@@ -1278,7 +1285,7 @@ impl FileWriter {
&d.attrs,
dense_blob.as_ref(),
d.fill_time,
);
)?;
dummy_blobs.push(DataBlob {
data: vec![],
oh_bytes: oh,
@@ -1298,7 +1305,7 @@ impl FileWriter {
&d.attrs,
dense_blob.as_ref(),
d.fill_time,
);
)?;
dummy_blobs.push(DataBlob {
data: d.raw.clone(),
oh_bytes: oh,
@@ -1407,7 +1414,7 @@ impl FileWriter {
&d.attrs,
ds_dense_blobs[i].as_ref(),
d.fill_time,
);
)?;
ds_blobs2.push(DataBlob {
data: gcol_bytes.clone(),
oh_bytes: oh,
@@ -1434,7 +1441,7 @@ impl FileWriter {
&d.attrs,
ds_dense_blobs[i].as_ref(),
d.fill_time,
);
)?;
ds_blobs2.push(DataBlob {
data: result.data_bytes,
oh_bytes: oh,
@@ -1449,7 +1456,7 @@ impl FileWriter {
&d.attrs,
ds_dense_blobs[i].as_ref(),
d.fill_time,
);
)?;
ds_blobs2.push(DataBlob {
data: vec![],
oh_bytes: oh,
@@ -1474,7 +1481,7 @@ impl FileWriter {
&d.attrs,
ds_dense_blobs[i].as_ref(),
d.fill_time,
);
)?;
let mut data = vec![0u8; padding];
data.extend_from_slice(&d.raw);
cursor2 += d.raw.len();
@@ -1530,7 +1537,7 @@ impl FileWriter {
root_dl,
&root_attrs,
root_dense_blob.as_ref(),
));
)?);
if let Some(ref b) = root_link_blob {
buf.extend_from_slice(&b.blob);
}
@@ -1555,7 +1562,7 @@ impl FileWriter {
dl,
&g.attrs,
group_dense_blobs[gi].as_ref(),
));
)?);
if let Some(ref b) = link_blob {
buf.extend_from_slice(&b.blob);
}
crates/clawhdf5-format/src/object_header_writer.rs
+59 -13
View File
@@ -1,11 +1,17 @@
//! Object header writer for v2 format.
#[cfg(not(feature = "std"))]
use alloc::vec::Vec;
use alloc::{format, vec::Vec};
use crate::checksum::jenkins_lookup3;
use crate::error::FormatError;
use crate::message_type::MessageType;
/// Largest message payload a v2 object header can describe: the per-message
/// size field is 2 bytes. A bigger message cannot be encoded at all — writing
/// its size truncated to 16 bits produced files libhdf5 refuses.
pub const MAX_MESSAGE_SIZE: usize = u16::MAX as usize;
/// Writer for v2 object headers with proper checksums.
pub struct ObjectHeaderWriter {
messages: Vec<(MessageType, Vec<u8>, u8)>, // (type, data, msg_flags)
@@ -30,7 +36,22 @@ impl ObjectHeaderWriter {
}
/// Serialize the complete v2 object header (OHDR + messages + checksum).
pub fn serialize(&self) -> Vec<u8> {
///
/// Fails with [`FormatError::SerializationError`] when a message is larger
/// than [`MAX_MESSAGE_SIZE`] (e.g. an attribute over ~64 KiB, which would
/// need dense attribute storage), rather than writing a corrupt header.
pub fn serialize(&self) -> Result<Vec<u8>, FormatError> {
if let Some((msg_type, data, _)) = self
.messages
.iter()
.find(|(_, data, _)| data.len() > MAX_MESSAGE_SIZE)
{
return Err(FormatError::SerializationError(format!(
"{msg_type:?} message is {} bytes; an object header message holds at most \
{MAX_MESSAGE_SIZE} bytes",
data.len()
)));
}
// Calculate total message bytes: each message has type(1) + size(2) + flags(1) + data
let msg_bytes_total: usize = self
.messages
@@ -80,7 +101,7 @@ impl ObjectHeaderWriter {
let checksum = jenkins_lookup3(&buf);
buf.extend_from_slice(&checksum.to_le_bytes());
buf
Ok(buf)
}
}
@@ -125,15 +146,22 @@ impl BatchObjectHeaderWriter {
/// Compute the serialized size of each header without actually serializing.
/// Returns sizes in the same order as headers were added.
pub fn compute_sizes(&self) -> Vec<usize> {
self.headers.iter().map(|h| h.serialize().len()).collect()
pub fn compute_sizes(&self) -> Result<Vec<usize>, FormatError> {
self.headers
.iter()
.map(|h| h.serialize().map(|b| b.len()))
.collect()
}
/// Serialize all headers into a single contiguous buffer.
/// Returns `(combined_bytes, offsets)` where `offsets[i]` is the byte
/// offset of header `i` within the combined buffer.
pub fn serialize_all(&self) -> (Vec<u8>, Vec<usize>) {
let serialized: Vec<Vec<u8>> = self.headers.iter().map(|h| h.serialize()).collect();
pub fn serialize_all(&self) -> Result<(Vec<u8>, Vec<usize>), FormatError> {
let serialized: Vec<Vec<u8>> = self
.headers
.iter()
.map(|h| h.serialize())
.collect::<Result<_, _>>()?;
let total: usize = serialized.iter().map(|s| s.len()).sum();
let mut buf = Vec::with_capacity(total);
let mut offsets = Vec::with_capacity(serialized.len());
@@ -141,7 +169,7 @@ impl BatchObjectHeaderWriter {
offsets.push(buf.len());
buf.extend_from_slice(s);
}
(buf, offsets)
Ok((buf, offsets))
}
}
@@ -159,7 +187,7 @@ mod tests {
#[test]
fn empty_header_roundtrip() {
let writer = ObjectHeaderWriter::new();
let bytes = writer.serialize();
let bytes = writer.serialize().unwrap();
let hdr = ObjectHeader::parse(&bytes, 0, 8, 8).unwrap();
assert_eq!(hdr.version, 2);
assert_eq!(hdr.messages.len(), 0);
@@ -170,7 +198,7 @@ mod tests {
let mut writer = ObjectHeaderWriter::new();
writer.add_message(MessageType::Dataspace, vec![1, 2, 3, 4]);
writer.add_message(MessageType::Datatype, vec![5, 6]);
let bytes = writer.serialize();
let bytes = writer.serialize().unwrap();
let hdr = ObjectHeader::parse(&bytes, 0, 8, 8).unwrap();
assert_eq!(hdr.messages.len(), 2);
assert_eq!(hdr.messages[0].msg_type, MessageType::Dataspace);
@@ -184,12 +212,30 @@ mod tests {
let mut writer = ObjectHeaderWriter::new();
// Add a message with >255 bytes of payload
writer.add_message(MessageType::Datatype, vec![0xAA; 300]);
let bytes = writer.serialize();
let bytes = writer.serialize().unwrap();
let hdr = ObjectHeader::parse(&bytes, 0, 8, 8).unwrap();
assert_eq!(hdr.messages.len(), 1);
assert_eq!(hdr.messages[0].data.len(), 300);
}
#[test]
fn oversized_message_is_an_error_not_a_truncated_size() {
// 65535 bytes is the largest encodable payload.
let mut writer = ObjectHeaderWriter::new();
writer.add_message(MessageType::Attribute, vec![0; MAX_MESSAGE_SIZE]);
let bytes = writer.serialize().unwrap();
let hdr = ObjectHeader::parse(&bytes, 0, 8, 8).unwrap();
assert_eq!(hdr.messages[0].data.len(), MAX_MESSAGE_SIZE);
// One byte more used to be written with its size wrapped to 0.
let mut writer = ObjectHeaderWriter::new();
writer.add_message(MessageType::Attribute, vec![0; MAX_MESSAGE_SIZE + 1]);
assert!(matches!(
writer.serialize(),
Err(FormatError::SerializationError(_))
));
}
#[test]
fn batch_writer_serialize_all() {
let mut batch = BatchObjectHeaderWriter::new();
@@ -204,7 +250,7 @@ mod tests {
batch.add(w2);
assert_eq!(batch.len(), 2);
let (buf, offsets) = batch.serialize_all();
let (buf, offsets) = batch.serialize_all().unwrap();
assert_eq!(offsets.len(), 2);
assert_eq!(offsets[0], 0);
@@ -222,7 +268,7 @@ mod tests {
fn batch_writer_empty() {
let batch = BatchObjectHeaderWriter::new();
assert!(batch.is_empty());
let (buf, offsets) = batch.serialize_all();
let (buf, offsets) = batch.serialize_all().unwrap();
assert!(buf.is_empty());
assert!(offsets.is_empty());
}
crates/clawhdf5-format/src/type_builders.rs
+1 -1
View File
@@ -718,7 +718,7 @@ impl DatasetBuilder {
/// Use compact (inline) storage for this dataset.
///
/// The raw data is stored directly in the dataset's object header rather
/// than as a separate data blob. Only effective when raw data <= 65536 bytes
/// than as a separate data blob. Only effective when raw data <= 65531 bytes
/// and the dataset is not chunked.
pub fn compact(&mut self) -> &mut Self {
self.compact = true;
crates/clawhdf5-format/tests/writer_meta_tests.rs
+153
View File
@@ -0,0 +1,153 @@
//! Regression tests for writer metadata bugs that produced files libhdf5
//! refuses (or reads differently from us), plus the reader-side counterparts.
//!
//! The plain tests check the bytes we write with our own parser. The
//! `#[ignore]`d ones are the interop half: they open what we write in h5py
//! (`CLAWHDF5_PYTHON`, as in `writer_h5py_tests.rs`) and run `h5dump` over it.
use clawhdf5_format::data_layout::DataLayout;
use clawhdf5_format::file_writer::{AttrValue, FileWriter};
use clawhdf5_format::group_v2::resolve_path_any;
use clawhdf5_format::message_type::MessageType;
use clawhdf5_format::object_header::ObjectHeader;
use clawhdf5_format::signature;
use clawhdf5_format::superblock::Superblock;
use clawhdf5_format::type_builders::make_u8_type;
// ---- helpers ----
fn header_at(bytes: &[u8], path: &str) -> (Superblock, ObjectHeader) {
let sig = signature::find_signature(bytes).unwrap();
let sb = Superblock::parse(bytes, sig).unwrap();
let addr = if path == "/" {
sb.root_group_address
} else {
resolve_path_any(bytes, &sb, path).unwrap()
};
let oh = ObjectHeader::parse(bytes, addr as usize, sb.offset_size, sb.length_size).unwrap();
(sb, oh)
}
fn layout_of(bytes: &[u8], path: &str) -> DataLayout {
let (sb, oh) = header_at(bytes, path);
let msg = oh
.messages
.iter()
.find(|m| m.msg_type == MessageType::DataLayout)
.unwrap();
DataLayout::parse(&msg.data, sb.offset_size, sb.length_size).unwrap()
}
fn python() -> String {
std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string())
}
fn write_tmp(name: &str, bytes: &[u8]) -> std::path::PathBuf {
let path = std::env::temp_dir().join(format!("clawhdf5_writer_meta_{name}.h5"));
std::fs::write(&path, bytes).unwrap();
path
}
/// Run `script` (with `path` bound to the file) under h5py; return stdout.
fn h5py(path: &std::path::Path, script: &str) -> String {
let full = format!(
"import h5py, numpy as np, json\npath = {:?}\n{script}",
path.display().to_string()
);
let o = std::process::Command::new(python())
.args(["-c", &full])
.output()
.expect("python interpreter");
assert!(
o.status.success(),
"h5py failed: {}",
String::from_utf8_lossy(&o.stderr)
);
String::from_utf8(o.stdout).unwrap().trim().to_string()
}
/// `h5dump` must read the whole file without error.
fn h5dump_ok(path: &std::path::Path) {
let o = std::process::Command::new("h5dump")
.arg(path)
.output()
.expect("h5dump");
assert!(
o.status.success(),
"h5dump failed: {}{}",
String::from_utf8_lossy(&o.stdout),
String::from_utf8_lossy(&o.stderr)
);
}
fn u8_ramp(n: usize) -> Vec<u8> {
(0..n).map(|i| (i % 251) as u8).collect()
}
// ---- 1. object header message size limit ----
#[test]
fn attribute_too_big_for_a_header_message_is_an_error() {
// Measured: a 70000-byte attribute was written with its message size
// wrapped to 16 bits, and libhdf5 refused the whole root group.
let mut fw = FileWriter::new();
fw.set_root_attr(
"a",
AttrValue::Raw {
datatype: make_u8_type(),
shape: vec![70_000],
data: u8_ramp(70_000),
},
);
assert!(fw.finish().is_err());
// 65500 bytes still fits and still works.
let mut fw = FileWriter::new();
fw.set_root_attr(
"a",
AttrValue::Raw {
datatype: make_u8_type(),
shape: vec![65_500],
data: u8_ramp(65_500),
},
);
let bytes = fw.finish().unwrap();
let (sb, oh) = header_at(&bytes, "/");
let attrs = clawhdf5_format::attribute::extract_attributes(&oh, sb.length_size).unwrap();
assert_eq!(attrs[0].raw_data, u8_ramp(65_500));
}
#[test]
fn compact_layout_falls_back_to_contiguous_past_the_message_limit() {
// Layout message = 4 bytes + data; data may be at most 65531 bytes.
for (n, compact) in [(65_531, true), (65_532, false), (65_534, false)] {
let mut fw = FileWriter::new();
fw.create_dataset("d").with_u8_data(&u8_ramp(n)).compact();
let bytes = fw.finish().unwrap();
match layout_of(&bytes, "d") {
DataLayout::Compact { data } => {
assert!(compact, "{n} bytes must not be compact");
assert_eq!(data, u8_ramp(n));
}
DataLayout::Contiguous { .. } => assert!(!compact, "{n} bytes should be compact"),
other => panic!("unexpected layout {other:?}"),
}
}
}
#[test]
#[ignore = "requires Python h5py module and h5dump"]
fn h5py_reads_compact_datasets_at_the_limit() {
for n in [65_531usize, 65_534] {
let mut fw = FileWriter::new();
fw.create_dataset("d").with_u8_data(&u8_ramp(n)).compact();
let path = write_tmp(&format!("compact_{n}"), &fw.finish().unwrap());
let out = h5py(
&path,
"f = h5py.File(path, 'r'); v = f['d'][()]\n\
print(bool((v == (np.arange(v.size) % 251).astype(np.uint8)).all()), v.size)",
);
assert_eq!(out, format!("True {n}"));
h5dump_ok(&path);
}
}