Skip to content

Commit

Permalink
Fix issues with AsyncBufRead::read_line and AsyncBufReadExt::lines`
Browse files Browse the repository at this point in the history
Fixes the following issues in `AsyncBufRead::read_line`:
* When the future is dropped the previous string contents are not restored so the string is empty.
* If invalid UTF-8 is encountered the previous string contents are not restored.
* If an IO error occurs after `read_until_internal` already read a couple of bytes a debug assertion fails.
* Performance: The whole string to which read contents are appended is check for UTF-8 validity instead of just the added bytes.

* Fixes the following issues in `AsyncBufRead::read_line`
* If an IO error occurs after `read_until_internal` already read a couple of bytes a debug assertion fails. (#2862)

Fixes #2862.
  • Loading branch information
hkratz committed Sep 14, 2024
1 parent fd71f88 commit dc4720b
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 17 deletions.
1 change: 1 addition & 0 deletions futures-util/src/io/lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ impl<R: AsyncBufRead> Stream for Lines<R> {
fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
let this = self.project();
let n = ready!(read_line_internal(this.reader, cx, this.buf, this.bytes, this.read))?;
*this.read = 0;
if n == 0 && this.buf.is_empty() {
return Poll::Ready(None);
}
Expand Down
47 changes: 32 additions & 15 deletions futures-util/src/io/read_line.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ pub struct ReadLine<'a, R: ?Sized> {
buf: &'a mut String,
bytes: Vec<u8>,
read: usize,
finished: bool,
}

impl<R: ?Sized + Unpin> Unpin for ReadLine<'_, R> {}

impl<'a, R: AsyncBufRead + ?Sized + Unpin> ReadLine<'a, R> {
pub(super) fn new(reader: &'a mut R, buf: &'a mut String) -> Self {
Self { reader, bytes: mem::take(buf).into_bytes(), buf, read: 0 }
Self { reader, bytes: mem::take(buf).into_bytes(), buf, read: 0, finished: false }
}
}

Expand All @@ -35,26 +36,42 @@ pub(super) fn read_line_internal<R: AsyncBufRead + ?Sized>(
bytes: &mut Vec<u8>,
read: &mut usize,
) -> Poll<io::Result<usize>> {
let ret = ready!(read_until_internal(reader, cx, b'\n', bytes, read));
if str::from_utf8(bytes).is_err() {
bytes.clear();
Poll::Ready(ret.and_then(|_| {
Err(io::Error::new(io::ErrorKind::InvalidData, "stream did not contain valid UTF-8"))
}))
} else {
debug_assert!(buf.is_empty());
debug_assert_eq!(*read, 0);
// Safety: `bytes` is a valid UTF-8 because `str::from_utf8` returned `Ok`.
mem::swap(unsafe { buf.as_mut_vec() }, bytes);
Poll::Ready(ret)
let mut ret = ready!(read_until_internal(reader, cx, b'\n', bytes, read));
if str::from_utf8(&bytes[bytes.len() - *read..bytes.len()]).is_err() {
bytes.truncate(bytes.len() - *read);
if ret.is_ok() {
ret = Err(io::Error::new(
io::ErrorKind::InvalidData,
"stream did not contain valid UTF-8",
));
}
}
*read = 0;
// Safety: `bytes` is valid UTF-8 because it was taken from a String
// and the newly read bytes are either valid UTF-8 or have been removed.
mem::swap(unsafe { buf.as_mut_vec() }, bytes);
Poll::Ready(ret)
}

impl<R: AsyncBufRead + ?Sized + Unpin> Future for ReadLine<'_, R> {
type Output = io::Result<usize>;

fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
let Self { reader, buf, bytes, read } = &mut *self;
read_line_internal(Pin::new(reader), cx, buf, bytes, read)
let Self { reader, buf, bytes, read, finished: _ } = &mut *self;
let ret = ready!(read_line_internal(Pin::new(reader), cx, buf, bytes, read));
self.finished = true;
Poll::Ready(ret)
}
}

impl<R: ?Sized> Drop for ReadLine<'_, R> {
fn drop(&mut self) {
// restore old string contents
if !self.finished {
self.bytes.truncate(self.bytes.len() - self.read);
// Safety: `bytes` is valid UTF-8 because it was taken from a String
// and the newly read bytes have been removed.
mem::swap(unsafe { self.buf.as_mut_vec() }, &mut self.bytes);
}
}
}
3 changes: 1 addition & 2 deletions futures-util/src/io/read_until.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use futures_core::ready;
use futures_core::task::{Context, Poll};
use futures_io::AsyncBufRead;
use std::io;
use std::mem;
use std::pin::Pin;
use std::vec::Vec;

Expand Down Expand Up @@ -46,7 +45,7 @@ pub(super) fn read_until_internal<R: AsyncBufRead + ?Sized>(
reader.as_mut().consume(used);
*read += used;
if done || used == 0 {
return Poll::Ready(Ok(mem::replace(read, 0)));
return Poll::Ready(Ok(*read));
}
}
}
Expand Down

0 comments on commit dc4720b

Please sign in to comment.