Skip to content

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Oct 7, 2021

@serhiy-storchaka serhiy-storchaka added the type-feature A feature request or enhancement label Oct 7, 2021
@serhiy-storchaka serhiy-storchaka changed the title bpo-27580: Add support of null characters in :mod:csv. bpo-27580: Add support of null characters in csv Oct 7, 2021
@serhiy-storchaka serhiy-storchaka requested review from encukou and removed request for encukou October 7, 2021 18:54
@serhiy-storchaka
Copy link
Member Author

@smontanaro, please take a look. For unknown reasons I cannot add you to reviewers.

@smontanaro
Copy link
Contributor

For unknown reasons I cannot add you to reviewers.

Maybe that's by design. :-)

LGTM. I assume Doc/library/csv.rst will be updated at some point to make it clear that ASCII NUL is now a valid.

@ammaraskar
Copy link
Member

You don't have to necessarily do it here, but just a note to myself that we should update the csv fuzzer:

/* Ignore non null-terminated strings since _csv can't handle
embedded nulls */
if (memchr(data, '\0', size) == NULL) {
return 0;
}

to generate inputs with nulls, such as in the json fuzzer here:

PyObject* input_bytes = PyBytes_FromStringAndSize(data, size);
if (input_bytes == NULL) {
return 0;
}

in this or a follow up PR.

Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

lgtm

@serhiy-storchaka
Copy link
Member Author

I assume Doc/library/csv.rst will be updated at some point to make it clear that ASCII NUL is now a valid.

I have not found a good place for this. It was not documented that ASCII NUL is special. Perhaps a NEWS entry will be enough.

@serhiy-storchaka
Copy link
Member Author

@ammaraskar, I have no experience with fuzzers. Feel free to add a new test if it is useful.

@merwok
Copy link
Member

merwok commented Nov 8, 2022

#97503 is requesting a backport to 3.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants