Skip to content

Commit

Permalink
Fix handling of empty data (#24)
Browse files Browse the repository at this point in the history
* Add fmt::Debug to InflateState

* Add failing empty_data variant

* All fixed except empty_data rountrip into

* Fix compress_into test when empty data

* Bump version -> 0.5.2
  • Loading branch information
milesgranger authored Oct 5, 2024
1 parent 9f2c942 commit 846a262
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 10 deletions.
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "isal-rs"
version = "0.5.1+496255c"
version = "0.5.2+496255c"
edition = "2021"
description = "isa-l Rust bindings"
readme = "README.md"
Expand Down Expand Up @@ -28,7 +28,7 @@ use-system-isal = ["isal-sys/use-system-isal"]

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]
isal-sys = { path = "isal-sys", version = "0.5.1+496255c" }
isal-sys = { path = "isal-sys", version = "0.5.2+496255c" }

[dev-dependencies]
criterion = { version = "0.5", features = ["html_reports"] }
Expand Down
2 changes: 1 addition & 1 deletion isal-sys/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "isal-sys"
version = "0.5.1+496255c"
version = "0.5.2+496255c"
edition = "2021"
description = "isa-l sys crate"
license = "MIT AND BSD-3-Clause"
Expand Down
35 changes: 29 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,17 @@ impl ZStream {

pub(crate) struct InflateState {
state: isal::inflate_state,
no_change_count: usize,
}

impl std::fmt::Debug for InflateState {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("InflateState")
.field("block_state", &self.state.block_state)
.field("avail_in", &self.state.avail_in)
.field("avail_out", &self.state.avail_out)
.finish()
}
}

impl InflateState {
Expand All @@ -316,7 +327,10 @@ impl InflateState {
unsafe { isal::isal_inflate_init(uninit.as_mut_ptr()) };
let mut state = unsafe { uninit.assume_init() };
state.crc_flag = codec as _;
Self { state }
Self {
state,
no_change_count: 0,
}
}

pub fn block_state(&self) -> u32 {
Expand All @@ -325,6 +339,7 @@ impl InflateState {

pub fn reset(&mut self) {
unsafe { isal::isal_inflate_reset(&mut self.state) }
self.no_change_count = 0;
}

pub fn step_inflate(&mut self) -> Result<()> {
Expand All @@ -341,10 +356,13 @@ impl InflateState {
// ISA-L doesn't catch some bad data in the header and will loop endlessly
// unless we check if anything was written to the output
if self.state.avail_out == avail_out {
return Err(Error::Other((
None,
"Corrupt data, appears the compressed input is invalid".to_string(),
)));
self.no_change_count += 1;
if self.no_change_count >= 2 {
return Err(Error::Other((
None,
"Corrupt data, appears the compressed input is invalid".to_string(),
)));
}
}
Ok(())
}
Expand Down Expand Up @@ -395,6 +413,9 @@ pub mod tests {
pub fn small_data() -> Vec<u8> {
b"foobar".to_vec()
}
pub fn empty_data() -> Vec<u8> {
vec![]
}

pub fn same_same(a: &[u8], b: &[u8]) -> bool {
md5::compute(a) == md5::compute(b)
Expand Down Expand Up @@ -470,7 +491,8 @@ pub mod tests {
let compressed_len = compress(data.as_slice(), $lvl, $codec).unwrap().len();
let decompressed_len = data.len();

let mut compressed = vec![0; compressed_len * 2];
// cmp is special case when data to be compressed is empty, we at least need room for the header
let mut compressed = vec![0; std::cmp::max(compressed_len * 2, 8)];
let mut decompressed = vec![0; decompressed_len * 2];

// compress_into
Expand Down Expand Up @@ -657,6 +679,7 @@ pub mod tests {
}
}
}
test_data_size!(empty_data);
test_data_size!(small_data);
test_data_size!(large_data);
}
Expand Down
19 changes: 18 additions & 1 deletion src/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@ pub struct Decoder<W: io::Write> {
inner: W,
zst: InflateState,
out_buf: Vec<u8>,
total_out: usize,
total_in: usize,
#[allow(dead_code)]
codec: Codec,
}
Expand All @@ -196,6 +198,8 @@ impl<W: io::Write> Decoder<W> {
inner: writer,
zst,
out_buf: Vec::with_capacity(BUF_SIZE),
total_out: 0,
total_in: 0,
codec,
}
}
Expand All @@ -216,8 +220,10 @@ impl<W: io::Write> io::Write for Decoder<W> {
self.zst.state.avail_in = buf.len() as _;
self.zst.state.next_in = buf.as_ptr() as *mut _;

self.total_in += buf.len();

let mut n_bytes = 0;
while self.zst.state.avail_in > 0 {
loop {
loop {
self.out_buf.resize(n_bytes + BUF_SIZE, 0);

Expand All @@ -227,6 +233,7 @@ impl<W: io::Write> io::Write for Decoder<W> {
self.zst.step_inflate()?;

n_bytes += BUF_SIZE - self.zst.state.avail_out as usize;
self.total_out += n_bytes;

if self.zst.block_state() == isal::isal_block_state_ISAL_BLOCK_FINISH {
break;
Expand All @@ -235,13 +242,23 @@ impl<W: io::Write> io::Write for Decoder<W> {
if self.zst.block_state() == isal::isal_block_state_ISAL_BLOCK_FINISH {
self.zst.reset();
}

if self.zst.state.avail_in == 0 {
break;
}
}
self.inner.write_all(&self.out_buf[..n_bytes])?;

let nbytes = buf.len() - self.zst.state.avail_in as usize;
Ok(nbytes)
}
fn flush(&mut self) -> io::Result<()> {
if self.total_out == 0 && self.total_in == 0 {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
Error::DecompressionError(DecompCode::EndInput),
));
}
self.inner.flush()
}
}
Expand Down

0 comments on commit 846a262

Please sign in to comment.