Skip to content

Commit 96e361f

Browse files
andre-vmAndré Vicente Milack
authored and
André Vicente Milack
committed
Fix buffer invalidation for BufRead
There are two moments when a BufRead object needs to empty it's internal buffer: - In a seek call; - In a read call when all data in the internal buffer had been already consumed and the output buffer has a greater or equal size than the internal buffer. In both cases, the buffer was not being properly emptied, but only marked as consumed (self.pos = self.cap). That should be no problem if the inner reader is only Read, but if it is Seek as well, then it's possible to access the data in the buffer by using the seek_relative method. In order to prevent this from happening, both self.pos and self.cap should be set to 0. Two test cases were added to detect that failure: - test_buffered_reader_invalidated_after_read - test_buffered_reader_invalidated_after_seek Both tests are very similar to each other. The inner reader contains the following data: [5, 6, 7, 0, 1, 2, 3, 4]. The buffer capacity is 3 bytes. - First, we call fill_buffer, which loads [5, 6, 7] into the internal buffer, and then consume those 3 bytes. - Then we either read the 5 remaining bytes in a single read call or we move to the end of the stream by calling seek. In both cases the buffer should be emptied to prevent the previous data [5, 6, 7] from being read. - We now call seek_relative(-2) and read two bytes, which should give us the last 2 bytes of the stream: [3, 4]. Before this commit, the the seek_relative method would consider that we're still in the range of the internal buffer, so instead of fetching data from the inner reader, it would return the two last bytes that were incorrectly still in the buffer: [6, 7]. Therefore, the test would fail. Now, when seek_relative is called the buffer is empty. So the expected data [3, 4] is fetched from the inner reader and the test passes.
1 parent a9da8fc commit 96e361f

File tree

1 file changed

+43
-2
lines changed

1 file changed

+43
-2
lines changed

src/libstd/io/buffered.rs

+43-2
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,9 @@ impl<R: Read> Read for BufReader<R> {
225225
// (larger than our internal buffer), bypass our internal buffer
226226
// entirely.
227227
if self.pos == self.cap && buf.len() >= self.buf.len() {
228+
// Empty the buffer
229+
self.cap = 0;
230+
self.pos = 0;
228231
return self.inner.read(buf);
229232
}
230233
let nread = {
@@ -323,14 +326,18 @@ impl<R: Seek> Seek for BufReader<R> {
323326
} else {
324327
// seek backwards by our remainder, and then by the offset
325328
self.inner.seek(SeekFrom::Current(-remainder))?;
326-
self.pos = self.cap; // empty the buffer
329+
// Empty the buffer
330+
self.cap = 0;
331+
self.pos = 0;
327332
result = self.inner.seek(SeekFrom::Current(n))?;
328333
}
329334
} else {
330335
// Seeking with Start/End doesn't care about our buffer length.
331336
result = self.inner.seek(pos)?;
332337
}
333-
self.pos = self.cap; // empty the buffer
338+
// Empty the buffer
339+
self.cap = 0;
340+
self.pos = 0;
334341
Ok(result)
335342
}
336343
}
@@ -1066,6 +1073,40 @@ mod tests {
10661073
assert_eq!(reader.fill_buf().ok(), Some(&[2, 3][..]));
10671074
}
10681075

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

0 commit comments

Comments
 (0)