Skip to content

Commit b83fac5

Browse files
committed
Ignore quotes when detecting the row separator
`:auto` row separator detection scanned the sample for the first CR/LF without tracking quote state, so a CR inside a quoted field was mistaken for the row separator. As a result CSV could not parse its own output: `CSV.parse(CSV.generate)` corrupted or crashed on any field containing a carriage return. For example `["a\rb", "x"]` round-tripped to `["a\rb", "x\n"]`, and fields with `\r\n` or a lone `\r` raised CSV::MalformedCSVError. Make detection skip CR/LF that fall inside a quoted field. Unquoted CR is still honored as a separator, so old Mac line endings keep working.
1 parent 0873ab3 commit b83fac5

2 files changed

Lines changed: 47 additions & 3 deletions

File tree

‎lib/csv/parser.rb‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -731,11 +731,11 @@ def resolve_row_separator(separator)
731731
end
732732

733733
def detect_row_separator(sample, cr, lf)
734-
lf_index = sample.index(lf)
734+
lf_index = unquoted_index(sample, lf)
735735
if lf_index
736-
cr_index = sample[0, lf_index].index(cr)
736+
cr_index = unquoted_index(sample[0, lf_index], cr)
737737
else
738-
cr_index = sample.index(cr)
738+
cr_index = unquoted_index(sample, cr)
739739
end
740740
if cr_index and lf_index
741741
if cr_index + 1 == lf_index
@@ -754,6 +754,21 @@ def detect_row_separator(sample, cr, lf)
754754
end
755755
end
756756

757+
# Index of the first +target+ that is outside a quoted field, so a CR or
758+
# LF inside quotes is not mistaken for the row separator.
759+
def unquoted_index(sample, target)
760+
return sample.index(target) if @quote_character.nil?
761+
in_quote = false
762+
sample.each_char.with_index do |char, i|
763+
if char == @quote_character
764+
in_quote = !in_quote
765+
elsif char == target and not in_quote
766+
return i
767+
end
768+
end
769+
nil
770+
end
771+
757772
def prepare_line
758773
@lineno = 0
759774
@last_line = nil

‎test/csv/parse/test_row_separator.rb‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,33 @@ def test_multiple_characters
1313
CSV.parse("a\r\nb\r\n", row_sep: "\r\n"))
1414
end
1515
end
16+
17+
# :auto detection must ignore CR/LF inside a quoted field.
18+
def test_auto_quoted_cr
19+
assert_equal([["a\rb", "x"]],
20+
CSV.parse("\"a\rb\",x\n"))
21+
end
22+
23+
def test_auto_quoted_cr_lf
24+
assert_equal([["a\r\nb", "x"]],
25+
CSV.parse("\"a\r\nb\",x\n"))
26+
end
27+
28+
def test_auto_quoted_cr_only_field
29+
assert_equal([["\r"]],
30+
CSV.parse("\"\r\"\n"))
31+
end
32+
33+
# An unquoted CR is still a valid (old Mac) row separator.
34+
def test_auto_unquoted_cr_still_detected
35+
assert_equal([["a"], ["b"]],
36+
CSV.parse("a\rb\r"))
37+
end
38+
39+
# CSV must be able to parse its own generated output.
40+
def test_auto_round_trip_cr_fields
41+
rows = [["a\rb", "x"], ["\r", "y"], ["c", "d\r\ne"]]
42+
generated = CSV.generate {|csv| rows.each {|row| csv << row}}
43+
assert_equal(rows, CSV.parse(generated))
44+
end
1645
end

0 commit comments

Comments
 (0)