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
This commit is contained in:
hyoinandout committed 2026-08-08 00:55:44 +09:00
1 parent 5e2da0cdba
commit 70ebe18539
2 files changed
+60 -120

No files matched your search

-1
View File
@@ -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):
+60 -119
View File
@@ -100,8 +100,10 @@ mod _csv {
impl Constructor for PyDialect {
type Args = PyObjectRef;
fn py_new(_cls: &Py<PyType>, ctx: Self::Args, vm: &VirtualMachine) -> PyResult<Self> {
Self::try_from_object(vm, ctx)
fn py_new(_cls: &Py<PyType>, obj: Self::Args, vm: &VirtualMachine) -> PyResult<Self> {
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<u8>,
quotechar: Option<Option<u8>>,
escapechar: Option<u8>,
escapechar: Option<Option<u8>>,
doublequote: Option<bool>,
skipinitialspace: Option<bool>,
lineterminator: Option<String>,
@@ -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<u8>); 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<u8>, terminator: &str) {