diff --git a/CHANGELOG.md b/CHANGELOG.md index 847f3c6..fc22849 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,22 @@ object running past its collection. Conformance unchanged at 575 of 697 (`crates/clawhdf5-format/tests/vl_heap_bounds.rs`). +- **Every reader resolves VL data the same way.** `h5rs` (`dump`, `ls`, + `diff`, `check --data`) had its own lenient VL decoder: a heap object + longer than the element's length was cut to it (h5py refuses it), a null + string printed `""` where h5dump prints `NULL`, the stored element size + was trusted, and each heap collection was kept as a copy for the whole + run. It now resolves through `VlResolver`, so `dump` matches h5dump byte + for byte on VL strings (`"a\0b"` as `"a"`, null as `NULL`), VL sequences + and 4-byte-offset files, `dump --json` gives h5py's values, and + `check --data` reports any heap object whose size is not exactly the + element's length × base size. `clawhdf5-wasm` already resolved VL strings + with `read_vl_strings`; it now uses `VlResolver` and refuses a VL type + whose stored element size disagrees with the file, as `File` does + (`crates/clawhdf5-tools/tests/h5rs_interop.rs`, + `crates/clawhdf5-wasm/tests/vl_strings.rs`). New + `VlResolver::element` / `string_element` resolve one element in place. + ### Plugin filters (2026-09-26) - **LZF, bitshuffle, bzip2 and Blosc read and write, in pure Rust.** Files written by h5py with `compression="lzf"`, or with hdf5plugin's @@ -264,10 +280,10 @@ printed with its address; exit 1 when there are any. libhdf5's h5check reads only the 1.8 format. On the conformance corpus it passes all 418 files that both clawhdf5 and h5py read in full, and `check --data` flags - 134 of the 150 CVE and fuzzer files of the `cve_hdf5` corpus (tank, + 135 of the 150 CVE and fuzzer files of the `cve_hdf5` corpus (tank, 2026-09-26). `--data` also follows variable-length data into its global heap collections and reports a damaged one at its address. It inherits - the library's tolerance, though: 9 of the 16 it passes are files h5dump + the library's tolerance, though: 8 of the 15 it passes are files h5dump 1.14.6 rejects (see `docs/known-issues.md`, header checks). - Values over `--max-bytes` (default 1 GiB) are reported instead of read; a panic is caught and reported as an internal error (exit 3). diff --git a/crates/clawhdf5-tools/README.md b/crates/clawhdf5-tools/README.md index 79f775d..8f089c3 100644 --- a/crates/clawhdf5-tools/README.md +++ b/crates/clawhdf5-tools/README.md @@ -76,8 +76,13 @@ print the same bytes as h5dump 1.14.6 and as Debian's h5dump 1.14.5 (the was run in that image on 2026-09-26) — `dump_matches_h5dump` in `tests/h5rs_interop.rs` checks this, and `dump_shows_nul_padding_in_nested_strings` that null-padded strings show their NULs (`"a\000b"`) at any depth, as -h5dump's do. Not covered by those tests: references, opaque, bitfield, -variable-length sequences and virtual datasets. Known differences from +h5dump's do. `dump_prints_vl_data_like_h5dump` covers variable-length +strings (one with an embedded NUL, which prints up to the NUL; empty; null, +which prints `NULL`), variable-length sequences, a VL compound member and +a VL attribute, with 8- and 4-byte offsets. Not covered by those tests: +references, opaque, bitfield, non-ASCII UTF-8 (h5dump prints each byte +above 0x7f as a sign-extended octal escape, h5rs the character) and +virtual datasets. Known differences from h5dump: - Floats print at their own precision (a `float32` 0.1 prints as `0.1`), @@ -233,9 +238,11 @@ extension) and checks: (which catches corrupt compressed data and Fletcher-32 mismatches), and follows every variable-length element (strings and sequences, also inside compounds and arrays) of every dataset and attribute into its global heap -collection: a collection that does not parse, a missing heap object, or a -sequence longer than its heap object is a problem at the collection's -address. Data the +collection: a collection that does not parse or overlaps another, a missing +heap object, or a heap object whose size is not exactly the element's +length times its base size (libhdf5 refuses such an element) is a problem +at the collection's address. Variable-length elements are resolved by the +library's `VlResolver`, as `clawhdf5::File` resolves them. Data the tool cannot decode (a filter it does not implement, such as szip, or a dataset over `--max-bytes`) is a `note:`, not a problem. Every problem is printed with the address of the structure involved; the exit status is 0 @@ -252,9 +259,10 @@ none at all without `--data`), and objects reachable only by external links. It clawhdf5's parsers, so it accepts what they accept: some header damage that libhdf5 refuses goes unreported. Of the 150 CVE and fuzzer files of the HDF Group's `cve_hdf5` corpus (`cvefiles/` and `fuzzerfiles/`), -`check --data` passes 16, and h5dump 1.14.6 rejects 9 of those (tank, +`check --data` passes 15, and h5dump 1.14.6 rejects 8 of those (tank, 2026-09-26, `h5rs check --data F` and `h5dump F` per file; before the -library's header checks it passed 28, of which h5dump rejects 21). +library's header checks it passed 28, of which h5dump rejects 21, and 16 +and 9 before a VL type's stored element size was checked). ## Robustness diff --git a/crates/clawhdf5-tools/src/check.rs b/crates/clawhdf5-tools/src/check.rs index 8c56771..da90247 100644 --- a/crates/clawhdf5-tools/src/check.rs +++ b/crates/clawhdf5-tools/src/check.rs @@ -15,11 +15,13 @@ use clawhdf5_format::btree_v2::{BTreeV2Header, collect_btree_v2_records}; use clawhdf5_format::data_layout::DataLayout; use clawhdf5_format::dataspace::{Dataspace, DataspaceType}; use clawhdf5_format::datatype::Datatype; +use clawhdf5_format::error::FormatError; use clawhdf5_format::group_info::GroupInfoMessage; use clawhdf5_format::link_info::LinkInfoMessage; use clawhdf5_format::message_type::MessageType; use clawhdf5_format::object_header::ObjectHeader; use clawhdf5_format::symbol_table::SymbolTableMessage; +use clawhdf5_format::vl_data::{VlResolver, check_element_size, parse_vl_references}; use crate::cli::{Args, Out}; use crate::h5::{Error, ErrorKind, H5, Kind}; @@ -49,6 +51,15 @@ found, 3 internal error."; const MAX_CHUNKS_CHECKED: usize = 10_000_000; +/// A variable-length element's problem, worded as `check` reports heap +/// problems ("global heap ..."). +fn heap_problem(e: FormatError) -> String { + match e { + FormatError::VlDataError(m) if m.starts_with("global heap") => m, + e => format!("global heap: {e}"), + } +} + /// Whether values of `dt` hold variable-length data (in the global heap). fn has_vl(dt: &Datatype, depth: u32) -> bool { if depth > 32 { @@ -104,6 +115,8 @@ struct Checker<'a> { btrees_seen: HashSet, /// Global heap collections already read (with --data). gcols_seen: HashSet, + /// Resolves variable-length elements (with --data), for the whole file. + vl: VlResolver<'a>, panicked: bool, } @@ -157,6 +170,7 @@ pub fn run(args: &mut Args, out: &mut Out) -> std::io::Result { heaps_seen: HashSet::new(), btrees_seen: HashSet::new(), gcols_seen: HashSet::new(), + vl: VlResolver::new(h5.data(), h5.os(), h5.ls()), panicked: false, }; c.superblock(); @@ -707,62 +721,48 @@ impl Checker<'_> { } match dt { Datatype::VariableLength { + size, is_string, base_type, .. } => { - let os = usize::from(self.h5.os()); - let (Some(lenb), Some(addrb), Some(idxb)) = - (b.get(..4), b.get(4..4 + os), b.get(4 + os..8 + os)) - else { + // Resolved by the library's VlResolver, as every other + // reader resolves them (and as libhdf5 does): a heap object + // whose size is not the element's length × base size, a + // collection that overlaps another, or a missing object is + // a problem at the collection's address. + let Ok(vl) = parse_vl_references(b, 1, self.h5.os()) else { return; }; - let le = |x: &[u8]| { - x.iter() - .enumerate() - .fold(0u64, |a, (i, &v)| a | (u64::from(v) << (8 * i))) - }; - let (len, gcol, idx) = (le(lenb), le(addrb), le(idxb)); - let undef = if os >= 8 { - u64::MAX - } else { - (1u64 << (8 * os)) - 1 - }; - if len == 0 || gcol == 0 || gcol == undef || bad.contains_key(&gcol) { + let gcol = vl[0].collection_address; + if gcol == 0 || bad.contains_key(&gcol) { return; } - let obj = match self.h5.heap_object(gcol, idx as u32) { - Ok(o) => o, + if let Err(e) = check_element_size(*size, self.h5.os()) { + bad.insert(gcol, e.to_string()); + return; + } + let bs = if *is_string { + 1 + } else { + base_type.type_size() as usize + }; + if bs == 0 { + return; + } + let obj = match self.vl.element(b, bs) { + Ok(o) => o.unwrap_or(&[]), Err(e) => { - bad.insert(e.addr.unwrap_or(gcol), e.msg); + bad.insert(gcol, heap_problem(e)); return; } }; if self.gcols_seen.insert(gcol) { self.counts.global_heaps += 1; } - let bs = if *is_string { - 1 - } else { - u64::from(base_type.type_size()) - }; - if len - .checked_mul(bs) - .is_none_or(|need| need > obj.len() as u64) - { - bad.insert( - gcol, - format!( - "global heap object {idx} holds {} bytes; the element needs {len} x {bs}", - obj.len() - ), - ); - return; - } - if !*is_string && bs > 0 && has_vl(base_type, depth + 1) { - let bs = bs as usize; - for k in 0..len as usize { - self.vl_element(base_type, &obj[k * bs..(k + 1) * bs], depth + 1, bad); + if !*is_string && has_vl(base_type, depth + 1) { + for eb in obj.chunks_exact(bs) { + self.vl_element(base_type, eb, depth + 1, bad); } } } diff --git a/crates/clawhdf5-tools/src/diff.rs b/crates/clawhdf5-tools/src/diff.rs index c7ad723..98d4ba5 100644 --- a/crates/clawhdf5-tools/src/diff.rs +++ b/crates/clawhdf5-tools/src/diff.rs @@ -699,6 +699,9 @@ impl Diff { } match (x, y) { (Value::Str(p), Value::Str(q)) => p == q, + // h5diff compares a null VL string equal to an empty one. + (Value::NullStr, Value::NullStr) => true, + (Value::NullStr, Value::Str(s)) | (Value::Str(s), Value::NullStr) => s.is_empty(), (Value::Bytes(p), Value::Bytes(q)) | (Value::OtherRef(p), Value::OtherRef(q)) => p == q, (Value::Compound(p), Value::Compound(q)) => { p.len() == q.len() diff --git a/crates/clawhdf5-tools/src/h5.rs b/crates/clawhdf5-tools/src/h5.rs index e2e7931..1f4354a 100644 --- a/crates/clawhdf5-tools/src/h5.rs +++ b/crates/clawhdf5-tools/src/h5.rs @@ -8,7 +8,6 @@ use std::cell::RefCell; use std::collections::HashMap; use std::path::{Path, PathBuf}; -use std::rc::Rc; use clawhdf5::File; use clawhdf5_format::attribute::{AttributeMessage, extract_attributes_tolerant}; @@ -20,7 +19,6 @@ use clawhdf5_format::datatype::Datatype; use clawhdf5_format::error::FormatError; use clawhdf5_format::filter_pipeline::FilterPipeline; use clawhdf5_format::fractal_heap::FractalHeapHeader; -use clawhdf5_format::global_heap::GlobalHeapCollection; use clawhdf5_format::group_v1; use clawhdf5_format::link_info::LinkInfoMessage; use clawhdf5_format::link_message::{LinkMessage, LinkTarget}; @@ -191,7 +189,6 @@ pub struct H5 { pub path: PathBuf, pub file: File, pub max_bytes: u64, - heaps: RefCell, String>>>, /// Fractal heaps whose blocks were verified: `None` = sound. verified_heaps: RefCell>>, } @@ -211,7 +208,6 @@ impl H5 { path: path.to_path_buf(), file, max_bytes: DEFAULT_MAX_BYTES, - heaps: RefCell::new(HashMap::new()), verified_heaps: RefCell::new(HashMap::new()), }) } @@ -433,29 +429,6 @@ impl H5 { r.map_or(Ok(()), Err) } - /// The global heap object `idx` of the collection at `addr` (cached per - /// collection). - pub fn heap_object(&self, addr: u64, idx: u32) -> Result> { - let coll = { - let mut cache = self.heaps.borrow_mut(); - cache - .entry(addr) - .or_insert_with(|| match usize::try_from(addr) { - Ok(a) => GlobalHeapCollection::parse(self.data(), a, self.ls()) - .map(Rc::new) - .map_err(|e| e.to_string()), - Err(_) => Err("address out of range".into()), - }) - .clone() - .map_err(|e| Error::at(addr, format!("global heap: {e}")))? - }; - let idx16 = u16::try_from(idx) - .map_err(|_| Error::at(addr, format!("global heap object index {idx} out of range")))?; - coll.get_object(idx16) - .map(|o| o.data.clone()) - .ok_or_else(|| Error::at(addr, format!("global heap has no object {idx}"))) - } - /// The dataspace of the dataset at `path` with a virtual dataset's /// extent resolved from its sources (as libhdf5 reports it) instead of /// the stored one. diff --git a/crates/clawhdf5-tools/src/value.rs b/crates/clawhdf5-tools/src/value.rs index 8160574..d9255f1 100644 --- a/crates/clawhdf5-tools/src/value.rs +++ b/crates/clawhdf5-tools/src/value.rs @@ -3,7 +3,10 @@ //! Decoding never panics: a short buffer, an unknown byte order or a //! dangling heap reference becomes [`Value::Error`]. +use std::cell::RefCell; + use clawhdf5_format::datatype::{Datatype, DatatypeByteOrder, ReferenceType, StringPadding}; +use clawhdf5_format::vl_data::{VlResolver, check_element_size}; use serde_json::Value as J; use crate::dtype; @@ -16,6 +19,9 @@ pub enum Value { /// its own precision. Float(f64, u8), Str(String), + /// A null variable-length string (heap address 0): h5dump prints it as + /// `NULL`, h5py reads it as empty. + NullStr, /// Opaque, bitfield, time and oversized integers. Bytes(Vec), /// An enum member (name, when the value matches one) and its value. @@ -129,10 +135,10 @@ fn decode_float(dt: &Datatype, b: &[u8]) -> Value { } } -fn trim_string(b: &[u8], pad: Option<&StringPadding>) -> String { +fn trim_string(b: &[u8], pad: &StringPadding) -> String { let cut = b.iter().position(|&c| c == 0).unwrap_or(b.len()); let mut s = &b[..cut]; - if matches!(pad, Some(StringPadding::SpacePad)) { + if matches!(pad, StringPadding::SpacePad) { while let [rest @ .., b' '] = s { s = rest; } @@ -140,22 +146,20 @@ fn trim_string(b: &[u8], pad: Option<&StringPadding>) -> String { String::from_utf8_lossy(s).into_owned() } -/// Little-endian unsigned integer of `b` (up to 8 bytes). -fn le(b: &[u8]) -> u64 { - b.iter() - .take(8) - .enumerate() - .fold(0u64, |a, (i, &x)| a | (u64::from(x) << (8 * i))) -} - /// Decodes elements of one file. pub struct Decoder<'a> { pub h5: &'a H5, + /// Variable-length elements are resolved as the library resolves them + /// (so as libhdf5 does), not by a decoder of our own. + vl: RefCell>, } impl<'a> Decoder<'a> { pub fn new(h5: &'a H5) -> Self { - Self { h5 } + Self { + h5, + vl: RefCell::new(VlResolver::new(h5.data(), h5.os(), h5.ls())), + } } /// Decode element `i` of `raw`, an array of `dt` elements. @@ -187,7 +191,7 @@ impl<'a> Decoder<'a> { Datatype::Time { .. } | Datatype::BitField { .. } | Datatype::Opaque { .. } => { Value::Bytes(b.to_vec()) } - Datatype::String { padding, .. } => Value::Str(trim_string(b, Some(padding))), + Datatype::String { padding, .. } => Value::Str(trim_string(b, padding)), Datatype::Compound { members, .. } => { let mut out = Vec::with_capacity(members.len()); for m in members { @@ -246,61 +250,43 @@ impl<'a> Decoder<'a> { Value::Array(out) } Datatype::VariableLength { + size, is_string, - padding, base_type, .. - } => self.decode_vlen(*is_string, padding.as_ref(), base_type, b, depth), + } => match check_element_size(*size, self.h5.os()) { + Ok(()) => self.decode_vlen(*is_string, base_type, b, depth), + Err(e) => Value::Error(e.to_string()), + }, } } - fn decode_vlen( - &self, - is_string: bool, - padding: Option<&StringPadding>, - base: &Datatype, - b: &[u8], - depth: u32, - ) -> Value { - let os = usize::from(self.h5.os()); - let (Some(lenb), Some(addrb), Some(idxb)) = - (b.get(..4), b.get(4..4 + os), b.get(4 + os..8 + os)) - else { - return Value::Error("short VL element".into()); - }; - let len = le(lenb) as usize; - let addr = le(addrb); - let idx = le(idxb) as u32; - let undef = if os >= 8 { - u64::MAX - } else { - (1u64 << (8 * os)) - 1 - }; - let obj = if len == 0 || addr == 0 || addr == undef { - Vec::new() - } else { - match self.h5.heap_object(addr, idx) { - Ok(o) => o, - Err(e) => return Value::Error(e.to_string()), - } - }; + /// A variable-length element, resolved by the library's + /// [`VlResolver`]: a string ends at its first NUL, a heap object whose + /// size is not the element's length × base size is an error, and a + /// heap address of 0 is null — all as libhdf5 (and so h5dump and h5py) + /// has it. + fn decode_vlen(&self, is_string: bool, base: &Datatype, b: &[u8], depth: u32) -> Value { if is_string { - let l = len.min(obj.len()); - return Value::Str(trim_string(&obj[..l], padding)); + return match self.vl.borrow_mut().string_element(b) { + Ok(Some(s)) => Value::Str(String::from_utf8_lossy(s).into_owned()), + Ok(None) => Value::NullStr, + Err(e) => Value::Error(e.to_string()), + }; } let bs = base.type_size() as usize; if bs == 0 { return Value::Error("VL base type of size 0".into()); } - match len.checked_mul(bs) { - Some(need) if need <= obj.len() => {} - _ => return Value::Error("VL sequence longer than its heap object".into()), - } - let mut out = Vec::with_capacity(len); - for k in 0..len { - out.push(self.decode(base, &obj[k * bs..], depth + 1)); - } - Value::Seq(out) + let obj = match self.vl.borrow_mut().element(b, bs) { + Ok(o) => o.unwrap_or(&[]), + Err(e) => return Value::Error(e.to_string()), + }; + Value::Seq( + obj.chunks_exact(bs) + .map(|e| self.decode(base, e, depth + 1)) + .collect(), + ) } } @@ -351,6 +337,7 @@ pub fn text(v: &Value, h5paths: &dyn Fn(u64) -> Option) -> String { Value::Int(i) => i.to_string(), Value::Float(f, w) => fmt_float(*f, *w), Value::Str(s) => format!("\"{}\"", escape(s)), + Value::NullStr => "NULL".into(), Value::Bytes(b) => hex(b), Value::Enum(Some(n), _) => n.clone(), Value::Enum(None, i) => i.to_string(), @@ -411,6 +398,7 @@ pub fn to_json(v: &Value, h5paths: &dyn Fn(u64) -> Option) -> J { } } Value::Str(s) => J::from(s.as_str()), + Value::NullStr => J::from(""), Value::Bytes(b) | Value::OtherRef(b) => J::from(hex(b)), Value::Enum(_, i) => to_json(&Value::Int(*i), h5paths), Value::Compound(ms) => J::Array(ms.iter().map(|(_, v)| to_json(v, h5paths)).collect()), diff --git a/crates/clawhdf5-tools/tests/gen_vl_files.py b/crates/clawhdf5-tools/tests/gen_vl_files.py new file mode 100644 index 0000000..3937c19 --- /dev/null +++ b/crates/clawhdf5-tools/tests/gen_vl_files.py @@ -0,0 +1,121 @@ +"""Write the variable-length data files the h5rs VL tests run on. + +usage: gen_vl_files.py OUTDIR + +For 8-byte (`vl8`) and 4-byte (`vl4`) offsets, writes OUTDIR/vl8.h5 and +OUTDIR/vl4.h5, which libhdf5 reads in full, and OUTDIR/bad8.h5 and +OUTDIR/bad4.h5, whose `bad` and `badseq` elements 0 have a length that +disagrees with their global heap object (libhdf5: "Expected global heap +object size does not match"). h5py cannot write a VL string with a NUL in +it or a null element in a contiguous dataset, so those are patched in. + +Prints one JSON object: for each file, each dataset's values as h5py reads +them one element at a time (strings as text, sequences as lists, a compound +as a list of its fields), with null for an element h5py cannot read; the +root attribute `va`; and the addresses of the `bad` elements' collections. +""" + +import json +import os +import struct +import sys + +import h5py +import numpy as np + +out = sys.argv[1] +S = h5py.string_dtype("utf-8") +I4 = h5py.vlen_dtype(np.dtype(" null + open(path, "wb").write(bytes(b)) + + +def bad(path, sizes): + os_ = 8 if sizes is None else sizes[0] + with create(path, sizes) as f: + f.create_dataset("bad", data=np.array(["cdefgh", "ok"], dtype=object), dtype=S) + s = f.create_dataset("badseq", shape=(2,), dtype=I4) + s[0] = [1, 2, 3] + s[1] = [4] + off, soff = f["bad"].id.get_offset(), f["badseq"].id.get_offset() + b = bytearray(open(path, "rb").read()) + gcol = int.from_bytes(b[off + 4 : off + 4 + os_], "little") + struct.pack_into(" 3 + struct.pack_into(" 2 + open(path, "wb").write(bytes(b)) + return gcol + + +def value(v): + if isinstance(v, bytes): + return v.decode() + if isinstance(v, str): + return v + if isinstance(v, np.void): + return [value(x) for x in v] + if isinstance(v, np.ndarray): + return [value(x) for x in v] + return v.item() if hasattr(v, "item") else v + + +def read(ds): + got = [] + for i in range(ds.shape[0]): + try: + got.append(value(ds[i])) + except OSError: + got.append(None) + return got + + +result = {} +for tag, sizes in (("8", None), ("4", (4, 4))): + g, x = os.path.join(out, f"vl{tag}.h5"), os.path.join(out, f"bad{tag}.h5") + good(g, sizes) + gcol = bad(x, sizes) + with h5py.File(g, "r") as f: + result[f"vl{tag}"] = {n: read(f[n]) for n in ("d", "u", "seq", "sequ", "cmp")} + result[f"vl{tag}"]["va"] = [value(s) for s in f.attrs["va"]] + with h5py.File(x, "r") as f: + result[f"bad{tag}"] = {n: read(f[n]) for n in ("bad", "badseq")} + result[f"bad{tag}"]["gcol"] = gcol +json.dump(result, sys.stdout) diff --git a/crates/clawhdf5-tools/tests/h5rs_interop.rs b/crates/clawhdf5-tools/tests/h5rs_interop.rs index 7d97cc5..a1b4492 100644 --- a/crates/clawhdf5-tools/tests/h5rs_interop.rs +++ b/crates/clawhdf5-tools/tests/h5rs_interop.rs @@ -773,3 +773,125 @@ fn every_subcommand_rejects_a_non_hdf5_file_cleanly() { assert_eq!(code(&h5rs(&["ls"])), 2); assert_eq!(code(&h5rs(&["--help"])), 0); } + +// --------------------------------------------------------------------------- +// variable-length data +// --------------------------------------------------------------------------- + +/// Runs `tests/gen_vl_files.py`: VL strings (with an embedded NUL, empty +/// and null elements), VL sequences, a VL compound member and a VL +/// attribute, with 8- and 4-byte offsets, plus files whose heap objects +/// disagree with their elements' lengths. +fn generate_vl() -> Option { + if missing(python_available(), "python3 with h5py") { + return None; + } + let dir = tempfile::tempdir().unwrap(); + let script = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/gen_vl_files.py"); + let out = Command::new(python()) + .arg(&script) + .arg(dir.path()) + .output() + .expect("run gen_vl_files.py"); + assert!( + out.status.success(), + "gen_vl_files.py failed:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + let values = serde_json::from_slice(&out.stdout).expect("gen_vl_files.py output"); + Some(Files { dir, values }) +} + +/// `dump` resolves VL elements through the library's `VlResolver`, as +/// libhdf5 does: "a\0b" prints as "a", a null string as NULL (it printed +/// ""), and with 4-byte offsets too; the output is h5dump's byte for byte. +#[test] +fn dump_prints_vl_data_like_h5dump() { + let Some(f) = generate_vl() else { return }; + for name in ["vl8.h5", "vl4.h5"] { + let p = f.p(name); + let ours = stdout(&h5rs(&["dump", &p])); + assert!( + ours.contains(r#"(0): "a", "", NULL, "zz", "hello""#), + "{name}:\n{ours}" + ); + assert!(ours.contains(r#"(0): NULL, "w", NULL, NULL"#), "{name}"); + assert!(ours.contains("(0): (1, 2, 3), (), (-5)"), "{name}"); + if missing(tool_available("h5dump"), "h5dump") { + continue; + } + let reference = run("h5dump", &[&p]); + assert!(reference.status.success(), "{name}: {reference:?}"); + assert_eq!(ours, stdout(&reference).replacen(&p, name, 1), "{name}"); + } +} + +/// `dump --json` gives the values h5py reads, element by element; and an +/// element whose heap object is not its length × base size is an error, as +/// in h5py, not a truncated value (it printed "cde" and (1, 2)). +#[test] +fn dump_json_vl_values_match_h5py() { + let Some(f) = generate_vl() else { return }; + for tag in ["8", "4"] { + let (good, bad) = (format!("vl{tag}"), format!("bad{tag}")); + let o = h5rs(&["dump", "--json", &f.p(&format!("{good}.h5"))]); + assert!(o.status.success(), "{good}: {o:?}"); + let doc: serde_json::Value = serde_json::from_slice(&o.stdout).unwrap(); + let want = &f.values[&good]; + for d in doc["datasets"].as_object().unwrap().values() { + let path = d["alias"][0].as_str().unwrap(); + assert_eq!(d["value"], want[&path[1..]], "{good}: {path}"); + } + let attrs = &doc["groups"][doc["root"].as_str().unwrap()]["attributes"]; + assert_eq!(attrs[0]["name"], "va"); + assert_eq!(attrs[0]["value"], want["va"], "{good}: va"); + + let o = h5rs(&["dump", "--json", &f.p(&format!("{bad}.h5"))]); + let doc: serde_json::Value = serde_json::from_slice(&o.stdout).unwrap(); + let want = &f.values[&bad]; + for d in doc["datasets"].as_object().unwrap().values() { + let path = d["alias"][0].as_str().unwrap(); + let got = d["value"].as_array().unwrap(); + let want = want[&path[1..]].as_array().unwrap(); + assert_eq!(got.len(), want.len(), "{bad}: {path}"); + for (g, w) in got.iter().zip(want) { + if w.is_null() { + // h5py cannot read it: neither can we. + let e = g["error"] + .as_str() + .unwrap_or_else(|| panic!("{bad}: {path}: {g}")); + assert!(e.contains("holds"), "{bad}: {path}: {e}"); + } else { + assert_eq!(g, w, "{bad}: {path}"); + } + } + } + } +} + +/// `check --data` holds VL elements to libhdf5's rule: a heap object whose +/// size is not exactly the element's length × base size is a problem (it +/// only caught objects shorter than the element). +#[test] +fn check_data_flags_mis_sized_vl_heap_objects() { + let Some(f) = generate_vl() else { return }; + for tag in ["8", "4"] { + let o = h5rs(&["check", "--data", &f.p(&format!("vl{tag}.h5"))]); + let s = stdout(&o); + assert_eq!(code(&o), 0, "vl{tag}: {s}"); + assert!( + s.contains("global heap collections read: 1"), + "vl{tag}: {s}" + ); + + let o = h5rs(&["check", "--data", &f.p(&format!("bad{tag}.h5"))]); + let s = stdout(&o); + assert_eq!(code(&o), 1, "bad{tag}: {s}"); + let at = f.values[format!("bad{tag}")]["gcol"].as_u64().unwrap(); + for (path, what) in [("/bad", "6 bytes"), ("/badseq", "12 bytes")] { + let want = format!("problem: {at:#x} {path}: variable-length data: global heap object"); + assert!(s.contains(&want), "bad{tag}: no {want:?} in\n{s}"); + assert!(s.contains(what), "bad{tag}: {s}"); + } + } +} diff --git a/crates/clawhdf5-wasm/src/core.rs b/crates/clawhdf5-wasm/src/core.rs index baadcda..4c6beab 100644 --- a/crates/clawhdf5-wasm/src/core.rs +++ b/crates/clawhdf5-wasm/src/core.rs @@ -9,6 +9,7 @@ use clawhdf5::{AttrValue, File, Selection}; use clawhdf5_format::data_read; use clawhdf5_format::datatype::{Datatype, DatatypeByteOrder}; +use clawhdf5_format::vl_data::{VlResolver, check_element_size}; /// Errors are reported to JavaScript as messages. pub type Result = std::result::Result; @@ -221,6 +222,12 @@ impl Reader { None => (Selection::All, shape.clone()), Some(h) => hyperslab_selection(h, &shape)?, }; + // A VL type whose stored element size is not the one the file's + // offset size implies is refused before its data is read, as + // `File::read_string` refuses it. + if let Datatype::VariableLength { size, .. } = array_base(&dt) { + check_element_size(*size, self.file.superblock().offset_size).map_err(err)?; + } let raw = ds.read_selection(&selection).map_err(err)?; let data = self.decode(&raw, &dt)?; out_shape.extend(element_shape(&dt)); @@ -270,23 +277,14 @@ impl Reader { Datatype::VariableLength { is_string: true, .. } if !is_array => { - let size = dt.type_size() as usize; - if size == 0 || !raw.len().is_multiple_of(size) { - return Err(format!( - "{} bytes is not a whole number of {size}-byte string references", - raw.len() - )); - } + // The library's resolver, as File::read_string uses: a + // string ends at its first NUL and a heap object of the + // wrong size is an error, as in libhdf5 and h5py. let sb = self.file.superblock(); Data::Strings( - clawhdf5_format::vl_data::read_vl_strings( - self.file.as_bytes(), - raw, - (raw.len() / size) as u64, - sb.offset_size, - sb.length_size, - ) - .map_err(err)?, + VlResolver::new(self.file.as_bytes(), sb.offset_size, sb.length_size) + .strings(raw) + .map_err(err)?, ) } Datatype::Enumeration { .. } if !is_array => { diff --git a/crates/clawhdf5-wasm/tests/vl_strings.rs b/crates/clawhdf5-wasm/tests/vl_strings.rs new file mode 100644 index 0000000..1baddd0 --- /dev/null +++ b/crates/clawhdf5-wasm/tests/vl_strings.rs @@ -0,0 +1,150 @@ +//! The wasm reader resolves VL strings with the library's `VlResolver`, so +//! it returns what `File::read_string` and h5py return: a string ends at +//! its first NUL, a null element is empty, a heap object of the wrong size +//! is an error, and a VL datatype whose stored element size disagrees with +//! the file's offset size is refused. Checked with 8- and 4-byte offsets. +//! +//! Skipped when python3 with h5py is missing, unless +//! `CLAWHDF5_REQUIRE_INTEROP=1`. `CLAWHDF5_PYTHON` names the interpreter. + +use std::process::Command; + +use clawhdf5::File; +use clawhdf5_wasm::core::{Data, Reader}; + +fn python() -> String { + std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string()) +} + +fn h5py_available() -> bool { + let ok = Command::new(python()) + .args(["-c", "import h5py, numpy"]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false); + if !ok { + assert!( + std::env::var("CLAWHDF5_REQUIRE_INTEROP").as_deref() != Ok("1"), + "CLAWHDF5_REQUIRE_INTEROP=1 but python with h5py is not available" + ); + eprintln!("SKIP: python with h5py not available"); + } + ok +} + +/// For each offset size: `vl{8,4}.h5` with dataset `d` = "a\0b", "", null, +/// "zz" (patched: h5py writes neither a NUL nor a null element); +/// `bad{8,4}.h5` whose element 0 claims 3 bytes of a 6-byte heap object; +/// and `size{8,4}.h5` whose VL datatype message stores a 24-byte element. +/// Prints h5py's reading of each element as hex, or "error". +const SCRIPT: &str = r#" +import struct, sys, h5py, numpy as np +out = sys.argv[1] +S = h5py.string_dtype('utf-8') +def create(path, os_): + if os_ == 8: + return h5py.File(path, 'w', libver='earliest') + # The earliest format, so the patched object header has no checksum. + fcpl = h5py.h5p.create(h5py.h5p.FILE_CREATE); fcpl.set_sizes(4, 4) + fapl = h5py.h5p.create(h5py.h5p.FILE_ACCESS) + fapl.set_libver_bounds(h5py.h5f.LIBVER_EARLIEST, h5py.h5f.LIBVER_V18) + return h5py.File(h5py.h5f.create(path.encode(), h5py.h5f.ACC_TRUNC, fcpl=fcpl, fapl=fapl)) +def elem(length, addr, index, os_): + return struct.pack(' = String::from_utf8(out.stdout) + .unwrap() + .lines() + .map(|l| { + let (k, v) = l.split_once('\t').unwrap(); + (k.to_string(), v.to_string()) + }) + .collect(); + let read = |name: &str| { + let bytes = std::fs::read(dir.path().join(format!("{name}.h5"))).unwrap(); + let wasm = Reader::open(bytes).unwrap().read("/d", None); + let file = File::open(dir.path().join(format!("{name}.h5"))) + .unwrap() + .dataset("d") + .unwrap() + .read_string(); + (wasm, file) + }; + for os in [8, 4] { + // h5py: "a\0b" is "a"; the null element (address 0) is empty. + assert_eq!(h5py[&format!("vl{os}")], "61,,,7a7a"); + let (wasm, file) = read(&format!("vl{os}")); + let Data::Strings(wasm) = wasm.unwrap().data else { + panic!("vl{os}: not strings") + }; + assert_eq!(wasm, ["a", "", "", "zz"], "vl{os}"); + assert_eq!(wasm, file.unwrap(), "vl{os}"); + + // h5py refuses the mis-sized element; so do both readers. + assert_eq!(h5py[&format!("bad{os}")], "error,6f6b"); + let (wasm, file) = read(&format!("bad{os}")); + assert!(wasm.unwrap_err().contains("holds 6 bytes"), "bad{os}"); + assert!(file.is_err(), "bad{os}"); + + // libhdf5 ignores the stored element size and reads the values; + // File refuses the datatype rather than guess its layout, and the + // wasm reader now does the same (it read with the stored size). + assert_eq!(h5py[&format!("size{os}")], "78,7979"); + let (wasm, file) = read(&format!("size{os}")); + let e = wasm.unwrap_err(); + assert!(e.contains("stores 24-byte elements"), "size{os}: {e}"); + assert!(file.is_err(), "size{os}"); + } +} diff --git a/docs/known-issues.md b/docs/known-issues.md index ceb6ab6..5f43bdb 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -207,9 +207,10 @@ fill-value item that did is fixed). - (`cve-2024-32616` `/group1/dset3` and `cve-2025-2309`'s `Comp_OBJREF` attribute are h5py/numpy type-mapping failures, not libhdf5 refusals.) - `h5rs check` validates with the library's parsers, so it inherits what - they accept: of the 150 CVE and fuzzer files, `check --data` passes 16, - and h5dump 1.14.6 rejects 9 of those (tank, 2026-09-26; 28 and 21 - before these checks). + they accept: of the 150 CVE and fuzzer files, `check --data` passes 15, + and h5dump 1.14.6 rejects 8 of those (tank, 2026-09-26; 28 and 21 + before these checks, 16 and 9 before a VL type's stored element size + was checked, which flags `cve-2024-32608`). - **Writer:** - Nested groups beyond one level: path-like names are now refused, not created. @@ -537,8 +538,9 @@ which is what libhdf5 itself writes. followed (no file system). - Variable-length string datasets are read by decoding `read_selection`'s bytes with `clawhdf5_format::vl_data` in the wasm crate; `File` itself still - cannot (see the audit gaps above). (`File` can since 2026-09-26; the wasm - crate still decodes them itself.) + cannot (see the audit gaps above). (`File` can since 2026-09-26. Since + 2026-09-26 the wasm crate resolves them with the same `VlResolver` as + `File` and `h5rs`, so all three return h5py's values.) ## The Node.js package (`packages/clawhdf5-node`) does not work