From 88517a369d6885b8ee8d8805897ca1624aa47441 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 30 Oct 2026 14:58:51 +0900 Subject: [PATCH] Validate field size after reading the whole record 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 --- doc/csv/options/parsing/field_size_limit.rdoc | 22 +++--- doc/csv/options/parsing/max_field_size.rdoc | 50 +++++++++++++ lib/csv.rb | 2 + lib/csv/parser.rb | 14 ++-- test/csv/parse/test_general.rb | 72 +++++++++++++++++++ 5 files changed, 138 insertions(+), 22 deletions(-) create mode 100644 doc/csv/options/parsing/max_field_size.rdoc diff --git a/doc/csv/options/parsing/field_size_limit.rdoc b/doc/csv/options/parsing/field_size_limit.rdoc index 797c5776..a8934ae3 100644 --- a/doc/csv/options/parsing/field_size_limit.rdoc +++ b/doc/csv/options/parsing/field_size_limit.rdoc @@ -1,18 +1,14 @@ ====== Option +field_size_limit+ Specifies the \Integer field size limit. +This is the maximum field size + 1 allowed. + +Deprecated since 3.2.3. Use {max_field_size}[#class-CSV-label-Option+max_field_size] instead. +field_size_limit: n is the same as max_field_size: n - 1. Default value: CSV::DEFAULT_OPTIONS.fetch(:field_size_limit) # => nil -This is a maximum size CSV will read ahead looking for the closing quote for a field. -(In truth, it reads to the first line ending beyond this size.) -If a quote cannot be found within the limit CSV will raise a MalformedCSVError, -assuming the data is faulty. -You can use this limit to prevent what are effectively DoS attacks on the parser. -However, this limit can cause a legitimate parse to fail; -therefore the default value is +nil+ (no limit). - For the examples in this section: str = <<~EOT "a","b" @@ -26,14 +22,12 @@ Using the default +nil+: ary = CSV.parse(str) ary # => [["a", "b"], ["\n2345\n", ""]] -Using 50: - field_size_limit = 50 - ary = CSV.parse(str, field_size_limit: field_size_limit) +Using 7: + ary = CSV.parse(str, field_size_limit: 7) ary # => [["a", "b"], ["\n2345\n", ""]] --- Raises an exception if a field is too long: - big_str = "123456789\n" * 1024 - # Raises CSV::MalformedCSVError (Field size exceeded in line 1.) - CSV.parse('valid,fields,"' + big_str + '"', field_size_limit: 2048) + # Raises CSV::MalformedCSVError (Field size exceeded: 6 > 5 in line 2.) + CSV.parse(str, field_size_limit: 6) diff --git a/doc/csv/options/parsing/max_field_size.rdoc b/doc/csv/options/parsing/max_field_size.rdoc new file mode 100644 index 00000000..cad03e0a --- /dev/null +++ b/doc/csv/options/parsing/max_field_size.rdoc @@ -0,0 +1,50 @@ +====== Option +max_field_size+ + +Specifies the \Integer maximum field size in characters. + +Default value: + CSV::DEFAULT_OPTIONS.fetch(:max_field_size) # => nil + +If a field is longer than this size, +CSV raises a MalformedCSVError for the record. +The size is checked after the whole record is read. +You can continue to read the next record after the error; +the record that has the long field is skipped. + +This doesn't limit how much data CSV reads. +For example, CSV reads the whole record that has a long field, +and an unclosed quoted field is read to the end of the input. +If you need to limit memory usage, limit the input size yourself. + +For examples in this section: + str = <<~EOT + a,b + abcde,"x + y" + c,d + EOT + str # => "a,b\nabcde,\"x\ny\"\nc,d\n" + +Using the default +nil+: + ary = CSV.parse(str) + ary # => [["a", "b"], ["abcde", "x\ny"], ["c", "d"]] + +Using 5: + ary = CSV.parse(str, max_field_size: 5) + ary # => [["a", "b"], ["abcde", "x\ny"], ["c", "d"]] + +--- + +Raises an exception if a field is too long: + # Raises CSV::MalformedCSVError (Field size exceeded: 5 > 4 in line 2.) + CSV.parse(str, max_field_size: 4) + +Continues to read the next record after the exception: + csv = CSV.new(str, max_field_size: 4) + csv.shift # => ["a", "b"] + begin + csv.shift + rescue CSV::MalformedCSVError + end + csv.line # => "abcde,\"x\ny\"\n" + csv.shift # => ["c", "d"] diff --git a/lib/csv.rb b/lib/csv.rb index b1f1b80c..8258eef9 100644 --- a/lib/csv.rb +++ b/lib/csv.rb @@ -374,6 +374,8 @@ # # :include: ../doc/csv/options/parsing/field_size_limit.rdoc # +# :include: ../doc/csv/options/parsing/max_field_size.rdoc +# # :include: ../doc/csv/options/parsing/converters.rdoc # # :include: ../doc/csv/options/parsing/unconverted_fields.rdoc diff --git a/lib/csv/parser.rb b/lib/csv/parser.rb index 9b8a009b..f14be3af 100644 --- a/lib/csv/parser.rb +++ b/lib/csv/parser.rb @@ -942,7 +942,6 @@ def skip_line?(line) def validate_field_size(field) return unless @max_field_size return if field.size <= @max_field_size - ignore_broken_line message = "Field size exceeded: #{field.size} > #{@max_field_size}" raise MalformedCSVError.new(message, @lineno) end @@ -959,11 +958,6 @@ def parse_no_quote(&block) else line = strip_value(line) row = line.split(@split_column_separator, -1) - if @max_field_size - row.each do |column| - validate_field_size(column) - end - end n_columns = row.size i = 0 while i < n_columns @@ -1028,7 +1022,6 @@ def parse_quotable_loose(&block) @parse_method = :parse_quotable_robust return parse_quotable_robust(&block) end - validate_field_size(row[i]) end i += 1 end @@ -1055,7 +1048,6 @@ def parse_quotable_robust(&block) value = parse_column_value if value @scanner.scan_all(@strip_value) if @strip_value - validate_field_size(value) end if parse_column_end row << value @@ -1276,6 +1268,12 @@ def start_row def emit_row(row, quoted_fields=FieldsConverter::NO_QUOTED_FIELDS, &block) @lineno += 1 + if @max_field_size + row.each do |field| + validate_field_size(field) if field + end + end + raw_row = row if @use_headers if @headers.nil? diff --git a/test/csv/parse/test_general.rb b/test/csv/parse/test_general.rb index 520eb8f0..d09fb96f 100644 --- a/test/csv/parse/test_general.rb +++ b/test/csv/parse/test_general.rb @@ -329,6 +329,70 @@ def test_max_field_size_in_extended_column_exceeding assert_parse_errors_out(data, max_field_size: 4) end + def test_max_field_size_recover_no_quote + assert_equal([ + "Field size exceeded: 5 > 4 in line 1.", + "abcde,b\n", + ["c", "d"], + 2, + ], + parse_after_error("abcde,b\nc,d\n", + max_field_size: 4, + quote_char: nil)) + end + + def test_max_field_size_recover_quotable_loose + assert_equal([ + "Field size exceeded: 5 > 4 in line 1.", + "abcde,b\n", + ["c", "d"], + 2, + ], + parse_after_error("abcde,b\nc,d\n", max_field_size: 4)) + end + + def test_max_field_size_recover_multiline_record + assert_equal([ + "Field size exceeded: 5 > 4 in line 1.", + %Q{abcde,"x\ny"\n}, + ["c", "d"], + 2, + ], + parse_after_error(%Q{abcde,"x\ny"\nc,d\n}, max_field_size: 4)) + end + + def test_max_field_size_recover_multiline_record_strip + assert_equal([ + "Field size exceeded: 5 > 4 in line 1.", + %Q{abcde , "x\ny" \n}, + ["c", "d"], + 2, + ], + parse_after_error(%Q{abcde , "x\ny" \nc,d\n}, + max_field_size: 4, + strip: true)) + end + + def test_max_field_size_malformed_record + assert_equal([ + "Any value after quoted field isn't allowed in line 1.", + %Q{"abcde"junk,b\n}, + ["c", "d"], + 2, + ], + parse_after_error(%Q{"abcde"junk,b\nc,d\n}, max_field_size: 4)) + end + + def test_max_field_size_unclosed_quote + assert_equal([ + "Unclosed quoted field in line 1.", + %Q{abcde,"x\nc,d\n}, + nil, + 1, + ], + parse_after_error(%Q{abcde,"x\nc,d\n}, max_field_size: 4)) + end + def test_row_sep_auto_cr assert_equal([["a"]], CSV.parse("a\r")) end @@ -383,4 +447,12 @@ def assert_parse_errors_out(data, timeout: PARSE_ERROR_TIMEOUT, **options) end end end + + def parse_after_error(data, **options) + csv = CSV.new(data, **options) + error = assert_raise(CSV::MalformedCSVError) do + csv.shift + end + [error.message, csv.line, csv.shift, csv.lineno] + end end