From c3be758dc18c1dad82a59c3a3dc17834559e5795 Mon Sep 17 00:00:00 2001 From: Jonas Bushart Date: Sun, 15 Dec 2024 01:38:03 +0100 Subject: [PATCH 1/5] Add finisher drop implementation to BzEncoder This ensures that finish will always be attempted to be called, for example when operating as a `Box`. --- src/write.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/write.rs b/src/write.rs index 168d6fc0..83fc5988 100644 --- a/src/write.rs +++ b/src/write.rs @@ -287,6 +287,14 @@ impl Drop for BzDecoder { } } +impl Drop for BzEncoder { + fn drop(&mut self) { + if self.obj.is_some() { + let _ = self.try_finish(); + } + } +} + #[cfg(test)] mod tests { use super::{BzDecoder, BzEncoder}; From 1539a777c7e46c25fb6d9ce1e2c39dbbf5e70df4 Mon Sep 17 00:00:00 2001 From: Jonas Bushart Date: Tue, 17 Dec 2024 22:56:20 +0100 Subject: [PATCH 2/5] Add test that ensures the Drop implementation for BzEncoder will not regress Without the `impl Drop for BzEncoder` the test fails with the error message: called `Result::unwrap()` on an `Err` value: Custom { kind: UnexpectedEof, error: "Input EOF reached before logical stream end" } --- src/write.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/write.rs b/src/write.rs index 83fc5988..c0fcc3de 100644 --- a/src/write.rs +++ b/src/write.rs @@ -391,4 +391,29 @@ mod tests { .into_inner() } } + + #[test] + fn terminate_on_drop() { + // Test that dropping the BzEncoder will result in a valid, decompressable datastream + let s = "12345".repeat(100000); + + let mut compressed = Vec::new(); + { + let mut c: Box = + Box::new(BzEncoder::new(&mut compressed, Compression::default())); + c.write_all(b"12834").unwrap(); + c.write_all(s.as_bytes()).unwrap(); + c.flush().unwrap(); + } + assert!(!compressed.is_empty()); + + let uncompressed = { + let mut d = BzDecoder::new(Vec::new()); + d.write_all(&compressed).unwrap(); + d.finish().unwrap() + }; + assert_eq!(&uncompressed[0..5], b"12834"); + assert_eq!(uncompressed.len(), 500005); + assert!(format!("12834{}", s).as_bytes() == &*uncompressed); + } } From 5859239922a9b6074ca927722e6d00832d495884 Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Wed, 18 Dec 2024 21:09:24 +0100 Subject: [PATCH 3/5] link PR in the test als use a bit less memory; it's not really needed to prove the point --- src/write.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/write.rs b/src/write.rs index c0fcc3de..f258fe1b 100644 --- a/src/write.rs +++ b/src/write.rs @@ -394,8 +394,11 @@ mod tests { #[test] fn terminate_on_drop() { - // Test that dropping the BzEncoder will result in a valid, decompressable datastream - let s = "12345".repeat(100000); + // Test that dropping the BzEncoder flushes bytes to the output, so that + // we get a valid, decompressable datastream + // + // see https://github.com/trifectatechfoundation/bzip2-rs/pull/121 + let s = "12345".repeat(100); let mut compressed = Vec::new(); { @@ -413,7 +416,7 @@ mod tests { d.finish().unwrap() }; assert_eq!(&uncompressed[0..5], b"12834"); - assert_eq!(uncompressed.len(), 500005); + assert_eq!(uncompressed.len(), s.len() + "12834".len()); assert!(format!("12834{}", s).as_bytes() == &*uncompressed); } } From c2e93b161611d7fc6f5c6449220b0d13cb7e5f6a Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Wed, 18 Dec 2024 21:15:33 +0100 Subject: [PATCH 4/5] add `panicked` flag to `BzEncoder` --- src/write.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/write.rs b/src/write.rs index f258fe1b..74a5ae44 100644 --- a/src/write.rs +++ b/src/write.rs @@ -12,6 +12,7 @@ pub struct BzEncoder { obj: Option, buf: Vec, done: bool, + panicked: bool, } /// A compression stream which will have compressed data written to it and @@ -32,17 +33,21 @@ impl BzEncoder { obj: Some(obj), buf: Vec::with_capacity(32 * 1024), done: false, + panicked: false, } } fn dump(&mut self) -> io::Result<()> { while !self.buf.is_empty() { - let n = match self.obj.as_mut().unwrap().write(&self.buf) { - Ok(n) => n, + self.panicked = true; + let r = self.obj.as_mut().unwrap().write(&self.buf); + self.panicked = false; + + match r { + Ok(n) => self.buf.drain(..n), Err(ref err) if err.kind() == io::ErrorKind::Interrupted => continue, Err(err) => return Err(err), }; - self.buf.drain(..n); } Ok(()) } @@ -289,7 +294,7 @@ impl Drop for BzDecoder { impl Drop for BzEncoder { fn drop(&mut self) { - if self.obj.is_some() { + if self.obj.is_some() && !self.panicked { let _ = self.try_finish(); } } From 2353eed7084d413a83372b837f1d178dd4bbda6c Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Wed, 18 Dec 2024 21:17:39 +0100 Subject: [PATCH 5/5] add `panicked` flag to `BzDecoder` --- src/write.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/write.rs b/src/write.rs index 74a5ae44..0f550b47 100644 --- a/src/write.rs +++ b/src/write.rs @@ -22,6 +22,7 @@ pub struct BzDecoder { obj: Option, buf: Vec, done: bool, + panicked: bool, } impl BzEncoder { @@ -167,6 +168,7 @@ impl BzDecoder { obj: Some(obj), buf: Vec::with_capacity(32 * 1024), done: false, + panicked: false, } } @@ -185,12 +187,15 @@ impl BzDecoder { fn dump(&mut self) -> io::Result<()> { while !self.buf.is_empty() { - let n = match self.obj.as_mut().unwrap().write(&self.buf) { - Ok(n) => n, + self.panicked = true; + let r = self.obj.as_mut().unwrap().write(&self.buf); + self.panicked = false; + + match r { + Ok(n) => self.buf.drain(..n), Err(ref err) if err.kind() == io::ErrorKind::Interrupted => continue, Err(err) => return Err(err), }; - self.buf.drain(..n); } Ok(()) }