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

bpo-36495: Fix two out-of-bounds array reads #12641

Merged
merged 3 commits into from
Apr 1, 2019

Conversation

bradlarsen
Copy link
Contributor

@bradlarsen bradlarsen commented Mar 31, 2019

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

@@ -0,0 +1 @@
Fix two out-of-bound reads in the code that constructs abstract syntax trees. Patch by Brad Larsen.
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure this fix needs a NEWS entry, especially in the Security section. The bug was introduces at the alpha stage, nobody should use it in production.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree; I was originally going to leave that blank, but bedevere-bot said one was needed (or I didn't understand how to skip that check).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I revert the commit that adds the NEWS entry?

@bradlarsen
Copy link
Contributor Author

It looks like that Azure Pipelines Ubuntu job has been usually failing for a while now?

@tirkarthi
Copy link
Member

It looks like that Azure Pipelines Ubuntu job has been usually failing for a while now?

It's a known issue tracked at #12625

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Great research. Let's fix these, but let's get rid of the NEWS item.

This reverts commit 6f90ef3.

No need for a NEWS entry on a prerelease bugfix.
@bradlarsen
Copy link
Contributor Author

Great! I've reverted the NEWS item commit, so I think this PR is ready now.

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.

6 participants