Skip to content

gh-97556: Raise null bytes syntax error upon null in multiline string #104136

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

Merged
merged 2 commits into from
May 4, 2023

Conversation

lysnikolaou
Copy link
Member

@lysnikolaou lysnikolaou commented May 3, 2023

Copy link
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

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

LGTM

Maybe we should add sone asserts here and there that tok->done is not error to prevent this issue from happening in other places

@lysnikolaou
Copy link
Member Author

lysnikolaou commented May 4, 2023

Maybe we should add sone asserts here and there that tok->done is not error to prevent this issue from happening in other places

That's a good idea. Where would you add these asserts though, since most of the time we shouldn't be asserting, but rather check for it?

One of the items I have in my to-do list is to add comments and assert in various places in the tokenizer to make the logic a bit easier to follow. Would it be okay to merge this as-is and then do that in a separate PR?

@pablogsal
Copy link
Member

Would it be okay to merge this as-is and then do that in a separate PR?

Absolutely, let's merge this as is 🚀

Great job as always man!

@lysnikolaou lysnikolaou merged commit ef0df52 into python:main May 4, 2023
@lysnikolaou lysnikolaou deleted the null-bytes-multiline-string branch May 4, 2023 12:26
@vstinner
Copy link
Member

vstinner commented May 4, 2023

@lysnikolaou: Do you consider backporting your change to Python 3.11?

@bskinn
Copy link
Contributor

bskinn commented May 4, 2023

See discussion linked in the associated issue, there was a decision made not to backport the initial change that worked for single quoted strings, to avoid breaking compatibility in a patch version release.

@bskinn
Copy link
Contributor

bskinn commented May 4, 2023

See #96670 (comment)

@lysnikolaou lysnikolaou added the needs backport to 3.11 only security fixes label May 4, 2023
@lysnikolaou lysnikolaou removed the needs backport to 3.11 only security fixes label May 4, 2023
@lysnikolaou
Copy link
Member Author

FYI I removed the label until it's more clear what should be backported.

@python python deleted a comment from miss-islington May 5, 2023
@python python deleted a comment from miss-islington May 5, 2023
@lysnikolaou
Copy link
Member Author

GH-104195 includes a backport of this commit to 3.11.

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

Successfully merging this pull request may close these issues.

5 participants