An overflow panic in Reader::read_byte_record

[xc01]

Summary

csv v1.4.0 can panic in debug builds due to unchecked addition in the read_byte_record call path:

• src/reader.rs:1649
• set_byte(byte + nin as u64)

If byte == u64::MAX and nin > 0, this overflows and panics.

Why this is report-worthy

• This panic is reachable from public APIs (Reader, Position, seek_raw, read_byte_record).
• The function docs do not mention this panic condition.
• The operation currently uses unchecked + instead of checked_add/error return.

Public API reproducer

use std::io::{Cursor, SeekFrom};
use csv::{ByteRecord, Position, Reader};

#[test]
#[should_panic]
fn panic_arithmetic_overflow_read_byte_record_impl_line_1649() {
    let mut rdr = Reader::from_reader(Cursor::new(b"h\nx\n".to_vec()));

    let mut pos = Position::new();
    pos.set_byte(u64::MAX);

    rdr.seek_raw(SeekFrom::Start(0), pos).unwrap();

    let mut record = ByteRecord::new();
    let _ = rdr.read_byte_record(&mut record);
}

Call chain

1. Reader::from_reader(...)
2. Reader::seek_raw(..., pos_with_byte_u64_max)
3. Reader::read_byte_record(...)
4. Internal read_byte_record_impl(...)
5. set_byte(byte + nin as u64) at src/reader.rs:1649 panics when nin > 0

Actual behavior

Panic on integer overflow (debug builds).

Expected behavior

Either:

• avoid panic via checked arithmetic and return an error, or
• explicitly document panic preconditions in API docs.

Suggested fix

At src/reader.rs:1649, replace unchecked addition with checked handling, e.g.:

• byte.checked_add(nin as u64) and map overflow to Error,
• or saturating behavior if that matches crate semantics.

Version

• crate: csv
• version: 1.4.0

[xc01]

There is one more related position-accounting overflow in the same public seek_raw flow. This one is not the byte-position update reported above; it is the record-position update that happens after a record is successfully read.

Panic site:

// src/reader.rs:1882-1884
fn add_record(&mut self, record: &ByteRecord) -> Result<()> {
    let i = self.cur_pos.record();
    self.cur_pos.set_record(i.checked_add(1).unwrap());

Trigger:

use std::io::{Cursor, SeekFrom};

use csv::{ByteRecord, Position, ReaderBuilder};

#[test]
#[should_panic]
fn add_record_panics_after_seek_raw_installs_max_record_position() {
    let mut rdr = ReaderBuilder::new()
        .has_headers(false)
        .from_reader(Cursor::new(b"a\n".to_vec()));

    let mut pos = Position::new();
    pos.set_byte(0).set_line(1).set_record(u64::MAX);

    rdr.seek_raw(SeekFrom::Start(0), pos).unwrap();

    let mut record = ByteRecord::new();
    let _ = rdr.read_byte_record(&mut record);
}

Call path:

Position::set_record(u64::MAX)
Reader::seek_raw(SeekFrom::Start(0), pos)
Reader::read_byte_record(...)
Reader::read_byte_record_impl(...)
ReaderState::add_record(...)
self.cur_pos.set_record(i.checked_add(1).unwrap())

[walker84837]

I'd like to help resolve this issue, but I'd like to check with the intended direction before rushing to open any PR.

I read the crate's existing position-accounting code, and I noticed it already fails loudly on overflow, for example

rust-csv/src/reader.rs

Line 1884 in 05612e8

self.cur_pos.set_record(i.checked_add(1).unwrap());

for the record counter in add_record (as xc01 noted).

The byte-offset update in read_byte_record_impl is the one site using a plain +, which panics in debug but silently wraps in release.

I have a branch ready for a minimal fix on the "document the precondition and make the panic loud and consistent" side:

• add a # Panics note to read_byte_record (and a short note on seek_raw) documenting that a Position with a byte offset at or near u64::MAX can cause a read to panic, and
• use checked_add(...).unwrap() instead, following the existing conventions, so release builds no longer silently wrap

The alternative--turning overflow into a recoverable Error (e.g. via a new ErrorKind variant)--is a larger behavior change I'd want to make only if that's what's preferred.

If the documented & loud-panic direction is good, I'm happy to open the PR.