Skip to content

Skip the whole record after a field size error - #381

Closed
wtn wants to merge 1 commit into
ruby:mainfrom
wtn:quotable
Closed

wtn wants to merge 1 commit into
ruby:mainfrom
wtn:quotable

Conversation

@wtn

@wtn wtn commented Oct 29, 2026

Copy link
Copy Markdown

After a max_field_size error, the field-scanning parser (parse_quotable_robust) skips only to the end of the physical line. If a later quoted field in the record spans lines, the rest of the record comes back as the next row:

require "csv"

csv = CSV.new(%Q{abcde,"x\ny"\nc,d\n}, max_field_size: 4, liberal_parsing: true)
csv.shift rescue nil
csv.line   # => "abcde,\"x\n"
csv.shift  # => ["y\""]

Without liberal_parsing, the field-scanning parser is used after a row with a quoted field that spans lines. There, the rest of the record raises Illegal quoting on the next shift.

Fix

parse_quotable_robust passes a block to validate_field_size, which calls it in place of ignore_broken_line. The block calls the new ignore_broken_record, which reads the remaining fields of the record with parse_column_value and then the row end. CSV#line is then the whole broken record, and the line number goes up by one. The line-splitting parser (parse_quotable_loose) and parse_no_quote are unchanged.

If the rest of the record is malformed, ignore_broken_record drops the rest of the physical line, and the error is still the field size error. An unclosed quote after the long field now reads to the end of the input, as it does without max_field_size. On main, abcde,"x\nc,d\n with liberal_parsing returned ["c", "d"] after the error.

When parse_quotable_robust found a field longer than max_field_size,
it skipped only to the end of the physical line. If a later quoted
field in the record spanned lines, the rest of the record was parsed
as the next row.

Skip the rest of the record with parse_column_value, so the next row
and CSV#line are correct.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@wtn
wtn marked this pull request as ready for review October 29, 2026 21:14
@kou

kou commented Oct 30, 2026

Copy link
Copy Markdown
Member

How about doing this in all parse modes like #382?

@wtn

wtn commented Oct 30, 2026

Copy link
Copy Markdown
Author

Closing in favor of #382.

Thank you.

@wtn wtn closed this Oct 30, 2026
kou added a commit that referenced this pull request Nov 1, 2026
GitHub: Fix GH-381

A max_field_size error skipped only the rest of the physical line. In
parse_quotable_robust, the rest of a record that has a quoted field
spanning lines was returned as the next row or raised another error. In
parse_quotable_loose and parse_no_quote, the line was already read, so
the next line was skipped too, and CSV#line raised TypeError when the
error was in the first row.

Validate field sizes in emit_row instead. All parsers call it after
reading the whole record, so nothing needs to be skipped and CSV#line
returns the whole broken record.

If the record has another error after the long field, such as an
unclosed quoted field, that error is raised instead, as it is without
max_field_size.

The field_size_limit document said that it limits how much CSV reads
ahead and prevents DoS attacks. It wasn't true even with the old line
based parser: it read a whole line without limit and stopped reading
only lines of an unclosed quoted field. The current parser doesn't stop
reading at all. Remove it from the document and add a document for
max_field_size that says it doesn't limit how much CSV reads.

Reported by William T. Nelson. Thanks!!!

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants