From f29b7a31f48d37a9e32677fd2df588cb1915a602 Mon Sep 17 00:00:00 2001 From: Kwak Byoung Min Date: Tue, 30 Jun 2026 20:19:40 +0900 Subject: [PATCH 1/3] Prevent csv QUOTE_NONE writer panics without quotechar Handle csv.writer rows with QUOTE_NONE through a RustPython-owned unquoted writer path so quotechar=None no longer reaches the unfinished csv_core configuration branch. Escape delimiter, newline, quotechar, and escapechar bytes according to the active dialect, and preserve CPython's single-empty-field error behavior. Constraint: Match CPython csv.writer behavior without changing Lib/ copied stdlib files. Rejected: Relying on csv_core QuoteStyle::Never | it does not model CPython QUOTE_NONE escaping with quotechar=None. Confidence: high Scope-risk: narrow Directive: Keep QUOTE_NONE writer behavior separate unless csv_core gains matching CPython semantics. Tested: prek run --all-files; cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher; cargo build --release --features sqlite; pytest -v in extra_tests Assisted-by: Codex:gpt-5.5 --- crates/stdlib/src/csv.rs | 85 +++++++++++++++++++++++++++--- extra_tests/snippets/stdlib_csv.py | 53 +++++++++++++++++++ 2 files changed, 132 insertions(+), 6 deletions(-) diff --git a/crates/stdlib/src/csv.rs b/crates/stdlib/src/csv.rs index bd80cf7eccf..9ba499f5096 100644 --- a/crates/stdlib/src/csv.rs +++ b/crates/stdlib/src/csv.rs @@ -910,12 +910,8 @@ mod _csv { writer = writer.delimiter(t); } - if let Some(t) = self.quotechar { - if let Some(u) = t { - writer = writer.quote(u); - } else { - todo!() - } + if let Some(Some(t)) = self.quotechar { + writer = writer.quote(t); } if let Some(t) = self.doublequote { @@ -1148,6 +1144,24 @@ mod _csv { Ok(()) } + fn write_unquoted_field( + output: &mut Vec, + data: &[u8], + dialect: PyDialect, + vm: &VirtualMachine, + ) -> PyResult<()> { + for &byte in data { + if field_needs_escape(byte, dialect) { + let escapechar = dialect + .escapechar + .ok_or_else(|| new_csv_error(vm, "need to escape, but no escapechar set"))?; + output.push(escapechar); + } + output.push(byte); + } + Ok(()) + } + fn field_needs_quotes(data: &[u8], dialect: PyDialect) -> bool { data.iter().any(|&byte| { byte == dialect.delimiter @@ -1157,6 +1171,14 @@ mod _csv { }) } + fn field_needs_escape(byte: u8, dialect: PyDialect) -> bool { + byte == dialect.delimiter + || dialect.quotechar == Some(byte) + || dialect.escapechar == Some(byte) + || matches!(byte, b'\r' | b'\n') + || matches!(dialect.lineterminator, Terminator::Any(t) if byte == t) + } + fn write_lineterminator(output: &mut Vec, terminator: Terminator) { match terminator { Terminator::CRLF => output.extend_from_slice(b"\r\n"), @@ -1222,8 +1244,59 @@ mod _csv { self.write.call((s,), vm) } + fn writerow_quote_none(&self, row: PyObjectRef, vm: &VirtualMachine) -> PyResult { + let _state = self.state.lock(); + + let row: ArgIterable = ArgIterable::try_from_object(vm, row.clone()).map_err(|_e| { + new_csv_error( + vm, + format!("'{}' object is not iterable", row.class().name()), + ) + })?; + + let fields = row.iter(vm)?.collect::>>()?; + let single_field = fields.len() == 1; + let mut output = Vec::new(); + + for (index, field) in fields.into_iter().enumerate() { + if index > 0 { + output.push(self.dialect.delimiter); + } + + let stringified; + let data: &[u8] = match_class!(match field { + ref s @ PyStr => s.as_bytes(), + crate::builtins::PyNone => b"", + ref obj => { + stringified = obj.str(vm)?; + stringified.as_bytes() + } + }); + + if single_field && data.is_empty() { + return Err(new_csv_error( + vm, + "single empty field record must be quoted", + )); + } + + write_unquoted_field(&mut output, data, self.dialect, vm)?; + } + + write_lineterminator(&mut output, self.dialect.lineterminator); + + let s = core::str::from_utf8(&output) + .map_err(|_| vm.new_unicode_decode_error("csv not utf8"))?; + + self.write.call((s,), vm) + } + #[pymethod] fn writerow(&self, row: PyObjectRef, vm: &VirtualMachine) -> PyResult { + if matches!(self.dialect.quoting, QuoteStyle::None) { + return self.writerow_quote_none(row, vm); + } + if matches!( self.dialect.quoting, QuoteStyle::Strings | QuoteStyle::Notnull diff --git a/extra_tests/snippets/stdlib_csv.py b/extra_tests/snippets/stdlib_csv.py index aa7b41223b6..9ac2b459b35 100644 --- a/extra_tests/snippets/stdlib_csv.py +++ b/extra_tests/snippets/stdlib_csv.py @@ -72,3 +72,56 @@ def test_quote_strings_and_notnull_writer(): test_quote_strings_and_notnull_writer() + + +def test_quote_none_writer_without_quotechar(): + no_quotechar_buf = io.StringIO() + csv.writer( + no_quotechar_buf, + quoting=csv.QUOTE_NONE, + quotechar=None, + escapechar="\\", + ).writerow(["a,b", 'x"y']) + assert no_quotechar_buf.getvalue() == 'a\\,b,x"y\r\n' + + default_quotechar_buf = io.StringIO() + csv.writer( + default_quotechar_buf, + quoting=csv.QUOTE_NONE, + escapechar="\\", + ).writerow(["a,b", 'x"y']) + assert default_quotechar_buf.getvalue() == 'a\\,b,x\\"y\r\n' + + escapechar_buf = io.StringIO() + csv.writer( + escapechar_buf, + quoting=csv.QUOTE_NONE, + quotechar=None, + escapechar="\\", + ).writerow(["a\\b"]) + assert escapechar_buf.getvalue() == "a\\\\b\r\n" + + with assert_raises(csv.Error): + csv.writer(io.StringIO(), quoting=csv.QUOTE_NONE, quotechar=None).writerow( + ["a,b"] + ) + + with assert_raises(csv.Error): + csv.writer( + io.StringIO(), + quoting=csv.QUOTE_NONE, + quotechar=None, + escapechar="\\", + ).writerow([None]) + + two_empty_buf = io.StringIO() + csv.writer( + two_empty_buf, + quoting=csv.QUOTE_NONE, + quotechar=None, + escapechar="\\", + ).writerow([None, ""]) + assert two_empty_buf.getvalue() == ",\r\n" + + +test_quote_none_writer_without_quotechar() From 9723fff32d94de8d9d4b004186946f804004cf22 Mon Sep 17 00:00:00 2001 From: Kwak Byoung Min Date: Tue, 30 Jun 2026 23:14:44 +0900 Subject: [PATCH 2/3] Keep csv parity tests aligned with QUOTE_NONE support Remove the stale RustPython expected-failure marker for the CPython escaped-field writer test that now passes, and cover CR/LF escaping in the snippet regression requested during review. Constraint: RustPython test policy allows removing expectedFailure markers only when the upstream test passes. Rejected: Refactoring writer row boilerplate in this follow-up | review marked it as a heavy-lift nitpick and it would broaden the CI fix. Confidence: high Scope-risk: narrow Tested: cargo run --release --features sqlite -- -m test test_csv; cargo run -- extra_tests/snippets/stdlib_csv.py; PATH=/tmp/pyshim:$PATH prek run --all-files; git diff --check; cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher; pytest -v in extra_tests Assisted-by: Codex:gpt-5.5 --- Lib/test/test_csv.py | 1 - extra_tests/snippets/stdlib_csv.py | 9 +++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_csv.py b/Lib/test/test_csv.py index 0e1f020d389..494cce50a2a 100644 --- a/Lib/test/test_csv.py +++ b/Lib/test/test_csv.py @@ -854,7 +854,6 @@ class EscapedExcel(csv.excel): class TestEscapedExcel(TestCsvBase): dialect = EscapedExcel() - @unittest.expectedFailure # TODO: RUSTPYTHON def test_escape_fieldsep(self): self.writerAssertEqual([['abc,def']], 'abc\\,def\r\n') diff --git a/extra_tests/snippets/stdlib_csv.py b/extra_tests/snippets/stdlib_csv.py index 9ac2b459b35..b9c741cbb16 100644 --- a/extra_tests/snippets/stdlib_csv.py +++ b/extra_tests/snippets/stdlib_csv.py @@ -101,6 +101,15 @@ def test_quote_none_writer_without_quotechar(): ).writerow(["a\\b"]) assert escapechar_buf.getvalue() == "a\\\\b\r\n" + linebreak_buf = io.StringIO() + csv.writer( + linebreak_buf, + quoting=csv.QUOTE_NONE, + quotechar=None, + escapechar="\\", + ).writerow(["a\rb", "c\nd"]) + assert linebreak_buf.getvalue() == "a\\\rb,c\\\nd\r\n" + with assert_raises(csv.Error): csv.writer(io.StringIO(), quoting=csv.QUOTE_NONE, quotechar=None).writerow( ["a,b"] From 97714729fa46fa25984d220ee90a41fa697c9c3c Mon Sep 17 00:00:00 2001 From: Kwak Byoung Min Date: Thu, 2 Jul 2026 19:47:13 +0900 Subject: [PATCH 3/3] Keep csv quote-style dispatch coherent Review feedback pointed out that adjacent branches selected behavior from the same quoting discriminator. Use one match so future quote-style additions are routed in a single place. Constraint: RustPython review requested merging adjacent quote-style checks. Confidence: high Scope-risk: narrow Tested: cargo fmt --check Tested: target/release/rustpython extra_tests/snippets/stdlib_csv.py Tested: target/release/rustpython -m test test_csv Tested: PATH=/tmp/pyshim:$PATH prek run --all-files Tested: RUST_TEST_THREADS=1 cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher -- --test-threads=1 Tested: cargo clippy --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --all-targets Assisted-by: Codex:gpt-5.5 --- crates/stdlib/src/csv.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/crates/stdlib/src/csv.rs b/crates/stdlib/src/csv.rs index 9ba499f5096..aaffab18252 100644 --- a/crates/stdlib/src/csv.rs +++ b/crates/stdlib/src/csv.rs @@ -1293,15 +1293,12 @@ mod _csv { #[pymethod] fn writerow(&self, row: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if matches!(self.dialect.quoting, QuoteStyle::None) { - return self.writerow_quote_none(row, vm); - } - - if matches!( - self.dialect.quoting, - QuoteStyle::Strings | QuoteStyle::Notnull - ) { - return self.writerow_quoted_strings(row, vm); + match self.dialect.quoting { + QuoteStyle::None => return self.writerow_quote_none(row, vm), + QuoteStyle::Strings | QuoteStyle::Notnull => { + return self.writerow_quoted_strings(row, vm); + } + _ => {} } let mut state = self.state.lock();