diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a1e3cd..c23d892 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `ImapStream` can wrap any kinds of streams, including streams which may become open again later, like files which can be rewinded after reaching end of file or appended to. +### Fixes + +- Update byte-pool to 0.2.4 to fix `ensure_capacity()`. + Previously this bug could have resulted in an attempt to read into a buffer of zero size + and erronous detection of the end of stream. + ## [0.7.0] - 2023-04-03 ### Added diff --git a/Cargo.toml b/Cargo.toml index f5c6b92..af78168 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,7 +33,7 @@ pin-utils = "0.1.0-alpha.4" futures = "0.3.15" ouroboros = "0.15" stop-token = "0.7" -byte-pool = "0.2.2" +byte-pool = "0.2.4" once_cell = "1.8.0" log = "0.4.8" thiserror = "1.0.9" diff --git a/src/imap_stream.rs b/src/imap_stream.rs index a6b94c5..3a29c97 100644 --- a/src/imap_stream.rs +++ b/src/imap_stream.rs @@ -185,6 +185,9 @@ impl Buffer { let increase = std::cmp::max(Buffer::BLOCK_SIZE, extra_bytes_needed); self.grow(increase)?; } + + // Assert that the buffer at least one free byte. + debug_assert!(self.offset < self.block.size()); Ok(()) } @@ -275,6 +278,13 @@ impl Stream for ImapStream { this.buffer.ensure_capacity(this.decode_needs)?; let buf = this.buffer.free_as_mut_slice(); + // The buffer should have at least one byte free + // before we try reading into it + // so we can treat 0 bytes read as EOF. + // This is guaranteed by `ensure_capacity()` above + // even if it is called with 0 as an argument. + debug_assert!(!buf.is_empty()); + #[cfg(feature = "runtime-async-std")] let num_bytes_read = ready!(Pin::new(&mut this.inner).poll_read(cx, buf))?; @@ -394,6 +404,35 @@ mod tests { assert_eq!(buf.block.size(), 3 * Buffer::BLOCK_SIZE); } + /// Regression test for a bug in ensure_capacity() caused + /// by a bug in byte-pool crate 0.2.2 dependency. + /// + /// ensure_capacity() sometimes did not ensure that + /// at least one byte is available, which in turn + /// resulted in attempt to read into a buffer of zero size. + /// When poll_read() reads into a buffer of zero size, + /// it can only read zero bytes, which is indistinguishable + /// from EOF and resulted in an erroneous detection of EOF + /// when in fact the stream was not closed. + #[test] + fn test_ensure_capacity_loop() { + let mut buf = Buffer::new(); + + for i in 1..500 { + // Ask for `i` bytes. + buf.ensure_capacity(i).unwrap(); + + // Test that we can read at least as much as requested. + let free = buf.free_as_mut_slice(); + let used = free.len(); + assert!(used >= i); + drop(free); + + // Use as much as allowed. + buf.extend_used(used); + } + } + #[test] fn test_buffer_take_and_return_block() { // This test identifies blocks by their size.