fix(format): read VL values in files with 4-byte offsets
In a file with sizeof_addr = 4, a VL string attribute came back as AttrValue::Raw, a compound's VL member failed with GlobalHeapObjectNotFound and VL datasets failed with a size mismatch. Two bugs: Datatype::type_size() said 16 for every VL type, while the element is 4 + offset size + 4 bytes (12 here); and the global heap was parsed without the padding libhdf5 puts after its collection and object headers (both round up to 8), so with 4-byte lengths every object was looked up 4 bytes early. Datatype::VariableLength now carries the size its datatype message stores, and writes it back. Checked against h5py in tests/vl_offset4_interop.rs (fails with either fix reverted). Conformance unchanged at 575 of 697; in cve-2024-32608 a VL attribute whose datatype claims 524304-byte elements is now an error (h5py cannot iterate those attributes at all). Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -2,6 +2,21 @@
|
|||||||
|
|
||||||
## Unreleased
|
## Unreleased
|
||||||
|
|
||||||
|
### Variable-length data (2026-09-26)
|
||||||
|
- **VL values in files with 4-byte offsets** (`sizeof_addr = 4`). A VL
|
||||||
|
string attribute came back as `AttrValue::Raw`, a VL member of a compound
|
||||||
|
failed with `GlobalHeapObjectNotFound`, and VL datasets failed with a
|
||||||
|
size mismatch. Two causes: `Datatype::type_size()` reported 16 for every
|
||||||
|
VL type (the element is 4 + offset size + 4 bytes: 12 here), and the
|
||||||
|
global heap was parsed without the padding libhdf5 puts after its
|
||||||
|
collection and object headers (`H5HG_SIZEOF_HDR`/`H5HG_SIZEOF_OBJHDR`
|
||||||
|
round up to 8), so with 4-byte lengths every object was looked up 4
|
||||||
|
bytes early. `Datatype::VariableLength` now carries the element `size`
|
||||||
|
stored in the datatype message (**breaking** for code that builds or
|
||||||
|
exhaustively destructures that variant; patterns with `..` are
|
||||||
|
unaffected), and it is written back as stored. Tested against h5py
|
||||||
|
(`crates/clawhdf5/tests/vl_offset4_interop.rs`).
|
||||||
|
|
||||||
### Plugin filters (2026-09-26)
|
### Plugin filters (2026-09-26)
|
||||||
- **LZF, bitshuffle, bzip2 and Blosc read and write, in pure Rust.** Files
|
- **LZF, bitshuffle, bzip2 and Blosc read and write, in pure Rust.** Files
|
||||||
written by h5py with `compression="lzf"`, or with hdf5plugin's
|
written by h5py with `compression="lzf"`, or with hdf5plugin's
|
||||||
|
|||||||
@@ -125,6 +125,11 @@ pub enum Datatype {
|
|||||||
},
|
},
|
||||||
/// Class 9: Variable-length type.
|
/// Class 9: Variable-length type.
|
||||||
VariableLength {
|
VariableLength {
|
||||||
|
/// Size of one element as stored in the file: a sequence length (4
|
||||||
|
/// bytes), a global heap collection address (the file's
|
||||||
|
/// `offset_size`) and an object index (4 bytes) — 16 in a file with
|
||||||
|
/// 8-byte offsets, 12 with 4-byte offsets.
|
||||||
|
size: u32,
|
||||||
is_string: bool,
|
is_string: bool,
|
||||||
padding: Option<StringPadding>,
|
padding: Option<StringPadding>,
|
||||||
charset: Option<CharacterSet>,
|
charset: Option<CharacterSet>,
|
||||||
@@ -771,6 +776,7 @@ impl Datatype {
|
|||||||
pos += consumed;
|
pos += consumed;
|
||||||
Ok((
|
Ok((
|
||||||
Datatype::VariableLength {
|
Datatype::VariableLength {
|
||||||
|
size,
|
||||||
is_string,
|
is_string,
|
||||||
padding,
|
padding,
|
||||||
charset,
|
charset,
|
||||||
@@ -1017,6 +1023,7 @@ impl Datatype {
|
|||||||
Self::build_header(3, 1, [bf0, 0, 0], *size)
|
Self::build_header(3, 1, [bf0, 0, 0], *size)
|
||||||
}
|
}
|
||||||
Datatype::VariableLength {
|
Datatype::VariableLength {
|
||||||
|
size,
|
||||||
is_string,
|
is_string,
|
||||||
padding,
|
padding,
|
||||||
charset,
|
charset,
|
||||||
@@ -1039,7 +1046,7 @@ impl Datatype {
|
|||||||
} else {
|
} else {
|
||||||
0
|
0
|
||||||
};
|
};
|
||||||
let mut buf = Self::build_header(9, 1, [bf0, bf1, 0], 16);
|
let mut buf = Self::build_header(9, 1, [bf0, bf1, 0], *size);
|
||||||
buf.extend_from_slice(&base_type.serialize());
|
buf.extend_from_slice(&base_type.serialize());
|
||||||
buf
|
buf
|
||||||
}
|
}
|
||||||
@@ -1208,7 +1215,7 @@ impl Datatype {
|
|||||||
Datatype::Compound { size, .. } => *size,
|
Datatype::Compound { size, .. } => *size,
|
||||||
Datatype::Reference { size, .. } => *size,
|
Datatype::Reference { size, .. } => *size,
|
||||||
Datatype::Enumeration { size, .. } => *size,
|
Datatype::Enumeration { size, .. } => *size,
|
||||||
Datatype::VariableLength { .. } => 16, // typically pointer + length
|
Datatype::VariableLength { size, .. } => *size,
|
||||||
Datatype::Array {
|
Datatype::Array {
|
||||||
base_type,
|
base_type,
|
||||||
dimensions,
|
dimensions,
|
||||||
@@ -1889,11 +1896,13 @@ mod tests {
|
|||||||
let (dt, _) = Datatype::parse(&buf).unwrap();
|
let (dt, _) = Datatype::parse(&buf).unwrap();
|
||||||
match dt {
|
match dt {
|
||||||
Datatype::VariableLength {
|
Datatype::VariableLength {
|
||||||
|
size,
|
||||||
is_string,
|
is_string,
|
||||||
padding,
|
padding,
|
||||||
charset,
|
charset,
|
||||||
base_type,
|
base_type,
|
||||||
} => {
|
} => {
|
||||||
|
assert_eq!(size, 16);
|
||||||
assert!(is_string);
|
assert!(is_string);
|
||||||
assert_eq!(padding, Some(StringPadding::NullTerminate));
|
assert_eq!(padding, Some(StringPadding::NullTerminate));
|
||||||
assert_eq!(charset, Some(CharacterSet::Utf8));
|
assert_eq!(charset, Some(CharacterSet::Utf8));
|
||||||
@@ -1914,11 +1923,13 @@ mod tests {
|
|||||||
let (dt, _) = Datatype::parse(&buf).unwrap();
|
let (dt, _) = Datatype::parse(&buf).unwrap();
|
||||||
match dt {
|
match dt {
|
||||||
Datatype::VariableLength {
|
Datatype::VariableLength {
|
||||||
|
size,
|
||||||
is_string,
|
is_string,
|
||||||
padding,
|
padding,
|
||||||
charset,
|
charset,
|
||||||
base_type,
|
base_type,
|
||||||
} => {
|
} => {
|
||||||
|
assert_eq!(size, 16);
|
||||||
assert!(!is_string);
|
assert!(!is_string);
|
||||||
assert_eq!(padding, None);
|
assert_eq!(padding, None);
|
||||||
assert_eq!(charset, None);
|
assert_eq!(charset, None);
|
||||||
@@ -1928,6 +1939,19 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn variable_length_size_is_the_stored_size() {
|
||||||
|
// A file with 4-byte offsets stores 12-byte VL elements (length 4 +
|
||||||
|
// address 4 + index 4); the type used to report 16 regardless, so
|
||||||
|
// every read laid the elements out 16 bytes apart.
|
||||||
|
let mut buf = build_dt_header(9, 1, [0x01, 0x00, 0], 12);
|
||||||
|
buf.extend_from_slice(&build_fixed_point(1, false, false, 0, 8));
|
||||||
|
let (dt, _) = Datatype::parse(&buf).unwrap();
|
||||||
|
assert_eq!(dt.type_size(), 12);
|
||||||
|
// And it is written back as stored.
|
||||||
|
assert_eq!(dt.serialize()[4..8], 12u32.to_le_bytes());
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_array_2d() {
|
fn test_array_2d() {
|
||||||
// Array [3][4] of i32 LE, version 3
|
// Array [3][4] of i32 LE, version 3
|
||||||
|
|||||||
@@ -64,8 +64,11 @@ impl GlobalHeapCollection {
|
|||||||
offset: usize,
|
offset: usize,
|
||||||
length_size: u8,
|
length_size: u8,
|
||||||
) -> Result<GlobalHeapCollection, FormatError> {
|
) -> Result<GlobalHeapCollection, FormatError> {
|
||||||
// signature(4) + version(1) + reserved(3) + collection_size(length_size)
|
// signature(4) + version(1) + reserved(3) + collection_size(length_size),
|
||||||
let header_size = 8 + length_size as usize;
|
// padded to a multiple of 8 as libhdf5 lays it out (`H5HG_SIZEOF_HDR`).
|
||||||
|
// With 8-byte lengths the padding is 0; with 4-byte lengths it is 4,
|
||||||
|
// and reading without it put every object 4 bytes early.
|
||||||
|
let header_size = pad8(8 + length_size as usize);
|
||||||
ensure_len(file_data, offset, header_size)?;
|
ensure_len(file_data, offset, header_size)?;
|
||||||
|
|
||||||
if file_data[offset..offset + 4] != GCOL_SIGNATURE {
|
if file_data[offset..offset + 4] != GCOL_SIGNATURE {
|
||||||
@@ -104,8 +107,9 @@ impl GlobalHeapCollection {
|
|||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
// object_index(2) + reference_count(2) + reserved(4) + object_size(length_size)
|
// object_index(2) + reference_count(2) + reserved(4) +
|
||||||
let obj_header_size = 8 + length_size as usize;
|
// object_size(length_size), padded to 8 (`H5HG_SIZEOF_OBJHDR`).
|
||||||
|
let obj_header_size = pad8(8 + length_size as usize);
|
||||||
ensure_len(file_data, pos, obj_header_size)?;
|
ensure_len(file_data, pos, obj_header_size)?;
|
||||||
|
|
||||||
let reference_count = u16::from_le_bytes([file_data[pos + 2], file_data[pos + 3]]);
|
let reference_count = u16::from_le_bytes([file_data[pos + 2], file_data[pos + 3]]);
|
||||||
@@ -149,10 +153,11 @@ mod tests {
|
|||||||
let ls = length_size as usize;
|
let ls = length_size as usize;
|
||||||
|
|
||||||
// Calculate total size
|
// Calculate total size
|
||||||
let header_size = 8 + ls;
|
// libhdf5 pads both headers to a multiple of 8.
|
||||||
|
let header_size = pad8(8 + ls);
|
||||||
let mut obj_size_total = 0usize;
|
let mut obj_size_total = 0usize;
|
||||||
for (_, _, data) in objects {
|
for (_, _, data) in objects {
|
||||||
let obj_header = 8 + ls;
|
let obj_header = pad8(8 + ls);
|
||||||
obj_size_total += obj_header + pad8(data.len());
|
obj_size_total += obj_header + pad8(data.len());
|
||||||
}
|
}
|
||||||
// Free space marker (2 bytes for index 0)
|
// Free space marker (2 bytes for index 0)
|
||||||
@@ -170,6 +175,7 @@ mod tests {
|
|||||||
8 => buf.extend_from_slice(&(collection_size as u64).to_le_bytes()),
|
8 => buf.extend_from_slice(&(collection_size as u64).to_le_bytes()),
|
||||||
_ => panic!("unsupported length_size"),
|
_ => panic!("unsupported length_size"),
|
||||||
}
|
}
|
||||||
|
buf.resize(header_size, 0);
|
||||||
|
|
||||||
// Objects
|
// Objects
|
||||||
for (index, ref_count, data) in objects {
|
for (index, ref_count, data) in objects {
|
||||||
@@ -181,6 +187,7 @@ mod tests {
|
|||||||
8 => buf.extend_from_slice(&(data.len() as u64).to_le_bytes()),
|
8 => buf.extend_from_slice(&(data.len() as u64).to_le_bytes()),
|
||||||
_ => panic!("unsupported"),
|
_ => panic!("unsupported"),
|
||||||
}
|
}
|
||||||
|
buf.resize(buf.len() + (pad8(8 + ls) - (8 + ls)), 0);
|
||||||
buf.extend_from_slice(data);
|
buf.extend_from_slice(data);
|
||||||
// Pad to 8 bytes
|
// Pad to 8 bytes
|
||||||
let padded = pad8(data.len());
|
let padded = pad8(data.len());
|
||||||
|
|||||||
@@ -0,0 +1,126 @@
|
|||||||
|
//! Variable-length values in files with 4-byte offsets and lengths
|
||||||
|
//! (`sizeof_addr = 4`), checked against h5py/libhdf5 through the
|
||||||
|
//! `clawhdf5_format` decoders.
|
||||||
|
//!
|
||||||
|
//! These failed with `GlobalHeapObjectNotFound` or came back as
|
||||||
|
//! `AttrValue::Raw`: the VL datatype claimed 16-byte elements whatever the
|
||||||
|
//! file's offset size, and the global heap was read without the padding
|
||||||
|
//! libhdf5 puts after its collection and object headers. Skipped when
|
||||||
|
//! python3 with h5py is unavailable, unless `CLAWHDF5_REQUIRE_INTEROP=1`.
|
||||||
|
|
||||||
|
use std::process::Command;
|
||||||
|
|
||||||
|
use clawhdf5::{AttrValue, File, Selection};
|
||||||
|
use clawhdf5_format::data_read::{read_as_i64, read_compound_fields};
|
||||||
|
use clawhdf5_format::vl_data::{read_vl_bytes, read_vl_strings};
|
||||||
|
|
||||||
|
fn python() -> String {
|
||||||
|
std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string())
|
||||||
|
}
|
||||||
|
|
||||||
|
fn interop_required() -> bool {
|
||||||
|
std::env::var("CLAWHDF5_REQUIRE_INTEROP").is_ok_and(|v| v == "1")
|
||||||
|
}
|
||||||
|
|
||||||
|
fn python_available() -> bool {
|
||||||
|
Command::new(python())
|
||||||
|
.args(["-c", "import h5py, numpy"])
|
||||||
|
.output()
|
||||||
|
.map(|o| o.status.success())
|
||||||
|
.unwrap_or(false)
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn vl_values_in_a_file_with_4_byte_offsets_read_like_h5py() {
|
||||||
|
if !python_available() {
|
||||||
|
assert!(
|
||||||
|
!interop_required(),
|
||||||
|
"CLAWHDF5_REQUIRE_INTEROP=1 but python3 with h5py is not available"
|
||||||
|
);
|
||||||
|
eprintln!("SKIP: python3 with h5py not available");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let path = dir.path().join("offset4.h5");
|
||||||
|
let script = format!(
|
||||||
|
r#"
|
||||||
|
import h5py, numpy as np
|
||||||
|
S = h5py.string_dtype()
|
||||||
|
fcpl = h5py.h5p.create(h5py.h5p.FILE_CREATE); fcpl.set_sizes(4, 4)
|
||||||
|
with h5py.File(h5py.h5f.create({path:?}.encode(), h5py.h5f.ACC_TRUNC, fcpl=fcpl)) as f:
|
||||||
|
f.attrs['vlattr'] = 'attr-value'
|
||||||
|
f.attrs.create('vlattr_arr', np.array(['p', 'qq', ''], dtype=object), dtype=S)
|
||||||
|
ct = np.dtype([('id', '<i4'), ('name', S), ('v', '<f8')])
|
||||||
|
arr = np.zeros(3, dtype=ct); arr['id'] = [1, 2, 3]; arr['name'] = ['one', '', 'three']
|
||||||
|
f.create_dataset('compound', data=arr)
|
||||||
|
f.attrs.create('vlen_attr', np.array([np.array([1, 2], dtype='<i8'), np.array([3], dtype='<i8')],
|
||||||
|
dtype=object), dtype=h5py.vlen_dtype(np.dtype('<i8')))
|
||||||
|
with h5py.File({path:?}, 'r') as f:
|
||||||
|
assert f.id.get_create_plist().get_sizes() == (4, 4)
|
||||||
|
print(f.attrs['vlattr'])
|
||||||
|
print(','.join(f.attrs['vlattr_arr']))
|
||||||
|
print(','.join(s.decode() for s in f['compound']['name']))
|
||||||
|
print(';'.join(' '.join(str(x) for x in s) for s in f.attrs['vlen_attr']))
|
||||||
|
"#,
|
||||||
|
path = path.display().to_string()
|
||||||
|
);
|
||||||
|
let output = Command::new(python())
|
||||||
|
.args(["-c", &script])
|
||||||
|
.output()
|
||||||
|
.expect("failed to run python");
|
||||||
|
assert!(
|
||||||
|
output.status.success(),
|
||||||
|
"python failed:\n{}",
|
||||||
|
String::from_utf8_lossy(&output.stderr)
|
||||||
|
);
|
||||||
|
let stdout = String::from_utf8(output.stdout).unwrap();
|
||||||
|
let lines: Vec<&str> = stdout.lines().collect();
|
||||||
|
let (vlattr, vlattr_arr, names, seqs) = (lines[0], lines[1], lines[2], lines[3]);
|
||||||
|
|
||||||
|
let file = File::open(&path).unwrap();
|
||||||
|
let sb = file.superblock();
|
||||||
|
assert_eq!((sb.offset_size, sb.length_size), (4, 4));
|
||||||
|
|
||||||
|
let attrs = file.root().attrs().unwrap();
|
||||||
|
match &attrs["vlattr"] {
|
||||||
|
AttrValue::String(s) => assert_eq!(s, vlattr),
|
||||||
|
other => panic!("vlattr: {other:?}"),
|
||||||
|
}
|
||||||
|
match &attrs["vlattr_arr"] {
|
||||||
|
AttrValue::StringArray(s) => assert_eq!(s.join(","), vlattr_arr),
|
||||||
|
other => panic!("vlattr_arr: {other:?}"),
|
||||||
|
}
|
||||||
|
|
||||||
|
// The compound's VL string member.
|
||||||
|
let ds = file.dataset("compound").unwrap();
|
||||||
|
let dt = ds.raw_datatype().unwrap();
|
||||||
|
assert_eq!(dt.type_size(), 24, "4 + 12-byte VL element + 8");
|
||||||
|
let raw = ds.read_selection(&Selection::All).unwrap();
|
||||||
|
let fields = read_compound_fields(&raw, &dt).unwrap();
|
||||||
|
let name = fields.iter().find(|f| f.name == "name").unwrap();
|
||||||
|
assert_eq!(name.datatype.type_size(), 12);
|
||||||
|
let got = read_vl_strings(file.as_bytes(), &name.raw_data, 3, 4, 4).unwrap();
|
||||||
|
assert_eq!(got.join(","), names);
|
||||||
|
let id = fields.iter().find(|f| f.name == "id").unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
read_as_i64(&id.raw_data, &id.datatype).unwrap(),
|
||||||
|
vec![1, 2, 3]
|
||||||
|
);
|
||||||
|
|
||||||
|
// A VL sequence attribute.
|
||||||
|
let AttrValue::Raw { datatype, data, .. } = &attrs["vlen_attr"] else {
|
||||||
|
panic!("vlen_attr is Raw");
|
||||||
|
};
|
||||||
|
let clawhdf5_format::datatype::Datatype::VariableLength { base_type, .. } = datatype else {
|
||||||
|
panic!("vlen_attr is VL");
|
||||||
|
};
|
||||||
|
let got: Vec<String> = read_vl_bytes(file.as_bytes(), data, 2, 4, 4)
|
||||||
|
.unwrap()
|
||||||
|
.iter()
|
||||||
|
.map(|b| {
|
||||||
|
let v = read_as_i64(b, base_type).unwrap();
|
||||||
|
v.iter().map(i64::to_string).collect::<Vec<_>>().join(" ")
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
assert_eq!(got.join(";"), seqs);
|
||||||
|
}
|
||||||
@@ -146,7 +146,10 @@ fill-value item that did is fixed).
|
|||||||
`GlobalHeapObjectNotFound` or come back as `Raw`: these paths assume
|
`GlobalHeapObjectNotFound` or come back as `Raw`: these paths assume
|
||||||
the 16-byte element of an 8-byte-offset file. The datatype itself reads
|
the 16-byte element of an 8-byte-offset file. The datatype itself reads
|
||||||
(it was refused as "member overlaps with previous member" until
|
(it was refused as "member overlaps with previous member" until
|
||||||
2026-09-26).
|
2026-09-26). **Fixed 2026-09-26:** a VL type's element size is the one
|
||||||
|
its datatype message stores (12 with 4-byte offsets), and the global
|
||||||
|
heap is read with libhdf5's header padding
|
||||||
|
(`crates/clawhdf5/tests/vl_offset4_interop.rs`).
|
||||||
- Metadata cache images are not supported.
|
- Metadata cache images are not supported.
|
||||||
- x87 long double and binary128 are refused.
|
- x87 long double and binary128 are refused.
|
||||||
- N-Bit on 64-bit scale-offset data and some N-Bit parameter layouts fail.
|
- N-Bit on 64-bit scale-offset data and some N-Bit parameter layouts fail.
|
||||||
|
|||||||
Reference in New Issue
Block a user