Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

write::BzDecoder: Fix infinite loop on drop when no data is read or written #118

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions src/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,13 @@ impl<W: Write> BzDecoder<W> {
/// [`write`]: Self::write
pub fn try_finish(&mut self) -> io::Result<()> {
while !self.done {
let _ = self.write(&[])?;
// The write is effectively a `self.flush()`, but we want to know how many
// bytes were written. exit if no input was read and no output was written
if self.write(&[])? == 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the self.total_in() == read_before check here because on self.write(&[])? there is no way (unless I'm really missing something) that total_in could change: that empty input buffer is handed to decompress_vec, which ultimately calls BZ2_bzDecompress with an empty input buffer. total_in will not change in that case, how could it?

// finishing the output stream is effectively EOF of the input
let msg = "Input EOF reached before logical stream end";
return Err(io::Error::new(io::ErrorKind::UnexpectedEof, msg));
}
}
self.dump()
}
Expand Down Expand Up @@ -303,14 +309,39 @@ mod tests {
}

#[test]
fn write_empty() {
fn roundtrip_empty() {
// this encodes and then decodes an empty input file
let d = BzDecoder::new(Vec::new());
let mut c = BzEncoder::new(d, Compression::default());
let _ = c.write(b"").unwrap();
let data = c.finish().unwrap().finish().unwrap();
assert_eq!(&data[..], b"");
}

#[test]
fn finish_empty_explicit() {
// The empty sequence is not a valid .bzip2 file!
// A valid file at least includes the magic bytes, the checksum, etc.
//
// This used to loop infinitely, see
//
// - https://github.com/trifectatechfoundation/bzip2-rs/issues/96
// - https://github.com/trifectatechfoundation/bzip2-rs/pull/97
let mut d = BzDecoder::new(Vec::new());
d.write(b"").unwrap();
let e = d.finish().unwrap_err();
assert_eq!(e.kind(), std::io::ErrorKind::UnexpectedEof);
}

#[test]
fn finish_empty_drop() {
// the drop implementation used to loop infinitely for empty input
//
// see https://github.com/trifectatechfoundation/bzip2-rs/pull/118
let d = BzDecoder::new(Vec::new());
drop(d);
}

#[test]
fn write_invalid() {
// see https://github.com/trifectatechfoundation/bzip2-rs/issues/98
Expand Down
Loading