From 3a23fc573ad5e41bbee51bbfb8bc712f03f188f6 Mon Sep 17 00:00:00 2001 From: jinmay Date: Mon, 20 Jul 2026 11:57:21 +0900 Subject: [PATCH 1/4] csv: support multi-character lineterminator in writer Store the dialect line terminator as an owned String instead of the single-byte csv_core::Terminator, dropping PyDialect's Copy derive. The manual writer paths emit the full terminator; the csv-core-backed QUOTE_ALL/QUOTE_NONNUMERIC paths emit a sentinel byte (preserving csv-core's quote/empty-record bookkeeping) and append the real terminator. field_needs_quotes/escape now quote a field containing any terminator byte. The reader ignores lineterminator and always uses CRLF, matching CPython and avoiding mid-UTF-8 record splits. --- crates/stdlib/src/csv.rs | 188 ++++++++++++++--------------- extra_tests/snippets/stdlib_csv.py | 68 +++++++++++ 2 files changed, 160 insertions(+), 96 deletions(-) diff --git a/crates/stdlib/src/csv.rs b/crates/stdlib/src/csv.rs index 91717801bc4..2163d0e2ebb 100644 --- a/crates/stdlib/src/csv.rs +++ b/crates/stdlib/src/csv.rs @@ -77,18 +77,27 @@ mod _csv { #[pyattr] #[pyclass(module = "csv", name = "Dialect")] - #[derive(Debug, PyPayload, Clone, Copy)] + #[derive(Debug, PyPayload, Clone)] struct PyDialect { delimiter: u8, quotechar: Option, escapechar: Option, doublequote: bool, skipinitialspace: bool, - lineterminator: csv_core::Terminator, + lineterminator: String, quoting: QuoteStyle, strict: bool, } + /// Placeholder single-byte terminator for the csv-core writer paths + /// (`QUOTE_ALL` / `QUOTE_NONNUMERIC`). csv-core can only emit a single byte + /// for the record terminator, but its `terminator()` call also performs + /// essential bookkeeping — closing the final quote and emitting `""` for an + /// empty record — that must not be bypassed. So the writer emits this + /// sentinel byte, and `writerow` strips it and appends the real (possibly + /// multi-character) line terminator afterwards. + const CSV_CORE_TERMINATOR_SENTINEL: u8 = b'\n'; + impl Constructor for PyDialect { type Args = PyObjectRef; @@ -121,11 +130,7 @@ mod _csv { #[pygetset] fn lineterminator(&self, vm: &VirtualMachine) -> PyRef { - match self.lineterminator { - Terminator::CRLF => vm.ctx.new_str("\r\n".to_string()), - Terminator::Any(t) => vm.ctx.new_str(format!("{}", t as char)), - _ => unreachable!(), - } + vm.ctx.new_str(self.lineterminator.clone()) } #[pygetset] @@ -230,19 +235,20 @@ mod _csv { }) } - fn prase_lineterminator_from_obj(vm: &VirtualMachine, obj: &PyObject) -> PyResult { + fn prase_lineterminator_from_obj(vm: &VirtualMachine, obj: &PyObject) -> PyResult { match_class!(match obj.get_attr("lineterminator", vm)? { s @ PyStr => { - Ok(if s.as_bytes().eq(b"\r\n") { - csv_core::Terminator::CRLF - } else if let Some(t) = s.as_bytes().first() { - // Due to limitations in the current implementation within csv_core - // the support for multiple characters in lineterminator is not complete. - // only capture the first character - csv_core::Terminator::Any(*t) - } else { + // Store the full line terminator string. CPython accepts an + // arbitrary-length terminator; the manual writer paths emit it + // verbatim and the csv-core writer path appends it after a + // sentinel terminator (see `writerow`). + let value = s + .to_str() + .ok_or_else(|| new_csv_error(vm, r#""lineterminator" must be a string"#))?; + if value.is_empty() { return Err(new_csv_error(vm, r#""lineterminator" must be a string"#)); - }) + } + Ok(value.to_owned()) } attr => { Err(vm.new_type_error(format!( @@ -344,7 +350,7 @@ mod _csv { let g = GLOBAL_HASHMAP.lock(); if let Some(dialect) = g.get(name.as_str()) { - return Ok(*dialect); + return Ok(dialect.clone()); } Err(new_csv_error(vm, "unknown dialect")) @@ -540,7 +546,7 @@ mod _csv { escapechar: Option, doublequote: Option, skipinitialspace: Option, - lineterminator: Option, + lineterminator: Option, quoting: Option, strict: Option, } @@ -629,15 +635,16 @@ mod _csv { }; if let Some(lineterminator) = args.kwargs.swap_remove("lineterminator") { - res.lineterminator = Some(csv_core::Terminator::Any( - lineterminator - .try_to_value::<&str>(vm)? - .bytes() - .exactly_one() - .map_err(|_| { - vm.new_type_error(r#""lineterminator" must be a 1-character string"#) - })?, - )) + let value = lineterminator.try_to_value::<&str>(vm)?; + // Preserve the previous behavior of rejecting an empty terminator + // (full validation parity is deferred to a follow-up). Any + // non-empty string, including multi-character ones, is stored. + if value.is_empty() { + return Err(vm + .new_type_error(r#""lineterminator" must be a 1-character string"#) + .into()); + } + res.lineterminator = Some(value.to_owned()); }; if let Some(doublequote) = args.kwargs.swap_remove("doublequote") { @@ -717,7 +724,7 @@ mod _csv { } impl FormatOptions { - const fn update_py_dialect(&self, mut res: PyDialect) -> PyDialect { + fn update_py_dialect(&self, mut res: PyDialect) -> PyDialect { macro_rules! check_and_fill { ($res:ident, $e:ident) => {{ if let Some(t) = self.$e { @@ -741,7 +748,9 @@ mod _csv { }; check_and_fill!(res, quoting); - check_and_fill!(res, lineterminator); + if let Some(t) = &self.lineterminator { + res.lineterminator.clone_from(t); + }; check_and_fill!(res, strict); res } @@ -751,16 +760,16 @@ mod _csv { DialectItem::Str(name) => { let g = GLOBAL_HASHMAP.lock(); if let Some(dialect) = g.get(name) { - Ok(self.update_py_dialect(*dialect)) + Ok(self.update_py_dialect(dialect.clone())) } else { Err(new_csv_error(vm, format!("{name} is not registered."))) } // TODO: Maybe need to update the obj from HashMap } - DialectItem::Obj(o) => Ok(self.update_py_dialect(*o)), + DialectItem::Obj(o) => Ok(self.update_py_dialect(o.clone())), DialectItem::None => { let g = GLOBAL_HASHMAP.lock(); - let res = *g.get("excel").unwrap(); + let res = g.get("excel").unwrap().clone(); Ok(self.update_py_dialect(res)) } } @@ -788,27 +797,6 @@ mod _csv { skipinitialspace } - fn get_lineterminator(&self) -> csv_core::Terminator { - let mut lineterminator = match &self.dialect { - DialectItem::Str(name) => { - let g = GLOBAL_HASHMAP.lock(); - if let Some(dialect) = g.get(name) { - dialect.lineterminator - } else { - Terminator::CRLF - } - } - DialectItem::Obj(obj) => obj.lineterminator, - _ => Terminator::CRLF, - }; - - if let Some(attr) = self.lineterminator { - lineterminator = attr - } - - lineterminator - } - fn get_quoting(&self) -> QuoteStyle { let mut quoting = match &self.dialect { DialectItem::Str(name) => { @@ -832,11 +820,11 @@ mod _csv { fn to_reader(&self) -> csv_core::Reader { let dialect = match &self.dialect { - DialectItem::Str(name) => GLOBAL_HASHMAP.lock().get(name).copied(), - DialectItem::Obj(obj) => Some(*obj), + DialectItem::Str(name) => GLOBAL_HASHMAP.lock().get(name).cloned(), + DialectItem::Obj(obj) => Some(obj.clone()), DialectItem::None => { let g = GLOBAL_HASHMAP.lock(); - Some(*g.get("excel").unwrap()) + Some(g.get("excel").unwrap().clone()) } }; @@ -868,10 +856,6 @@ mod _csv { reader = reader.quoting(self.quoting != Some(QuoteStyle::None)); } - if let Some(t) = self.lineterminator { - reader = reader.terminator(t); - } - if let Some(t) = self.doublequote { reader = reader.double_quote(t); } @@ -880,7 +864,12 @@ mod _csv { reader = reader.escape(self.escapechar); } - reader = reader.terminator(self.lineterminator.unwrap_or(Terminator::CRLF)); + // CPython's reader ignores the dialect's `lineterminator` entirely and + // only recognizes `\r`, `\n`, and `\r\n` as record separators. Match + // that: always use CRLF mode. Feeding a multi-byte terminator's first + // byte here would otherwise split records mid-UTF-8 and raise a + // UnicodeDecodeError. + reader = reader.terminator(Terminator::CRLF); reader.build() } @@ -894,7 +883,7 @@ mod _csv { let mut builder = builder .delimiter(dialect.delimiter) .double_quote(dialect.doublequote) - .terminator(dialect.lineterminator); + .terminator(Terminator::Any(CSV_CORE_TERMINATOR_SENTINEL)); if let Some(t) = dialect.quotechar { builder = builder.quote(t); @@ -911,7 +900,7 @@ mod _csv { let mut builder = builder .delimiter(obj.delimiter) .double_quote(obj.doublequote) - .terminator(obj.lineterminator); + .terminator(Terminator::Any(CSV_CORE_TERMINATOR_SENTINEL)); if let Some(t) = obj.quotechar { builder = builder.quote(t); @@ -934,7 +923,7 @@ mod _csv { writer = writer.double_quote(t); } - writer = writer.terminator(self.get_lineterminator()); + writer = writer.terminator(Terminator::Any(CSV_CORE_TERMINATOR_SENTINEL)); if let Some(e) = self.escapechar { writer = writer.escape(e); @@ -979,8 +968,8 @@ mod _csv { } #[pygetset] - const fn dialect(&self, _vm: &VirtualMachine) -> PyDialect { - self.dialect + fn dialect(&self, _vm: &VirtualMachine) -> PyDialect { + self.dialect.clone() } } @@ -1014,7 +1003,7 @@ mod _csv { &mut self, input: &[u8], index: usize, - dialect: PyDialect, + dialect: &PyDialect, unquoted_escape: bool, ) -> (QuoteScanEvent, usize) { let byte = input[index]; @@ -1067,7 +1056,7 @@ mod _csv { fn read_quote_record( input: &[u8], - dialect: PyDialect, + dialect: &PyDialect, field_limit: isize, vm: &VirtualMachine, ) -> PyResult> { @@ -1179,13 +1168,13 @@ mod _csv { ) || (zelf.dialect.quoting == QuoteStyle::None && zelf.dialect.escapechar.is_some()); if use_quote_record { - let out = read_quote_record(input, zelf.dialect, field_limit, vm)?; + let out = read_quote_record(input, &zelf.dialect, field_limit, vm)?; *line_num += 1; return Ok(PyIterReturn::Return(vm.ctx.new_list(out).into())); } #[inline] - fn trim_initial_spaces(input: &[u8], dialect: PyDialect) -> Vec { + fn trim_initial_spaces(input: &[u8], dialect: &PyDialect) -> Vec { let mut trimmed = Vec::with_capacity(input.len()); let mut scan_state = QuoteScanState::new(); let mut index = 0; @@ -1215,7 +1204,7 @@ mod _csv { } let input = if *skipinitialspace { - String::from_utf8(trim_initial_spaces(input, zelf.dialect)).unwrap() + String::from_utf8(trim_initial_spaces(input, &zelf.dialect)).unwrap() } else { String::from_utf8(input.to_vec()).unwrap() }; @@ -1314,7 +1303,7 @@ mod _csv { fn write_quoted_field( output: &mut Vec, data: &[u8], - dialect: PyDialect, + dialect: &PyDialect, vm: &VirtualMachine, ) -> PyResult<()> { let quotechar = dialect @@ -1346,7 +1335,7 @@ mod _csv { fn write_unquoted_field( output: &mut Vec, data: &[u8], - dialect: PyDialect, + dialect: &PyDialect, vm: &VirtualMachine, ) -> PyResult<()> { for &byte in data { @@ -1361,36 +1350,35 @@ mod _csv { Ok(()) } - fn field_needs_quotes(data: &[u8], dialect: PyDialect) -> bool { + fn field_needs_quotes(data: &[u8], dialect: &PyDialect) -> bool { data.iter().any(|&byte| { byte == dialect.delimiter || dialect.quotechar == Some(byte) || matches!(byte, b'\r' | b'\n') - || matches!(dialect.lineterminator, Terminator::Any(t) if byte == t) + // CPython quotes a field that contains any character of the line + // terminator. This byte-wise `contains` matches CPython for + // ASCII terminators; non-ASCII terminators are not fully handled. + || dialect.lineterminator.as_bytes().contains(&byte) }) } - fn field_needs_escape(byte: u8, dialect: PyDialect) -> bool { + 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) + || dialect.lineterminator.as_bytes().contains(&byte) } - fn write_lineterminator(output: &mut Vec, terminator: Terminator) { - match terminator { - Terminator::CRLF => output.extend_from_slice(b"\r\n"), - Terminator::Any(byte) => output.push(byte), - _ => unreachable!(), - } + fn write_lineterminator(output: &mut Vec, terminator: &str) { + output.extend_from_slice(terminator.as_bytes()); } #[pyclass(flags(DISALLOW_INSTANTIATION))] impl Writer { #[pygetset(name = "dialect")] - const fn get_dialect(&self, _vm: &VirtualMachine) -> PyDialect { - self.dialect + fn get_dialect(&self, _vm: &VirtualMachine) -> PyDialect { + self.dialect.clone() } fn writerow_quoted_strings(&self, row: PyObjectRef, vm: &VirtualMachine) -> PyResult { @@ -1421,12 +1409,12 @@ mod _csv { }); let should_quote = match self.dialect.quoting { - QuoteStyle::Strings => is_str || field_needs_quotes(data, self.dialect), + QuoteStyle::Strings => is_str || field_needs_quotes(data, &self.dialect), QuoteStyle::Notnull => !is_none, _ => unreachable!(), }; if should_quote { - write_quoted_field(&mut output, data, self.dialect, vm)?; + write_quoted_field(&mut output, data, &self.dialect, vm)?; } else if single_field && data.is_empty() { return Err(new_csv_error( vm, @@ -1437,7 +1425,7 @@ mod _csv { } } - write_lineterminator(&mut output, self.dialect.lineterminator); + write_lineterminator(&mut output, &self.dialect.lineterminator); let s = core::str::from_utf8(&output).map_err(|e| new_not_utf8_error(vm, &output, e))?; self.write.call((s,), vm) @@ -1479,10 +1467,10 @@ mod _csv { )); } - write_unquoted_field(&mut output, data, self.dialect, vm)?; + write_unquoted_field(&mut output, data, &self.dialect, vm)?; } - write_lineterminator(&mut output, self.dialect.lineterminator); + write_lineterminator(&mut output, &self.dialect.lineterminator); let s = core::str::from_utf8(&output).map_err(|e| new_not_utf8_error(vm, &output, e))?; @@ -1524,14 +1512,14 @@ mod _csv { // terminator, regardless of which line terminator is // configured. A row with a single empty field is also quoted // so that it is not read back as an empty line. - if field_needs_quotes(data, self.dialect) || (single_field && data.is_empty()) { - write_quoted_field(&mut output, data, self.dialect, vm)?; + if field_needs_quotes(data, &self.dialect) || (single_field && data.is_empty()) { + write_quoted_field(&mut output, data, &self.dialect, vm)?; } else { output.extend_from_slice(data); } } - write_lineterminator(&mut output, self.dialect.lineterminator); + write_lineterminator(&mut output, &self.dialect.lineterminator); let s = core::str::from_utf8(&output).map_err(|e| new_not_utf8_error(vm, &output, e))?; @@ -1607,8 +1595,16 @@ mod _csv { handle_res!(writer.terminator(&mut buffer[buffer_offset..])); } - let s = core::str::from_utf8(&buffer[..buffer_offset]) - .map_err(|e| new_not_utf8_error(vm, &buffer[..buffer_offset], e))?; + // csv-core just emitted the single-byte sentinel terminator (after + // closing the final quote / emitting an empty record as needed). + // Drop that sentinel byte and append the real, possibly + // multi-character, line terminator. + debug_assert_eq!(buffer[buffer_offset - 1], CSV_CORE_TERMINATOR_SENTINEL); + let mut output = buffer[..buffer_offset - 1].to_vec(); + output.extend_from_slice(self.dialect.lineterminator.as_bytes()); + + let s = + core::str::from_utf8(&output).map_err(|e| new_not_utf8_error(vm, &output, e))?; self.write.call((s,), vm) } diff --git a/extra_tests/snippets/stdlib_csv.py b/extra_tests/snippets/stdlib_csv.py index 61b82459a28..17fb05e9281 100644 --- a/extra_tests/snippets/stdlib_csv.py +++ b/extra_tests/snippets/stdlib_csv.py @@ -181,6 +181,74 @@ def test_quote_minimal_writer_lineterminator(): test_quote_minimal_writer_lineterminator() +def test_multichar_lineterminator(): + # https://github.com/RustPython/RustPython/issues/8322 + # The writer must store and emit a full multi-character line terminator. + for lineterminator in "\r\n", "\n", "\r", "!@#", "\0": + buf = io.StringIO() + writer = csv.writer(buf, lineterminator=lineterminator) + writer.writerow(["a", "b"]) + writer.writerow([1, 2]) + writer.writerow(["\r", "\n"]) + assert buf.getvalue() == ( + f'a,b{lineterminator}1,2{lineterminator}"\r","\n"{lineterminator}' + ), (lineterminator, buf.getvalue()) + + # A field is quoted when it contains any byte of the terminator (QUOTE_MINIMAL). + for field, expected in [ + ("a@b", '"a@b",x!@#'), + ("a!b", '"a!b",x!@#'), + ("a#b", '"a#b",x!@#'), + ("abc", "abc,x!@#"), + ]: + buf = io.StringIO() + csv.writer(buf, lineterminator="!@#").writerow([field, "x"]) + assert buf.getvalue() == expected, (field, buf.getvalue()) + + # The csv-core-backed QUOTE_ALL / QUOTE_NONNUMERIC paths emit the full + # terminator too, and keep the state machine correct across rows. + allq = io.StringIO() + writer = csv.writer(allq, lineterminator="!@#", quoting=csv.QUOTE_ALL) + writer.writerow(["a", "b"]) + writer.writerow(["c", "d"]) + assert allq.getvalue() == '"a","b"!@#"c","d"!@#', allq.getvalue() + + nonnum = io.StringIO() + csv.writer(nonnum, lineterminator="!@#", quoting=csv.QUOTE_NONNUMERIC).writerow( + ["a", 1] + ) + assert nonnum.getvalue() == '"a",1!@#', nonnum.getvalue() + + # QUOTE_NONE escapes any byte of the terminator. + none = io.StringIO() + csv.writer( + none, lineterminator="!@#", quoting=csv.QUOTE_NONE, escapechar="\\" + ).writerow(["a!b", "x"]) + assert none.getvalue() == "a\\!b,x!@#", none.getvalue() + + # register_dialect round-trips a multi-character terminator. + csv.register_dialect("multichar_lt", delimiter=",", lineterminator="!@#") + try: + reg = io.StringIO() + csv.writer(reg, dialect="multichar_lt").writerow(["a", "b"]) + assert reg.getvalue() == "a,b!@#", reg.getvalue() + finally: + csv.unregister_dialect("multichar_lt") + + # The dialect attribute reflects the full terminator. + assert ( + csv.writer(io.StringIO(), lineterminator="!@#").dialect.lineterminator == "!@#" + ) + + # The reader ignores lineterminator (like CPython) and only splits on \r\n. + assert list(csv.reader(io.StringIO("a,b!@#c,d!@#"), lineterminator="!@#")) == [ + ["a", "b!@#c", "d!@#"] + ] + + +test_multichar_lineterminator() + + def test_quote_minimal_writer_empty_fields(): buf = io.StringIO() writer = csv.writer(buf) From 6d5353c3f012da4ed2a28df843f461c82c950dcc Mon Sep 17 00:00:00 2001 From: jinmay Date: Mon, 20 Jul 2026 11:57:21 +0900 Subject: [PATCH 2/4] csv: unmark now-passing test_write_lineterminator --- Lib/test/test_csv.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_csv.py b/Lib/test/test_csv.py index 379f4c9b799..2b1de5d70d9 100644 --- a/Lib/test/test_csv.py +++ b/Lib/test/test_csv.py @@ -262,7 +262,6 @@ def test_write_escape(self): self._write_test(['C\\', '6', '7', 'X"'], 'C\\\\,6,7,"X"""', escapechar='\\', quoting=csv.QUOTE_MINIMAL) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_write_lineterminator(self): for lineterminator in '\r\n', '\n', '\r', '!@#', '\0': with self.subTest(lineterminator=lineterminator): From e8bb8873902cff566c06939eadcb24c8b79e74f3 Mon Sep 17 00:00:00 2001 From: jinmay Date: Mon, 20 Jul 2026 15:01:36 +0900 Subject: [PATCH 3/4] csv: address review feedback - Remove the redundant per-branch sentinel terminator setup in to_writer; the unconditional terminator call after the match is the single source. - Reword the empty-lineterminator errors to "must not be empty" (the constraint is non-empty, not single-character) on both entry points. - Promote the sentinel invariant check in writerow from debug_assert_eq! to assert_eq! so it also guards release builds. - Add a snippet case for a field containing a line-break byte to ensure the csv-core path drops only the trailing terminator. --- crates/stdlib/src/csv.rs | 12 +++++------- extra_tests/snippets/stdlib_csv.py | 9 +++++++++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/crates/stdlib/src/csv.rs b/crates/stdlib/src/csv.rs index 2163d0e2ebb..d3d71272b97 100644 --- a/crates/stdlib/src/csv.rs +++ b/crates/stdlib/src/csv.rs @@ -246,7 +246,7 @@ mod _csv { .to_str() .ok_or_else(|| new_csv_error(vm, r#""lineterminator" must be a string"#))?; if value.is_empty() { - return Err(new_csv_error(vm, r#""lineterminator" must be a string"#)); + return Err(new_csv_error(vm, r#""lineterminator" must not be empty"#)); } Ok(value.to_owned()) } @@ -641,7 +641,7 @@ mod _csv { // non-empty string, including multi-character ones, is stored. if value.is_empty() { return Err(vm - .new_type_error(r#""lineterminator" must be a 1-character string"#) + .new_type_error(r#""lineterminator" must not be empty"#) .into()); } res.lineterminator = Some(value.to_owned()); @@ -882,8 +882,7 @@ mod _csv { if let Some(dialect) = g.get(name) { let mut builder = builder .delimiter(dialect.delimiter) - .double_quote(dialect.doublequote) - .terminator(Terminator::Any(CSV_CORE_TERMINATOR_SENTINEL)); + .double_quote(dialect.doublequote); if let Some(t) = dialect.quotechar { builder = builder.quote(t); @@ -899,8 +898,7 @@ mod _csv { DialectItem::Obj(obj) => { let mut builder = builder .delimiter(obj.delimiter) - .double_quote(obj.doublequote) - .terminator(Terminator::Any(CSV_CORE_TERMINATOR_SENTINEL)); + .double_quote(obj.doublequote); if let Some(t) = obj.quotechar { builder = builder.quote(t); @@ -1599,7 +1597,7 @@ mod _csv { // closing the final quote / emitting an empty record as needed). // Drop that sentinel byte and append the real, possibly // multi-character, line terminator. - debug_assert_eq!(buffer[buffer_offset - 1], CSV_CORE_TERMINATOR_SENTINEL); + assert_eq!(buffer[buffer_offset - 1], CSV_CORE_TERMINATOR_SENTINEL); let mut output = buffer[..buffer_offset - 1].to_vec(); output.extend_from_slice(self.dialect.lineterminator.as_bytes()); diff --git a/extra_tests/snippets/stdlib_csv.py b/extra_tests/snippets/stdlib_csv.py index 17fb05e9281..d686672f1d8 100644 --- a/extra_tests/snippets/stdlib_csv.py +++ b/extra_tests/snippets/stdlib_csv.py @@ -219,6 +219,15 @@ def test_multichar_lineterminator(): ) assert nonnum.getvalue() == '"a",1!@#', nonnum.getvalue() + # A field that itself contains a line-break byte must be kept intact: the + # csv-core path drops only the trailing record terminator, not a byte from + # the field data. + embedded = io.StringIO() + csv.writer(embedded, lineterminator="!@#", quoting=csv.QUOTE_ALL).writerow( + ["x\ny", "z"] + ) + assert embedded.getvalue() == '"x\ny","z"!@#', embedded.getvalue() + # QUOTE_NONE escapes any byte of the terminator. none = io.StringIO() csv.writer( From 61e9782ac69bdb24c66479e14e23b7a401ea6710 Mon Sep 17 00:00:00 2001 From: jinmay Date: Mon, 27 Jul 2026 23:19:46 +0900 Subject: [PATCH 4/4] csv: reject non-ASCII lineterminator The writer decides what to quote and escape by comparing raw bytes, so a non-ASCII terminator quoted a field that merely shared a UTF-8 lead byte, and QUOTE_NONE escaped individual bytes of the terminator and then failed to decode the record back to a string. Reject non-ASCII terminators, including lone surrogates, as csv.Error when the dialect is parsed, and leave code-point-wise handling to a follow-up (#8310). The non-string error message now matches CPython as well. --- crates/stdlib/src/csv.rs | 45 +++++++++++++++++++++++++----- extra_tests/snippets/stdlib_csv.py | 37 ++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/crates/stdlib/src/csv.rs b/crates/stdlib/src/csv.rs index d3d71272b97..cd065f634f2 100644 --- a/crates/stdlib/src/csv.rs +++ b/crates/stdlib/src/csv.rs @@ -235,6 +235,30 @@ mod _csv { }) } + /// Validate that a line terminator is ASCII and return it as a `str`. + /// + /// The writer's quoting and escaping predicates compare raw bytes, so a + /// non-ASCII terminator would either quote a field that merely shares a + /// UTF-8 lead byte or splice an escape character into the middle of a + /// multi-byte sequence. Reject those here. + /// + /// The ASCII check must come before any UTF-8 conversion so that lone + /// surrogates are reported as this `csv.Error` too. + /// + /// TODO: RUSTPYTHON; handle non-ASCII terminators code-point-wise as part + /// of full Unicode dialect support. + fn ascii_lineterminator<'a>(vm: &VirtualMachine, s: &'a PyStr) -> PyResult<&'a str> { + if !s.as_wtf8().is_ascii() { + return Err(new_csv_error( + vm, + r#""lineterminator" must be an ASCII string"#, + )); + } + // An ASCII string is always valid UTF-8. + s.to_str() + .ok_or_else(|| new_csv_error(vm, r#""lineterminator" must be a string"#)) + } + fn prase_lineterminator_from_obj(vm: &VirtualMachine, obj: &PyObject) -> PyResult { match_class!(match obj.get_attr("lineterminator", vm)? { s @ PyStr => { @@ -242,9 +266,7 @@ mod _csv { // arbitrary-length terminator; the manual writer paths emit it // verbatim and the csv-core writer path appends it after a // sentinel terminator (see `writerow`). - let value = s - .to_str() - .ok_or_else(|| new_csv_error(vm, r#""lineterminator" must be a string"#))?; + let value = ascii_lineterminator(vm, &s)?; if value.is_empty() { return Err(new_csv_error(vm, r#""lineterminator" must not be empty"#)); } @@ -635,7 +657,13 @@ mod _csv { }; if let Some(lineterminator) = args.kwargs.swap_remove("lineterminator") { - let value = lineterminator.try_to_value::<&str>(vm)?; + let s = lineterminator.downcast_ref::().ok_or_else(|| { + vm.new_type_error(format!( + r#""lineterminator" must be a string, not {}"#, + lineterminator.class().name() + )) + })?; + let value = ascii_lineterminator(vm, s)?; // Preserve the previous behavior of rejecting an empty terminator // (full validation parity is deferred to a follow-up). Any // non-empty string, including multi-character ones, is stored. @@ -1353,9 +1381,12 @@ mod _csv { byte == dialect.delimiter || dialect.quotechar == Some(byte) || matches!(byte, b'\r' | b'\n') - // CPython quotes a field that contains any character of the line - // terminator. This byte-wise `contains` matches CPython for - // ASCII terminators; non-ASCII terminators are not fully handled. + // CPython quotes a field containing any character of the line + // terminator. The terminator is ASCII-validated at parse time, so + // comparing raw bytes cannot match part of a multi-byte character. + // TODO: RUSTPYTHON; supporting non-ASCII terminators needs + // code-point-wise quoting and escaping as part of full + // Unicode dialect support. || dialect.lineterminator.as_bytes().contains(&byte) }) } diff --git a/extra_tests/snippets/stdlib_csv.py b/extra_tests/snippets/stdlib_csv.py index d686672f1d8..418d383d84a 100644 --- a/extra_tests/snippets/stdlib_csv.py +++ b/extra_tests/snippets/stdlib_csv.py @@ -1,5 +1,6 @@ import csv import io +import sys from testutils import assert_raises @@ -258,6 +259,42 @@ def test_multichar_lineterminator(): test_multichar_lineterminator() +def test_reject_non_ascii_lineterminator(): + # CPython accepts non-ASCII line terminators; RustPython rejects them + # because the writer quotes and escapes byte by byte. Supporting them + # requires code-point-wise handling as part of full Unicode dialect support. + with assert_raises(csv.Error): + csv.writer(io.StringIO(), lineterminator="é") + + with assert_raises(csv.Error): + csv.writer(io.StringIO(), lineterminator="\x85") + + with assert_raises(csv.Error): + csv.writer(io.StringIO(), lineterminator="\ud800") + + with assert_raises(csv.Error): + csv.writer( + io.StringIO(), lineterminator="é", quoting=csv.QUOTE_NONE, escapechar="\\" + ) + + with assert_raises(csv.Error): + csv.register_dialect("non_ascii_lt", lineterminator="é") + + class NonAsciiDialect(csv.excel): + lineterminator = "é" + + with assert_raises(csv.Error): + NonAsciiDialect() + + buf = io.StringIO() + csv.writer(buf, lineterminator="!@#").writerow(["a", "b"]) + assert buf.getvalue() == "a,b!@#" + + +if sys.implementation.name == "rustpython": + test_reject_non_ascii_lineterminator() + + def test_quote_minimal_writer_empty_fields(): buf = io.StringIO() writer = csv.writer(buf)