Skip to content

Commit 15a5dfa

Browse files
committed
Auto merge of #58913 - Milack27:patch_buf_reader, r=joshtriplett
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.
2 parents 20958fc + c36d91c commit 15a5dfa

File tree

1 file changed

+45
-2
lines changed

1 file changed

+45
-2
lines changed

src/libstd/io/buffered.rs

+45-2
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,13 @@ impl<R> BufReader<R> {
193193
/// ```
194194
#[stable(feature = "rust1", since = "1.0.0")]
195195
pub fn into_inner(self) -> R { self.inner }
196+
197+
/// Invalidates all data in the internal buffer.
198+
#[inline]
199+
fn discard_buffer(&mut self) {
200+
self.pos = 0;
201+
self.cap = 0;
202+
}
196203
}
197204

198205
impl<R: Seek> BufReader<R> {
@@ -227,6 +234,7 @@ impl<R: Read> Read for BufReader<R> {
227234
// (larger than our internal buffer), bypass our internal buffer
228235
// entirely.
229236
if self.pos == self.cap && buf.len() >= self.buf.len() {
237+
self.discard_buffer();
230238
return self.inner.read(buf);
231239
}
232240
let nread = {
@@ -240,6 +248,7 @@ impl<R: Read> Read for BufReader<R> {
240248
fn read_vectored(&mut self, bufs: &mut [IoVecMut<'_>]) -> io::Result<usize> {
241249
let total_len = bufs.iter().map(|b| b.len()).sum::<usize>();
242250
if self.pos == self.cap && total_len >= self.buf.len() {
251+
self.discard_buffer();
243252
return self.inner.read_vectored(bufs);
244253
}
245254
let nread = {
@@ -325,14 +334,14 @@ impl<R: Seek> Seek for BufReader<R> {
325334
} else {
326335
// seek backwards by our remainder, and then by the offset
327336
self.inner.seek(SeekFrom::Current(-remainder))?;
328-
self.pos = self.cap; // empty the buffer
337+
self.discard_buffer();
329338
result = self.inner.seek(SeekFrom::Current(n))?;
330339
}
331340
} else {
332341
// Seeking with Start/End doesn't care about our buffer length.
333342
result = self.inner.seek(pos)?;
334343
}
335-
self.pos = self.cap; // empty the buffer
344+
self.discard_buffer();
336345
Ok(result)
337346
}
338347
}
@@ -1068,6 +1077,40 @@ mod tests {
10681077
assert_eq!(reader.fill_buf().ok(), Some(&[2, 3][..]));
10691078
}
10701079

1080+
#[test]
1081+
fn test_buffered_reader_invalidated_after_read() {
1082+
let inner: &[u8] = &[5, 6, 7, 0, 1, 2, 3, 4];
1083+
let mut reader = BufReader::with_capacity(3, io::Cursor::new(inner));
1084+
1085+
assert_eq!(reader.fill_buf().ok(), Some(&[5, 6, 7][..]));
1086+
reader.consume(3);
1087+
1088+
let mut buffer = [0, 0, 0, 0, 0];
1089+
assert_eq!(reader.read(&mut buffer).ok(), Some(5));
1090+
assert_eq!(buffer, [0, 1, 2, 3, 4]);
1091+
1092+
assert!(reader.seek_relative(-2).is_ok());
1093+
let mut buffer = [0, 0];
1094+
assert_eq!(reader.read(&mut buffer).ok(), Some(2));
1095+
assert_eq!(buffer, [3, 4]);
1096+
}
1097+
1098+
#[test]
1099+
fn test_buffered_reader_invalidated_after_seek() {
1100+
let inner: &[u8] = &[5, 6, 7, 0, 1, 2, 3, 4];
1101+
let mut reader = BufReader::with_capacity(3, io::Cursor::new(inner));
1102+
1103+
assert_eq!(reader.fill_buf().ok(), Some(&[5, 6, 7][..]));
1104+
reader.consume(3);
1105+
1106+
assert!(reader.seek(SeekFrom::Current(5)).is_ok());
1107+
1108+
assert!(reader.seek_relative(-2).is_ok());
1109+
let mut buffer = [0, 0];
1110+
assert_eq!(reader.read(&mut buffer).ok(), Some(2));
1111+
assert_eq!(buffer, [3, 4]);
1112+
}
1113+
10711114
#[test]
10721115
fn test_buffered_reader_seek_underflow() {
10731116
// gimmick reader that yields its position modulo 256 for each byte

0 commit comments

Comments
 (0)