Fix silent wrong data and libhdf5 interop found by the HDF5 audit #11
@@ -991,21 +991,24 @@ enum Scalar {
|
||||
}
|
||||
|
||||
impl Scalar {
|
||||
/// Float to integer conversions follow libhdf5's hard conversions:
|
||||
/// truncate toward zero, and saturate a value outside the target range to
|
||||
/// its minimum or maximum. NaN converts to 0 (libhdf5 leaves that case to
|
||||
/// the C cast, whose result is platform-dependent).
|
||||
// Every conversion follows libhdf5's default (hard) conversions: a value
|
||||
// outside the target type's range saturates to its minimum or maximum —
|
||||
// including a negative value read as unsigned, which reads as 0 — rather
|
||||
// than being truncated to its low bits. Floats truncate toward zero; NaN
|
||||
// converts to 0 (libhdf5 leaves that case to the C cast, whose result is
|
||||
// platform-dependent).
|
||||
|
||||
fn to_i64(self) -> i64 {
|
||||
match self {
|
||||
Scalar::Signed(v) => v,
|
||||
Scalar::Unsigned(v) => v as i64,
|
||||
Scalar::Unsigned(v) => i64::try_from(v).unwrap_or(i64::MAX),
|
||||
Scalar::Float(v) => v as i64,
|
||||
}
|
||||
}
|
||||
|
||||
fn to_u64(self) -> u64 {
|
||||
match self {
|
||||
Scalar::Signed(v) => v as u64,
|
||||
Scalar::Signed(v) => u64::try_from(v).unwrap_or(0),
|
||||
Scalar::Unsigned(v) => v,
|
||||
Scalar::Float(v) => v as u64,
|
||||
}
|
||||
@@ -1013,8 +1016,8 @@ impl Scalar {
|
||||
|
||||
fn to_i32(self) -> i32 {
|
||||
match self {
|
||||
Scalar::Signed(v) => v as i32,
|
||||
Scalar::Unsigned(v) => v as i32,
|
||||
Scalar::Signed(v) => v.clamp(i32::MIN.into(), i32::MAX.into()) as i32,
|
||||
Scalar::Unsigned(v) => i32::try_from(v).unwrap_or(i32::MAX),
|
||||
Scalar::Float(v) => v as i32,
|
||||
}
|
||||
}
|
||||
@@ -1048,8 +1051,10 @@ fn decode_scalar(
|
||||
|
||||
/// Convert raw bytes to `i64` values.
|
||||
///
|
||||
/// Floating-point data is converted the way libhdf5 converts it: truncated
|
||||
/// toward zero, saturating at the target type's range, with NaN read as 0.
|
||||
/// Values are converted the way libhdf5 converts them: integers outside the
|
||||
/// target range saturate at its minimum or maximum (a negative value read as
|
||||
/// unsigned is 0), and floating-point data is truncated toward zero and
|
||||
/// saturated, with NaN read as 0.
|
||||
pub fn read_as_i64(raw: &[u8], datatype: &Datatype) -> Result<Vec<i64>, FormatError> {
|
||||
if let Datatype::Array { base_type, .. } = datatype {
|
||||
return read_as_i64(raw, base_type);
|
||||
@@ -1091,8 +1096,10 @@ pub fn read_as_i64(raw: &[u8], datatype: &Datatype) -> Result<Vec<i64>, FormatEr
|
||||
|
||||
/// Convert raw bytes to `u64` values.
|
||||
///
|
||||
/// Floating-point data is converted the way libhdf5 converts it: truncated
|
||||
/// toward zero, saturating at the target type's range, with NaN read as 0.
|
||||
/// Values are converted the way libhdf5 converts them: integers outside the
|
||||
/// target range saturate at its minimum or maximum (a negative value read as
|
||||
/// unsigned is 0), and floating-point data is truncated toward zero and
|
||||
/// saturated, with NaN read as 0.
|
||||
pub fn read_as_u64(raw: &[u8], datatype: &Datatype) -> Result<Vec<u64>, FormatError> {
|
||||
if let Datatype::Array { base_type, .. } = datatype {
|
||||
return read_as_u64(raw, base_type);
|
||||
@@ -1207,8 +1214,10 @@ pub fn read_as_f32(raw: &[u8], datatype: &Datatype) -> Result<Vec<f32>, FormatEr
|
||||
|
||||
/// Convert raw bytes to `i32` values.
|
||||
///
|
||||
/// Floating-point data is converted the way libhdf5 converts it: truncated
|
||||
/// toward zero, saturating at the target type's range, with NaN read as 0.
|
||||
/// Values are converted the way libhdf5 converts them: integers outside the
|
||||
/// target range saturate at its minimum or maximum (a negative value read as
|
||||
/// unsigned is 0), and floating-point data is truncated toward zero and
|
||||
/// saturated, with NaN read as 0.
|
||||
pub fn read_as_i32(raw: &[u8], datatype: &Datatype) -> Result<Vec<i32>, FormatError> {
|
||||
if let Datatype::Array { base_type, .. } = datatype {
|
||||
return read_as_i32(raw, base_type);
|
||||
@@ -1231,6 +1240,7 @@ pub fn read_as_i32(raw: &[u8], datatype: &Datatype) -> Result<Vec<i32>, FormatEr
|
||||
datatype,
|
||||
Datatype::FixedPoint {
|
||||
byte_order: DatatypeByteOrder::LittleEndian,
|
||||
signed: true,
|
||||
..
|
||||
}
|
||||
)
|
||||
|
||||
@@ -127,3 +127,64 @@ with h5py.File("{path}", "r") as f:
|
||||
let f2 = file.dataset("f2").unwrap();
|
||||
assert_eq!(f2.read_u64().unwrap(), vec![1, 0, 0, 0, 7, 65504, 0]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn integer_reads_saturate_out_of_range_values_like_libhdf5() {
|
||||
// Narrowing reads used to keep the low bits (i64 2^40+5 read as i32 was
|
||||
// 5, u64::MAX read as i64 was -1) and signed-to-unsigned reads wrapped
|
||||
// (-1 read as u64 was 4294967295).
|
||||
skip_if_no_python!();
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("int_narrowing.h5");
|
||||
let script = format!(
|
||||
r#"{PRELUDE}
|
||||
data = {{
|
||||
"i8": np.array([2**40 + 5, -(2**35), 7, -1, 2**63 - 1, -(2**63)], "<i8"),
|
||||
"i8be": np.array([2**40 + 5, -(2**35), 7, -1], ">i8"),
|
||||
"u8": np.array([2**64 - 1, 2**63, 5, 0], "<u8"),
|
||||
"u4": np.array([2**32 - 1, 2**31, 2**31 - 1, 3], "<u4"),
|
||||
"i4": np.array([-1, 5, -(2**31), 2**31 - 1], "<i4"),
|
||||
"i2be": np.array([-300, 300, -1], ">i2"),
|
||||
"u1": np.array([255, 0, 128], "u1"),
|
||||
}}
|
||||
with h5py.File("{path}", "w") as f:
|
||||
for name, arr in data.items():
|
||||
f.create_dataset(name, data=arr)
|
||||
with h5py.File("{path}", "r") as f:
|
||||
for name in data:
|
||||
d = f[name]
|
||||
emit(name + ":i32", d.astype("<i4")[()])
|
||||
emit(name + ":i64", d.astype("<i8")[()])
|
||||
emit(name + ":u64", d.astype("<u8")[()])
|
||||
"#,
|
||||
path = path.display()
|
||||
);
|
||||
let expected = run_python(&script);
|
||||
|
||||
let file = File::open(&path).unwrap();
|
||||
for name in ["i8", "i8be", "u8", "u4", "i4", "i2be", "u1"] {
|
||||
let ds = file.dataset(name).unwrap();
|
||||
assert_eq!(
|
||||
ds.read_i32().unwrap(),
|
||||
parse::<i32>(&expected[&format!("{name}:i32")]),
|
||||
"{name} as i32"
|
||||
);
|
||||
assert_eq!(
|
||||
ds.read_i64().unwrap(),
|
||||
parse::<i64>(&expected[&format!("{name}:i64")]),
|
||||
"{name} as i64"
|
||||
);
|
||||
// libhdf5 wraps a negative big-endian i64 read as little-endian u64
|
||||
// (it only byte-swaps when the sizes match and the order differs);
|
||||
// every other signed-to-unsigned read saturates at 0, so do that.
|
||||
if name != "i8be" {
|
||||
assert_eq!(
|
||||
ds.read_u64().unwrap(),
|
||||
parse::<u64>(&expected[&format!("{name}:u64")]),
|
||||
"{name} as u64"
|
||||
);
|
||||
}
|
||||
}
|
||||
let i8be = file.dataset("i8be").unwrap();
|
||||
assert_eq!(i8be.read_u64().unwrap(), vec![(1 << 40) + 5, 0, 7, 0]);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user