Skip to content

Commit

Permalink
Avoid running into infinite loops when recovering from Reader errors
Browse files Browse the repository at this point in the history
Resolves BurntSushi#207

When reading and parsing a record fails, it's correct behaviour
in most cases to keep on trying to read and parse the next record.

However when the Error is caused by failing to read from the underlying
`impl Read`, the next read would almost always fail with the exact
same error.
In that scenario, reading from the underlying `impl Read` should not be
attempted when trying to deserialize the next record, as this may lead
to infinite loops.
Instead, the `Reader` will behave in the same way as if an end-of-file
had been reached.
  • Loading branch information
Ten0 committed Nov 2, 2020
1 parent f0f53a4 commit 065f263
Showing 1 changed file with 39 additions and 8 deletions.
47 changes: 39 additions & 8 deletions src/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,10 @@ impl ReaderBuilder {
/// [`ReaderBuilder`](struct.ReaderBuilder.html).
/// * When reading CSV data from a resource (like a file), it is possible for
/// reading from the underlying resource to fail. This will return an error.
/// For subsequent calls to the `Reader` after encountering a such error
/// (unless `seek` is used), it will behave as if end of file had been reached,
/// in order to avoid running into infinite loops when still attempting to read
/// the next record when one has errored.
/// * When reading CSV data into `String` or `&str` fields (e.g., via a
/// [`StringRecord`](struct.StringRecord.html)), UTF-8 is strictly
/// enforced. If CSV data is invalid UTF-8, then an error is returned. If
Expand Down Expand Up @@ -741,7 +745,30 @@ struct ReaderState {
/// Whether the reader has been seeked or not.
seeked: bool,
/// Whether EOF of the underlying reader has been reached or not.
eof: bool,
///
/// IO errors on the underlying reader will be considered as an EOF for subsequent read attempts,
/// as it would be incorrect to keep on trying to read when the underlying reader has broken.
///
/// For clarity, having the best `Debug` impl and in case they need to be treated differently at
/// some point, we store whether the `EOF` is considered because an actual EOF happened, or
/// because we encoundered an IO error.
/// This has no additional runtime cost.
eof: ReaderEofState,
}

/// Whether EOF of the underlying reader has been reached or not.
///
/// IO errors on the underlying reader will be considered as an EOF for subsequent read attempts,
/// as it would be incorrect to keep on trying to read when the underlying reader has broken.
///
/// For clarity, having the best `Debug` impl and in case they need to be treated differently at
/// some point, we store whether the `EOF` is considered because an actual EOF happened, or
/// because we encoundered an IO error.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum ReaderEofState {
NotEof,
Eof,
IOError,
}

/// Headers encapsulates any data associated with the headers of CSV data.
Expand Down Expand Up @@ -798,7 +825,7 @@ impl<R: io::Read> Reader<R> {
cur_pos: Position::new(),
first: false,
seeked: false,
eof: false,
eof: ReaderEofState::NotEof,
},
}
}
Expand Down Expand Up @@ -1598,13 +1625,17 @@ impl<R: io::Read> Reader<R> {

record.clear();
record.set_position(Some(self.state.cur_pos.clone()));
if self.state.eof {
if self.state.eof != ReaderEofState::NotEof {
return Ok(false);
}
let (mut outlen, mut endlen) = (0, 0);
loop {
let (res, nin, nout, nend) = {
let input = self.rdr.fill_buf()?;
let input_res = self.rdr.fill_buf();
if input_res.is_err() {
self.state.eof = ReaderEofState::IOError;
}
let input = input_res?;
let (fields, ends) = record.as_parts();
self.core.read_record(
input,
Expand Down Expand Up @@ -1636,7 +1667,7 @@ impl<R: io::Read> Reader<R> {
return Ok(true);
}
End => {
self.state.eof = true;
self.state.eof = ReaderEofState::Eof;
return Ok(false);
}
}
Expand Down Expand Up @@ -1716,7 +1747,7 @@ impl<R: io::Read> Reader<R> {
/// }
/// ```
pub fn is_done(&self) -> bool {
self.state.eof
self.state.eof != ReaderEofState::NotEof
}

/// Returns true if and only if this reader has been configured to
Expand Down Expand Up @@ -1817,7 +1848,7 @@ impl<R: io::Read + io::Seek> Reader<R> {
self.core.reset();
self.core.set_line(pos.line());
self.state.cur_pos = pos;
self.state.eof = false;
self.state.eof = ReaderEofState::NotEof;
Ok(())
}

Expand Down Expand Up @@ -1845,7 +1876,7 @@ impl<R: io::Read + io::Seek> Reader<R> {
self.core.reset();
self.core.set_line(pos.line());
self.state.cur_pos = pos;
self.state.eof = false;
self.state.eof = ReaderEofState::NotEof;
Ok(())
}
}
Expand Down

0 comments on commit 065f263

Please sign in to comment.