From 70ebe18539f87a19de53fed6be0eb240ebd79252 Mon Sep 17 00:00:00 2001 From: hyoinandout Date: Sat, 8 Aug 2026 00:55:44 +0900 Subject: [PATCH] Validate csv dialect conflicts with line terminators - Reject dialect characters that overlap any lineterminator character - Validate Dialect construction consistently and allow non-ASCII terminators - Unskip csv invalid character coverage now that validation matches behavior --- Lib/test/test_csv.py | 1 - crates/stdlib/src/csv.rs | 179 +++++++++++++-------------------------- 2 files changed, 60 insertions(+), 120 deletions(-) diff --git a/Lib/test/test_csv.py b/Lib/test/test_csv.py index 1585f35d4..0fbf026ae 100644 --- a/Lib/test/test_csv.py +++ b/Lib/test/test_csv.py @@ -1286,7 +1286,6 @@ class TestDialectValidity(unittest.TestCase): self.assertEqual(str(cm.exception), '"lineterminator" must be a string, not NoneType') - @unittest.expectedFailure # TODO: RUSTPYTHON def test_invalid_chars(self): def create_invalid(field_name, value, **kwargs): class mydialect(csv.Dialect): diff --git a/crates/stdlib/src/csv.rs b/crates/stdlib/src/csv.rs index 2eda90e19..bfe701b06 100644 --- a/crates/stdlib/src/csv.rs +++ b/crates/stdlib/src/csv.rs @@ -100,8 +100,10 @@ mod _csv { impl Constructor for PyDialect { type Args = PyObjectRef; - fn py_new(_cls: &Py, ctx: Self::Args, vm: &VirtualMachine) -> PyResult { - Self::try_from_object(vm, ctx) + fn py_new(_cls: &Py, obj: Self::Args, vm: &VirtualMachine) -> PyResult { + let dialect = Self::try_from_object(vm, obj)?; + validate_dialect(vm, &dialect)?; + Ok(dialect) } } @@ -240,26 +242,7 @@ 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. + fn parse_lineterminator<'a>(vm: &VirtualMachine, s: &'a PyStr) -> PyResult<&'a str> { s.to_str() .ok_or_else(|| new_csv_error(vm, r#""lineterminator" must be a string"#)) } @@ -271,7 +254,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 = ascii_lineterminator(vm, &s)?; + let value = parse_lineterminator(vm, &s)?; Ok(value.to_owned()) } attr => { @@ -492,7 +475,7 @@ mod _csv { write, state: PyMutex::new(WriteState { buffer: vec![0; 1024], - writer: options.to_writer(), + writer: FormatOptions::to_writer(&dialect), }), dialect, }) @@ -578,7 +561,7 @@ mod _csv { dialect: DialectItem, delimiter: Option, quotechar: Option>, - escapechar: Option, + escapechar: Option>, doublequote: Option, skipinitialspace: Option, lineterminator: Option, @@ -661,9 +644,10 @@ mod _csv { if let Some(escapechar) = args.kwargs.swap_remove("escapechar") { res.escapechar = match_class!(match escapechar { - s @ PyStr => Some(parse_single_char(&s, |_| { + s @ PyStr => Some(Some(parse_single_char(&s, |_| { vm.new_type_error(r#""escapechar" must be a 1-character string"#) - })?), + })?)), + PyNone => Some(None), _ => { return Err(ArgumentError::Exception( vm.new_type_error(r#""escapechar" must be a 1-character string"#), @@ -679,7 +663,7 @@ mod _csv { lineterminator.class().name() )) })?; - let value = ascii_lineterminator(vm, s)?; + let value = parse_lineterminator(vm, s)?; res.lineterminator = Some(value.to_owned()); }; @@ -786,27 +770,29 @@ mod _csv { )); } - let values = [ - ("delimiter", Some(core::slice::from_ref(&dialect.delimiter))), - ( - "quotechar", - dialect.quotechar.as_ref().map(core::slice::from_ref), - ), - ( - "escapechar", - dialect.escapechar.as_ref().map(core::slice::from_ref), - ), - ("lineterminator", Some(dialect.lineterminator.as_bytes())), + let values: [(&str, Option); 3] = [ + ("delimiter", Some(dialect.delimiter)), + ("quotechar", dialect.quotechar), + ("escapechar", dialect.escapechar), ]; for (index, (left_name, left)) in values.iter().enumerate() { - let Some(left) = left else { continue }; for (right_name, right) in values.iter().skip(index + 1) { - if right.as_ref() == Some(left) { + if left.is_some() && left == right { return Err(vm.new_value_error(format!( "{left_name} and {right_name} cannot be the same" ))); } } + if left.is_some_and(|value| { + dialect + .lineterminator + .chars() + .any(|character| character == value as char) + }) { + return Err(vm.new_value_error(format!( + "{left_name} and lineterminator cannot be the same" + ))); + } } Ok(()) } @@ -828,7 +814,7 @@ mod _csv { check_and_fill!(res, skipinitialspace); if let Some(t) = self.escapechar { - res.escapechar = Some(t); + res.escapechar = t; }; if let Some(t) = self.quotechar { @@ -865,81 +851,23 @@ mod _csv { Ok(dialect) } - fn get_quoting(&self) -> QuoteStyle { - let mut quoting = match &self.dialect { - DialectItem::Str(name) => { - let g = GLOBAL_HASHMAP.lock(); - if let Some(dialect) = g.get(name) { - dialect.quoting - } else { - QuoteStyle::Minimal - } - } - DialectItem::Obj(obj) => obj.quoting, - _ => QuoteStyle::Minimal, - }; - - if let Some(attr) = self.quoting { - quoting = attr - } - - quoting - } - - fn to_writer(&self) -> csv_core::Writer { + fn to_writer(dialect: &PyDialect) -> csv_core::Writer { let mut builder = csv_core::WriterBuilder::new(); - let mut writer = match &self.dialect { - DialectItem::Str(name) => { - let g = GLOBAL_HASHMAP.lock(); - if let Some(dialect) = g.get(name) { - let mut builder = builder - .delimiter(dialect.delimiter) - .double_quote(dialect.doublequote); + let mut writer = builder + .delimiter(dialect.delimiter) + .double_quote(dialect.doublequote); - if let Some(t) = dialect.quotechar { - builder = builder.quote(t); - } - - builder - - // TODO: RUSTPYTHON; Perfecting the remaining attributes. - } else { - &mut builder - } - } - DialectItem::Obj(obj) => { - let mut builder = builder - .delimiter(obj.delimiter) - .double_quote(obj.doublequote); - - if let Some(t) = obj.quotechar { - builder = builder.quote(t); - } - - builder - } - _ => &mut builder, - }; - - if let Some(t) = self.delimiter { - writer = writer.delimiter(t); - } - - if let Some(Some(t)) = self.quotechar { + if let Some(t) = dialect.quotechar { writer = writer.quote(t); } - if let Some(t) = self.doublequote { - writer = writer.double_quote(t); - } - writer = writer.terminator(Terminator::Any(CSV_CORE_TERMINATOR_SENTINEL)); - if let Some(e) = self.escapechar { + if let Some(e) = dialect.escapechar { writer = writer.escape(e); } - writer = writer.quote_style(self.get_quoting().into()); + writer = writer.quote_style(dialect.quoting.into()); writer.build() } @@ -1355,39 +1283,52 @@ mod _csv { dialect: &PyDialect, vm: &VirtualMachine, ) -> PyResult<()> { - for &byte in data { - if field_needs_escape(byte, dialect) { + let mut data = data; + while let Some((&byte, rest)) = data.split_first() { + if field_needs_escape(data, 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); + data = rest; } Ok(()) } + fn data_contains_lineterminator_char(data: &[u8], dialect: &PyDialect) -> bool { + dialect.lineterminator.chars().any(|character| { + let mut encoded = [0; 4]; + let character = character.encode_utf8(&mut encoded).as_bytes(); + data.windows(character.len()) + .any(|window| window == character) + }) + } + + fn data_starts_with_lineterminator_char(data: &[u8], dialect: &PyDialect) -> bool { + dialect.lineterminator.chars().any(|character| { + let mut encoded = [0; 4]; + let character = character.encode_utf8(&mut encoded).as_bytes(); + data.starts_with(character) + }) + } + 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') - // 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) - }) + }) || data_contains_lineterminator_char(data, dialect) } - fn field_needs_escape(byte: u8, dialect: &PyDialect) -> bool { + fn field_needs_escape(data: &[u8], dialect: &PyDialect) -> bool { + let byte = data[0]; byte == dialect.delimiter || dialect.quotechar == Some(byte) || dialect.escapechar == Some(byte) || matches!(byte, b'\r' | b'\n') - || dialect.lineterminator.as_bytes().contains(&byte) + || data_starts_with_lineterminator_char(data, dialect) } fn write_lineterminator(output: &mut Vec, terminator: &str) {