Skip to content

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

Merged
pablogsal merged 1 commit into
python:mainfrom
pablogsal:gh-96670
Sep 27, 2022
Merged

gh-96670: Raise SyntaxError when parsing NULL bytes#97594
pablogsal merged 1 commit into
python:mainfrom
pablogsal:gh-96670

Conversation

@pablogsal

@pablogsal pablogsal commented Sep 27, 2022

Copy link
Copy Markdown
Member

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Comment thread Parser/tokenizer.c Outdated
@pablogsal

Copy link
Copy Markdown
Member Author

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

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

Comment thread Objects/fileobject.c
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
@gvanrossum

Copy link
Copy Markdown
Member

Ugh, force-push. :-(

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@lysnikolaou lysnikolaou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM as well!

@mdboom

mdboom commented Sep 27, 2022

Copy link
Copy Markdown
Contributor

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
@pablogsal
pablogsal deleted the gh-96670 branch September 27, 2022 22:23
@pablogsal

Copy link
Copy Markdown
Member Author

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

lysnikolaou added a commit to lysnikolaou/cpython that referenced this pull request May 5, 2023
…honGH-97594).

(cherry picked from commit aab01e3)

Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
@bedevere-bot

Copy link
Copy Markdown

GH-104195 is a backport of this pull request to the 3.11 branch.

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.

5 participants