Skip to content

csv: validate dialect options - #8402

Open
hyoinandout wants to merge 1 commit into
RustPython:mainfrom
hyoinandout:fix/csv-dialect-validation
Open

csv: validate dialect options#8402
hyoinandout wants to merge 1 commit into
RustPython:mainfrom
hyoinandout:fix/csv-dialect-validation

Conversation

@hyoinandout

@hyoinandout hyoinandout commented Jul 27, 2026

Copy link
Copy Markdown

Summary

Resolve each dialect once and validate the merged options before constructing readers and writers. Handle Unicode character parsing consistently and enable the corresponding CPython CSV tests.

Assisted-by: Tau:gpt-5.6-luna

Summary by CodeRabbit

  • Bug Fixes
    • Improved validation for CSV dialect settings, including delimiters, quote characters, escape characters, and line terminators.
    • Added clearer, more consistent errors when character options are invalid or conflict with one another.
    • Prevented incompatible combinations, such as using spaces with certain quoting or escaping options.
    • Ensured CSV readers and writers consistently apply the resolved dialect configuration.

Resolve each dialect once and validate the merged options before
constructing readers and writers. Handle Unicode character parsing
consistently and enable the corresponding CPython CSV tests.

Assisted-by: Tau:gpt-5.6-luna
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

CSV character parsing is centralized through Unicode-aware helpers. Dialect validation now runs during registration and resolution, while readers and writers consume the resolved dialect for their settings.

Changes

CSV dialect behavior

Layer / File(s) Summary
Character parsing and argument handling
crates/stdlib/src/csv.rs
Delimiter, quotechar, escapechar, and lineterminator parsing now uses shared Unicode character helpers with consistent errors.
Dialect resolution and validation
crates/stdlib/src/csv.rs
Resolved dialects are checked for line-break characters, invalid space combinations, and conflicting attributes during registration and option resolution.
Reader and writer dialect wiring
crates/stdlib/src/csv.rs
Reader and writer construction now derives settings from the resolved dialect.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant FormatOptions
  participant validate_dialect
  participant ReaderWriter
  Caller->>FormatOptions: parse dialect arguments
  FormatOptions->>validate_dialect: validate resolved dialect
  validate_dialect-->>FormatOptions: validated dialect
  FormatOptions-->>ReaderWriter: provide dialect settings
Loading

Possibly related PRs

Suggested labels: z-ca-2026

Suggested reviewers: shaharnaveh, doma17

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: validating CSV dialect options.
Linked Issues check ✅ Passed The dialect validation and character parsing changes address the escapechar TypeError issue in #8284.
Out of Scope Changes check ✅ Passed The changes stay focused on CSV dialect validation and related parsing, with no obvious unrelated additions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
crates/stdlib/src/csv.rs (2)

299-326: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Validate dialect attributes during direct _csv.Dialect(...) construction.

PyDialect::try_from_object currently succeeds for invalid attributes even though validate_dialect is only called later from register_dialect and FormatOptions::result; direct dialect construction / subclass initialization diverges from CPython. Add validate_dialect(vm, &dialect)? before returning the constructed PyDialect.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/stdlib/src/csv.rs` around lines 299 - 326, Update
PyDialect::try_from_object to construct the dialect value first, call
validate_dialect(vm, &dialect)? on it, and return it only after validation
succeeds. Preserve the existing attribute parsing and strict-default behavior,
while ensuring direct _csv.Dialect construction and subclass initialization
reject invalid attributes.

632-757: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use the dialect attribute validation helpers for escapechar and quotechar kwargs.

The delimiter kwarg already delegates to parse_delimiter_from_obj. escapechar/quotechar should use the corresponding helpers instead of inlining checks, so invalid types/lengths use the correct TypeError text and accept PyNone where that helper supports it. This closes the remaining keyword-argument path for issue #8284.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/stdlib/src/csv.rs` around lines 632 - 757, Update
FormatOptions::from_args to parse the escapechar and quotechar kwargs through
the existing dialect attribute validation helpers, matching the delimiter path.
Remove the inline match-based validation and preserve each helper’s handling of
invalid types, character length, and PyNone support.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/stdlib/src/csv.rs`:
- Around line 259-282: Update parse_single_char and parse_first_char so
char_len() is used only for empty or multi-code-point length errors, while
conversion failures use a distinct error path and message. Remove the u8-only
restriction for valid single Unicode code points such as €, and update the
functions’ return type and their callers as needed to preserve the full code
point for CSV dialect attributes.
- Around line 792-807: Update the duplicate-character validation around the
values collection and iteration so dialect_check_chars compares only delimiter,
quotechar, and escapechar. Keep lineterminator validation in the separate
dialect_check_char path, preserving CPython’s acceptance of dialects where it
matches another character setting.

---

Outside diff comments:
In `@crates/stdlib/src/csv.rs`:
- Around line 299-326: Update PyDialect::try_from_object to construct the
dialect value first, call validate_dialect(vm, &dialect)? on it, and return it
only after validation succeeds. Preserve the existing attribute parsing and
strict-default behavior, while ensuring direct _csv.Dialect construction and
subclass initialization reject invalid attributes.
- Around line 632-757: Update FormatOptions::from_args to parse the escapechar
and quotechar kwargs through the existing dialect attribute validation helpers,
matching the delimiter path. Remove the inline match-based validation and
preserve each helper’s handling of invalid types, character length, and PyNone
support.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: aa3d9483-5785-4d03-bc09-408322fa6732

📥 Commits

Reviewing files that changed from the base of the PR and between 59e903d and d49a37f.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_csv.py is excluded by !Lib/**
📒 Files selected for processing (1)
  • crates/stdlib/src/csv.rs

Comment thread crates/stdlib/src/csv.rs
Comment on lines +259 to +282
fn parse_single_char(
s: &Py<PyStr>,
error: impl Fn(usize) -> PyBaseExceptionRef,
) -> PyResult<u8> {
let ch = s
.as_wtf8()
.code_points()
.exactly_one()
.map_err(|_| error(s.char_len()))?;
u8::try_from(ch.to_u32()).map_err(|_| error(s.char_len()))
}

fn parse_first_char(
s: &Py<PyStr>,
error: impl Fn(usize) -> PyBaseExceptionRef,
) -> PyResult<u8> {
let ch = s
.as_wtf8()
.code_points()
.next()
.ok_or_else(|| error(s.char_len()))?;
u8::try_from(ch.to_u32()).map_err(|_| error(s.char_len()))
}

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
git ls-files | rg '(^|/)csv\.rs$|csv' || true

echo "== outline =="
ast-grep outline crates/stdlib/src/csv.rs --view compact || true

echo "== relevant csv lines =="
sed -n '1,380p' crates/stdlib/src/csv.rs | cat -n | sed -n '220,320p'

echo "== search parse_single_char/usages =="
rg -n "parse_single_char|parse_first_char|not a string of length|dialection|Delimiter|QuoteChar|DoubleQuote|error\\(" crates/stdlib/src/csv.rs crates -g '*.rs' || true

Repository: RustPython/RustPython

Length of output: 50378


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== csv.rs relevant section =="
sed -n '220,320p' crates/stdlib/src/csv.rs | cat -n

echo "== parse_single_char/parse_first_char and nearby dialect fields =="
rg -n "pub struct PyDialect|Delimiter|QuoteChar|DoubleQuoteChars|escapechar|lineterminator|skipinitialspace|parse_single_char|parse_first_char|error\\(" crates/stdlib/src/csv.rs -C 3

echo "== focused tests/usages =="
rg -n "sniffer|PyDialect|dialect|delimiter|quotechar|doublequote|escapechar|skipinitialspace|lineterminator|ParserError" crates -g '*.rs' --glob '!crates/vm/src/**' | head -n 200

echo "== CPython docs/source snippets online =="
python3 - <<'PY'
# Behavioral probe: model Rust string semantics around the parsing pattern.
s = "€"
print("single unicode char", len(s), "bytes", len(s.encode('utf-8')))
code_point = ord("€")
print("code point", code_point, "fits in u8", code_point <= 0xff)
s_multi = "€1"
print("multi char chars", len(s_multi))
PY

Repository: RustPython/RustPython

Length of output: 50377


🌐 Web query:

Python csv module Dialect delimiter quotechar lineterminator escapechar stored as Py_UCS4 accepts single Unicode characters longer than ASCII

💡 Result:

The Python csv module does not accept Unicode strings longer than a single character for the delimiter, quotechar, or escapechar parameters [1][2][3]. While the internal implementation of the CPython csv module (located in Modules/_csv.c) uses the Py_UCS4 type to store these parameters [4][5][6], this type is used specifically to hold a single Unicode code point [3]. Validation logic within the csv module explicitly ensures that these parameters are one-character strings [7][3]. If a string with more than one character (or an empty string, where disallowed) is provided, the module will raise a TypeError [5][7]. Specifically, the _csv module's internal dialect validation routines reject any attempt to assign a multi-character string to these attributes [5][3]. The lineterminator parameter, by contrast, is stored as a PyObject (a string) and is not restricted to a single character [4][5][6].

Citations:


🌐 Web query:

grep "_csv.c" Py_UCS4 unicode code point validate single character csv delimiter quotechar

💡 Result:

In Python's csv module, the validation of parameters such as the delimiter, quotechar, and escapechar occurs within the C-based implementation found in Modules/_csv.c [1][2][3]. The dialect object stores these characters as Py_UCS4 type values, which are used to handle Unicode code points [1][4]. When these parameters are set, the _csv module verifies that they are either None (where applicable) or a string of exactly one character [1][2][3]. The specific validation process is handled by internal helper functions, typically named dialect_check_char or similar, which are invoked during the initialization of a Dialect object [2][5][3]. These functions enforce that: 1. The provided input is a string [1][6]. 2. The length of the string is exactly 1 (checked via PyUnicode_GetLength) [1][4]. 3. If an invalid type (like an integer) or a string with a length other than 1 is provided, the module raises a TypeError with a message such as "must be a 1-character string" [1][7][8]. Additionally, the code ensures that the delimiter, quotechar, and escapechar do not conflict (e.g., they cannot be the same character), which is verified by functions like dialect_check_chars [2][5][3]. If quoting is enabled, the module also explicitly checks that the quotechar is properly set [2][5][3].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import csv
for attr, names in [
    ("delimiter", ("excel",)),
    ("quotechar", ("excel",)),
    ("escapechar", ("excel",)),
    ("lineterminator", ("excel",)),
]:
    dialect = csv.get_dialect(names[0])
    print(f"csv dialect {attr}: {getattr(dialect, attr)!r!s!U} type={type(getattr(dialect, attr)).__name__!r}")
    for value, param in [
        ("€", attr),
        ("€1", attr[:-1] if attr == "lineterminator" else attr),
        ("", attr),
        ("💙", attr),
        ("💙1", attr),
    ]:
        d = csv.registered_dialects[1]
        try:
            d.update(**{param: value})
            print(f"   OK {attr}={value!r!s!U} len_chars={len(value)}")
        except Exception as e:
            print(f"   {type(e).__name__}: {e}")

print("Python unicode len examples:")
for s in ["€", "💙", "🩵", "€1", "💙1"]:
    print(repr(s), "chars=", len(s), "bytes=", len(s.encode("utf-8")), "ord=", ord(s[0]))
PY

Repository: RustPython/RustPython

Length of output: 445


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import csv
for attr, names in [
    ("delimiter", ("excel",)),
    ("quotechar", ("excel",)),
    ("escapechar", ("excel",)),
    ("lineterminator", ("excel",)),
]:
    dialect = csv.get_dialect(names[0])
    val = getattr(dialect, attr)
    print(f"csv dialect {attr}: {val!r} type={type(val).__name__}")
    for value, param in [
        ("€", attr),
        ("€1", attr if attr != "lineterminator" else attr[:-1]),
        ("", attr),
        ("💙", attr),
        ("💙1", attr if attr != "lineterminator" else attr[:-1]),
    ]:
        try:
            dialect.update(**{param: value})
            print(f"   OK {attr}={value!r} chars={len(value)}")
        except Exception as e:
            print(f"   {type(e).__name__}: {e}")

print("Python unicode len examples:")
for s in ["€", "💙", "🩵", "€1", "💙1"]:
    print(repr(s), "chars=", len(s), "bytes=", len(s.encode("utf-8")), "ord=", ord(s[0]))
PY

Repository: RustPython/RustPython

Length of output: 1866


🌐 Web query:

Python csv.register_dialect Dialect delimiter quotechar escapechar lineterminator non-ASCII character accepts

💡 Result:

In Python's csv module, the delimiter, quotechar, and escapechar formatting parameters are strictly required to be one-character strings [1]. Regarding non-ASCII characters: 1. Python 3: You can use non-ASCII characters (such as Unicode characters) as a delimiter, quotechar, or escapechar, provided the character is a single Unicode character [2]. 2. Python 2: The csv module typically enforces that these parameters must be 1-character strings. While it may accept certain characters depending on the environment, using non-ASCII characters often leads to compatibility issues or TypeError exceptions [2]. The lineterminator parameter is not restricted to a single character and can be a string of any length [1]. To register a dialect with these parameters, you use the csv.register_dialect function [3][4]: import csv csv.register_dialect('my_dialect', delimiter='ę', quotechar='æ', escapechar='\', lineterminator='\r\n') When defining dialects, keyword arguments (such as delimiter or quotechar) override any parameters inherited from a passed Dialect class [3][4].

Citations:


Separate character-length errors from UTF8/u8 conversion errors.

parse_single_char() and parse_first_char() currently reuse s.char_len() for both empty/multi-character strings and invalid UTF8/u8 conversion failures. For a valid single Unicode code point above u8’s range, this produces the misleading “not a string of length 1” message. Also, RustPython still rejects single non-ASCII code points like here, unlike CPython csv, which accepts any single Unicode code point for dialect character attributes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/stdlib/src/csv.rs` around lines 259 - 282, Update parse_single_char
and parse_first_char so char_len() is used only for empty or multi-code-point
length errors, while conversion failures use a distinct error path and message.
Remove the u8-only restriction for valid single Unicode code points such as €,
and update the functions’ return type and their callers as needed to preserve
the full code point for CSV dialect attributes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing this requires a broader Unicode support change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hyoinandout, understood—this is outside the scope of the dialect-validation work in #8402. The finding can remain as a follow-up for broader Unicode representation in the CSV parser rather than blocking this PR.

Would you like me to open a GitHub issue to track support for single non-ASCII Unicode dialect characters (for example, )?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="RustPython/RustPython"
body=$(cat <<'EOF'
## Summary

Add support for single non-ASCII Unicode code points in CSV dialect character attributes, matching CPython behavior.

## Rationale

`csv` dialect options such as `delimiter`, `quotechar`, and `escapechar` accept exactly one Unicode character in CPython. RustPython currently stores these attributes as `u8` in `crates/stdlib/src/csv.rs`, so valid one-character values outside the byte range (for example, `€`) are rejected.

## Affected area

- `crates/stdlib/src/csv.rs`
- CSV dialect parsing, validation, reader, and writer configuration

## Required changes

Replace the byte-only representation and parsing path for dialect characters with a representation that preserves a full Unicode code point, then propagate that support through the CSV reader and writer implementation. Keep existing validation that rejects empty and multi-character values, while reporting conversion/representation failures accurately.

## Acceptance criteria

- `delimiter`, `quotechar`, and `escapechar` accept a single non-ASCII Unicode code point such as `€`.
- Empty and multi-character strings remain rejected according to CPython-compatible behavior.
- Reader and writer behavior correctly handles the supported Unicode dialect characters.
- Relevant RustPython CSV tests cover successful non-ASCII dialect-character usage and invalid lengths.

## Backlinks

- Follow-up from PR `#8402`: https://github.com/RustPython/RustPython/pull/8402
- Review discussion: https://github.com/RustPython/RustPython/pull/8402#discussion_r3658464157

Requested by `@hyoinandout`.
EOF
)

gh issue create \
  --repo "$repo" \
  --title "csv: support non-ASCII Unicode dialect characters" \
  --body "$body" \
  --assignee "hyoinandout"

Length of output: 1983


@hyoinandout, created and assigned the follow-up: #8408csv: support non-ASCII Unicode dialect characters.

🐇✨

Comment thread crates/stdlib/src/csv.rs
@github-actions

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] test: cpython/Lib/test/test_binascii.py (TODO: 4)

dependencies:

dependent tests: (95 tests)

  • binascii: test_base64 test_binascii test_codecs test_ctypes test_descr test_hashlib test_hmac test_plistlib test_struct test_zlib
    • base64: test_email test_gettext test_httpservers test_smtplib test_urllib2 test_urllib2_localnet test_xmlrpc test_zoneinfo
      • http.server: test_logging test_robotparser
      • logging.handlers: test_concurrent_futures test_pkgutil
      • secrets: test_secrets
      • smtplib: test_smtpnet
      • ssl: test_asyncio test_ftplib test_httplib test_imaplib test_poplib test_ssl test_urllib test_venv
      • urllib.request: test_http_cookiejar test_pathlib test_pydoc test_sax test_site test_urllib2net test_urllibnet
    • email: test_email test_mailbox test_zipfile
      • importlib.metadata: test_importlib
      • mailbox: test_genericalias
      • pydoc: test_enum
    • http.server:
      • wsgiref.simple_server: test_wsgiref
    • plistlib:
      • platform: test__locale test__osx_support test_asyncio test_baseexception test_builtin test_cmath test_ctypes test_fcntl test_math test_mimetypes test_os test_platform test_posix test_regrtest test_shutil test_socket test_strptime test_sysconfig test_time test_winreg
    • quopri: test_quopri
    • zipfile: test_pdb test_zipapp test_zipfile test_zipfile64 test_zipimport test_zipimport_support
      • shutil: test_argparse test_bz2 test_compileall test_ctypes test_embed test_filecmp test_glob test_importlib test_inspect test_largefile test_launcher test_modulefinder test_peg_generator test_py_compile test_reprlib test_string_literals test_subprocess test_support test_tarfile test_tempfile test_traceback test_unicode_file

[x] lib: cpython/Lib/sqlite3
[x] test: cpython/Lib/test/test_sqlite3 (TODO: 69)

dependencies:

  • sqlite3

dependent tests: (2 tests)

  • sqlite3: test_dbm_sqlite3 test_sqlite3

[x] lib: cpython/Lib/csv.py
[x] test: cpython/Lib/test/test_csv.py (TODO: 16)

dependencies:

  • csv

dependent tests: (4 tests)

  • csv: test_csv test_genericalias
    • importlib.metadata: test_importlib test_zoneinfo

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

@youknowone youknowone added the z-ca-2026 Tag to track Contribution Academy 2026 label Jul 28, 2026
@moreal

moreal commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

@hyoinandout Could you resolve conflicts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

z-ca-2026 Tag to track Contribution Academy 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

csv: reader allows escapechar=1

3 participants