Additional validation of CSV Dialect parameters #113796

serhiy-storchaka · 2024-01-07T18:59:41Z

Feature or enhancement

CSV writer and reader allow to configure parameters of the CSV dialect. They check for some incompatible combination, for example quotechar must be set if quoting enabled. But it is still possible to specify values that produce ambiguous output which cannot be parsed neither by the CSV reader not by any other CSV parser. For example if any two of delimiter, quotechar or escapechar match. The error can be only noticed when try to read the CSV file, and it can simply produce wrong result instead of failing.

I propose to add more validation checks in addition to existing checks in the Dialect constructor.

Linked PRs

gh-113796: Add more validation checks in the csv.Dialect constructor #113797

…uctor ValueError is now raised if the same character is used in different roles.

…H-113797) ValueError is now raised if the same character is used in different roles.

…uctor (pythonGH-113797) ValueError is now raised if the same character is used in different roles.

befeleme · 2024-02-20T11:14:41Z

This changed possibly broke docutils - it was built successfully with Python 3.13.0a1 to a3, started failing in a4 in Fedora's Copr environment: https://copr-be.cloud.fedoraproject.org/results/@python/python3.13/fedora-rawhide-x86_64/07033758-python-docutils/builder-live.log.gz

======================================================================
ERROR: test_parser (test_parsers.test_rst.test_directives.test_tables.ParserTestCase.test_parser) (id="totest['csv_table'][17]")
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/docutils-0.20.1/test/test_parsers/test_rst/test_directives/test_tables.py", line 41, in test_parser
    parser.parse(case_input, document)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/parsers/rst/__init__.py", line 184, in parse
    self.statemachine.run(inputlines, document, inliner=self.inliner)
    ~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/parsers/rst/states.py", line 169, in run
    results = StateMachineWS.run(self, input_lines, input_offset,
              ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                 input_source=document['source'])
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/statemachine.py", line 233, in run
    context, next_state, result = self.check_line(
                                  ~~~~~~~~~~~~~~~^
        context, state, transitions)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/statemachine.py", line 445, in check_line
    return method(match, context, next_state)
           ~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/parsers/rst/states.py", line 2357, in explicit_markup
    self.explicit_list(blank_finish)
    ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/parsers/rst/states.py", line 2382, in explicit_list
    newline_offset, blank_finish = self.nested_list_parse(
                                   ~~~~~~~~~~~~~~~~~~~~~~^
          self.state_machine.input_lines[offset:],
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    ...<2 lines>...
          blank_finish=blank_finish,
          ^^^^^^^^^^^^^^^^^^^^^^^^^^
          match_titles=self.state_machine.match_titles)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/parsers/rst/states.py", line 316, in nested_list_parse
    state_machine.run(block, input_offset, memo=self.memo,
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                      node=node, match_titles=match_titles)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/parsers/rst/states.py", line 195, in run
    results = StateMachineWS.run(self, input_lines, input_offset)
              ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/statemachine.py", line 233, in run
    context, next_state, result = self.check_line(
                                  ~~~~~~~~~~~~~~~^
        context, state, transitions)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/statemachine.py", line 445, in check_line
    return method(match, context, next_state)
           ~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/parsers/rst/states.py", line 2660, in explicit_markup
    nodelist, blank_finish = self.explicit_construct(match)
                             ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/parsers/rst/states.py", line 2367, in explicit_construct
    return method(self, expmatch)
           ~~~~~~^^^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/parsers/rst/states.py", line 2104, in directive
    return self.run_directive(
           ~~~~~~~~~~~~~~~~~~^
        directive_class, match, type_name, option_presets)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/parsers/rst/states.py", line 2154, in run_directive
    result = directive_instance.run()
             ~~~~~~~~~~~~~~~~~~~~~~^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/parsers/rst/directives/tables.py", line 295, in run
    csv_data, self.DocutilsDialect(self.options), source)
              ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
  File "/builddir/build/BUILD/docutils-0.20.1/docutils/parsers/rst/directives/tables.py", line 237, in __init__
    super().__init__()
    ~~~~~~~~~~~~~~~~^^
  File "/usr/lib64/python3.13/csv.py", line 110, in __init__
    self._validate()
    ~~~~~~~~~~~~~~^^
  File "/usr/lib64/python3.13/csv.py", line 114, in _validate
    _Dialect(self)
    ~~~~~~~~^^^^^^
ValueError: bad delimiter value

serhiy-storchaka · 2024-02-20T12:51:47Z

Thank you for your report. It fails if the delimiter is '\r', '\n' or a space when skipinitialspace is true. The failing test case is:

.. csv-table:: good delimiter
   :delim: /

   some/csv/data

.. csv-table:: good delimiter
   :delim: \

   some\csv\data

.. csv-table:: good delimiter
   :delim: 0x5c

   some\csv\data

.. csv-table:: good delimiter
   :delim: space

   some csv data

I guess that the problem is caused by :delim: space. skipinitialspace is set to True in DocutilsDialect.

It is perhaps a Python failure. I see now a use case for such combination. Opened #115712.

…ialspace=True Restore support of such combination, disabled in pythongh-113796. csv.writer() now quotes empty fields if delimiter is a space and skipinitialspace is true and raises exception if quoting is not possible.

…ce=True (GH-115721) Restore support of such combination, disabled in gh-113796. csv.writer() now quotes empty fields if delimiter is a space and skipinitialspace is true and raises exception if quoting is not possible.

…kipinitialspace=True (pythonGH-115721) Restore support of such combination, disabled in pythongh-113796. csv.writer() now quotes empty fields if delimiter is a space and skipinitialspace is true and raises exception if quoting is not possible. (cherry picked from commit 937d282) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

…ialspace=True (pythonGH-115721) Restore support of such combination, disabled in pythongh-113796. csv.writer() now quotes empty fields if delimiter is a space and skipinitialspace is true and raises exception if quoting is not possible.

serhiy-storchaka added the type-feature A feature request or enhancement label Jan 7, 2024

serhiy-storchaka self-assigned this Jan 7, 2024

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Jan 7, 2024

pythongh-113796: Add more validation checks in the csv.Dialect constr…

a1ee894

…uctor ValueError is now raised if the same character is used in different roles.

bedevere-app bot mentioned this issue Jan 7, 2024

gh-113796: Add more validation checks in the csv.Dialect constructor #113797

Merged

serhiy-storchaka added a commit that referenced this issue Jan 22, 2024

gh-113796: Add more validation checks in the csv.Dialect constructor (G…

c8351a6

…H-113797) ValueError is now raised if the same character is used in different roles.

serhiy-storchaka closed this as completed Jan 22, 2024

aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024

pythongh-113796: Add more validation checks in the csv.Dialect constr…

6ff8bed

…uctor (pythonGH-113797) ValueError is now raised if the same character is used in different roles.

serhiy-storchaka mentioned this issue Feb 20, 2024

CSV dialect with delimiter=' ' and skipinitialspace=True #115712

Closed

serhiy-storchaka reopened this Feb 20, 2024

serhiy-storchaka closed this as completed Feb 20, 2024

serhiy-storchaka mentioned this issue Feb 20, 2024

gh-115712: Support CSV dialects with delimiter=' ' and skipinitialspace=True #115721

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Additional validation of CSV Dialect parameters #113796

Additional validation of CSV Dialect parameters #113796

serhiy-storchaka commented Jan 7, 2024 •

edited by bedevere-app bot

befeleme commented Feb 20, 2024

serhiy-storchaka commented Feb 20, 2024

Additional validation of CSV Dialect parameters #113796

Additional validation of CSV Dialect parameters #113796

Comments

serhiy-storchaka commented Jan 7, 2024 • edited by bedevere-app bot

Feature or enhancement

Linked PRs

befeleme commented Feb 20, 2024

serhiy-storchaka commented Feb 20, 2024

serhiy-storchaka commented Jan 7, 2024 •

edited by bedevere-app bot