BurntSushi / BurntSushi/rust-csv

`Position::line` reports incorrect line when input has CRLF line endings

Open
#395 3 comments 2 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
2k
Forks
257
PR merge metrics
No merged PRs in 30d

Description

Given `reader: csv::Reader`, when `reader.read_record` returns, `reader.position().line()` should return the line number where the next record begins.

When the input has a single-byte record separator like `\n`, that's exactly what it does. However, when the input uses `\r\n` line endings, the *last* record's line number is reported instead.

---

This test demonstrates the issue:

```rust
#[test]
fn test_crlf_line_numbers() {
let input = std::io::Cursor::new(b"foo,bar\r\none,two\r\n");

let mut reader =
csv::ReaderBuilder::new()
.terminator(csv::Terminator::CRLF)
.has_headers(false)
.from_reader(input);

assert_eq!(reader.position().line(), 1);

reader.read_record(&mut csv::StringRecord::new()).unwrap();

assert_eq!(reader.position().line(), 2);
}
```

This test fails at the final `assert_eq!` because `reader.position().line()` still returns 1. If, however, the `\r`s are removed from the `input`, this test passes.

---

The problem seems to be that `Reader::read_record` consumes only the first byte of the terminator, but `line` is only incremented [here](https://github.com/BurntSushi/rust-csv/blob/f973cd49a91e5f71d063b45ffeb469765c4f5a86/csv-core/src/reader.rs#L670) when the parser consumes a `\n` byte. If the terminator is `\r\n`, then it only consumes `\n` and increments the line number when it starts parsing the *next* record.

For example, if you add the following test to the `tests` module in `csv-core/src/reader.rs`:

```rust
#[test]
fn stream_record_with_crlf() {
use crate::ReadRecordResult::*;

let mut inp = b("foo,bar\r\nbaz,\r\n3,4\r\n");
let out = &mut [0; 1024];
let ends = &mut [0; 10];
let mut rdr = ReaderBuilder::new().terminator(Terminator::CRLF).build();

assert_eq!(rdr.line(), 1);

assert_read_record!(rdr, &inp, out, ends, 9, 6, 2, Record);
assert_eq!(ends[0], 3);
assert_eq!(&out[0..3], b"foo");
assert_eq!(ends[1], 6);
assert_eq!(&out[3..6], b"bar");
assert_eq!(rdr.line(), 1);
inp = &inp[9..];

assert_read_record!(rdr, &inp, out, ends, 6, 3, 1, Record);
assert_eq!(ends[0], 3);
assert_eq!(&out[0..3], b"baz");
assert_eq!(rdr.line(), 2);
inp = &inp[6..];

assert_read_record!(rdr, &inp, out, ends, 5, 2, 2, Record);
assert_eq!(ends[0], 1);
assert_eq!(&out[0..1], b"3");
assert_eq!(ends[1], 2);
assert_eq!(&out[1..2], b"4");
assert_eq!(rdr.line(), 3);
inp = &inp[5..];

assert_read_record!(rdr, &inp, out, ends, 0, 0, 0, End);
}
```

The test will fail at the first `assert_read_record!` because the parser has only consumed 8 bytes, up to the first `\r`. For it to consume a `\n` and increment the line number, it would need to have consumed 9 bytes there.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.