Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-96670: Raise SyntaxError when parsing NULL bytes #97594

Merged
merged 1 commit into from Sep 27, 2022

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Sep 27, 2022

Copy link
Member

@gvanrossum gvanrossum left a comment

What's up with python3.12.abi.new having 22,644 additions?

Copy link
Member

@gvanrossum gvanrossum left a comment

LG except for one word in an error message, I think.

Parser/tokenizer.c Outdated Show resolved Hide resolved
@pablogsal
Copy link
Member Author

pablogsal commented Sep 27, 2022

What's up with python3.12.abi.new having 22,644 additions?

Wow, that's some error. Let me clean that up

Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
@gvanrossum
Copy link
Member

gvanrossum commented Sep 27, 2022

Ugh, force-push. :-(

Copy link
Member

@gvanrossum gvanrossum left a comment

I see nothing else wrong, let's wait for Lysandros' LGTM.

Copy link
Contributor

@lysnikolaou lysnikolaou left a comment

LGTM as well!

@mdboom
Copy link
Contributor

mdboom commented Sep 27, 2022

I was going to ask "doesn't this break things for encodings where NULLs are normal, like UTF-16", but then realized we never supported that anyway:

PEP 253: "It does not include encodings which use two or more bytes for all characters like e.g. UTF-16. The reason for this is to keep the encoding detection algorithm in the tokenizer simple."

In that sense, we get a better error message for UTF-16 files with this PR than before. Prior, it would silently fail (behave as if an empty file), and with this PR:

  File "x.py", line 1
    #
SyntaxError: source code cannot contain null bytes

@pablogsal pablogsal merged commit aab01e3 into python:main Sep 27, 2022
14 checks passed
@pablogsal pablogsal deleted the gh-96670 branch Sep 27, 2022
@pablogsal
Copy link
Member Author

pablogsal commented Sep 27, 2022

Landing, thank you everyone for your great review and comments! And also thank @mdboom for checking the UTF-16, that was a good consideration :)

P.S. Sorry for the force push :S

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants