Skip to content

Commit

Permalink
Rollup merge of rust-lang#58913 - Milack27:patch_buf_reader, r=joshtr…
Browse files Browse the repository at this point in the history
…iplett

Add new test case for possible bug in BufReader

When reading a large chunk from a BufReader, if all the bytes from the buffer have been already consumed, the internal buffer is bypassed entirely. However, it is not invalidated, and it's possible to access its contents using the `seek_relative` method, because it tries to reuse the existing buffer.
  • Loading branch information
Centril committed Mar 19, 2019
2 parents fdb2ecd + c36d91c commit 0329e03
Showing 1 changed file with 45 additions and 2 deletions.
47 changes: 45 additions & 2 deletions src/libstd/io/buffered.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,13 @@ impl<R> BufReader<R> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn into_inner(self) -> R { self.inner }

/// Invalidates all data in the internal buffer.
#[inline]
fn discard_buffer(&mut self) {
self.pos = 0;
self.cap = 0;
}
}

impl<R: Seek> BufReader<R> {
Expand Down Expand Up @@ -227,6 +234,7 @@ impl<R: Read> Read for BufReader<R> {
// (larger than our internal buffer), bypass our internal buffer
// entirely.
if self.pos == self.cap && buf.len() >= self.buf.len() {
self.discard_buffer();
return self.inner.read(buf);
}
let nread = {
Expand All @@ -240,6 +248,7 @@ impl<R: Read> Read for BufReader<R> {
fn read_vectored(&mut self, bufs: &mut [IoVecMut<'_>]) -> io::Result<usize> {
let total_len = bufs.iter().map(|b| b.len()).sum::<usize>();
if self.pos == self.cap && total_len >= self.buf.len() {
self.discard_buffer();
return self.inner.read_vectored(bufs);
}
let nread = {
Expand Down Expand Up @@ -325,14 +334,14 @@ impl<R: Seek> Seek for BufReader<R> {
} else {
// seek backwards by our remainder, and then by the offset
self.inner.seek(SeekFrom::Current(-remainder))?;
self.pos = self.cap; // empty the buffer
self.discard_buffer();
result = self.inner.seek(SeekFrom::Current(n))?;
}
} else {
// Seeking with Start/End doesn't care about our buffer length.
result = self.inner.seek(pos)?;
}
self.pos = self.cap; // empty the buffer
self.discard_buffer();
Ok(result)
}
}
Expand Down Expand Up @@ -1068,6 +1077,40 @@ mod tests {
assert_eq!(reader.fill_buf().ok(), Some(&[2, 3][..]));
}

#[test]
fn test_buffered_reader_invalidated_after_read() {
let inner: &[u8] = &[5, 6, 7, 0, 1, 2, 3, 4];
let mut reader = BufReader::with_capacity(3, io::Cursor::new(inner));

assert_eq!(reader.fill_buf().ok(), Some(&[5, 6, 7][..]));
reader.consume(3);

let mut buffer = [0, 0, 0, 0, 0];
assert_eq!(reader.read(&mut buffer).ok(), Some(5));
assert_eq!(buffer, [0, 1, 2, 3, 4]);

assert!(reader.seek_relative(-2).is_ok());
let mut buffer = [0, 0];
assert_eq!(reader.read(&mut buffer).ok(), Some(2));
assert_eq!(buffer, [3, 4]);
}

#[test]
fn test_buffered_reader_invalidated_after_seek() {
let inner: &[u8] = &[5, 6, 7, 0, 1, 2, 3, 4];
let mut reader = BufReader::with_capacity(3, io::Cursor::new(inner));

assert_eq!(reader.fill_buf().ok(), Some(&[5, 6, 7][..]));
reader.consume(3);

assert!(reader.seek(SeekFrom::Current(5)).is_ok());

assert!(reader.seek_relative(-2).is_ok());
let mut buffer = [0, 0];
assert_eq!(reader.read(&mut buffer).ok(), Some(2));
assert_eq!(buffer, [3, 4]);
}

#[test]
fn test_buffered_reader_seek_underflow() {
// gimmick reader that yields its position modulo 256 for each byte
Expand Down

0 comments on commit 0329e03

Please sign in to comment.