Skip to content

/ cpython

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-39421: Fix posible crash in heapq with custom comparison operators #18118

Merged
merged 3 commits into from Jan 23, 2020
Merged

bpo-39421: Fix posible crash in heapq with custom comparison operators #18118

merged 3 commits into from Jan 23, 2020
+59 −9

Conversation

@pablogsal
Copy link
Member

pablogsal commented Jan 22, 2020 •
edited by bedevere-bot

https://bugs.python.org/issue39421
@pablogsal
bpo-39421: Fix posible crash in heapq with custom comparison operators
Loading status checks…
d026117
@pablogsal pablogsal requested a review from rhettinger as a code owner Jan 22, 2020
@corona10
corona10 suggested changes Jan 22, 2020
View changes
Copy link
Member

corona10 left a comment 

import heapq

class h(int):
    def __lt__(self, o):
        list2.clear()
        return NotImplemented

class g(int):
    def __lt__(self, o):
        list1.clear()
        return NotImplemented

list1, list2 = [], []

heapq.heappush(list1, h(0))
heapq.heappush(list2, g(0))

print(list1)
print(list2)

heapq.heappush(list1, g(1))
heapq.heappush(list2, h(1))
print(list1)
print(list2)

This PR does not fix this case. Please take a look
@pablogsal
fixup! bpo-39421: Fix posible crash in heapq with custom comparison o…
Loading status checks…
9718f66 
…perators
@pablogsal

This comment has been minimized.

Sign in to view
Copy link
Member Author

pablogsal commented Jan 22, 2020

This PR does not fix this case. Please take a look

Thanks! I added a test in 9718f66
@pablogsal
fixup! fixup! bpo-39421: Fix posible crash in heapq with custom compa…
Loading status checks…
03d0c9c 
…rison operators
@corona10
corona10 approved these changes Jan 22, 2020
View changes
Copy link
Member

corona10 left a comment •
edited

LGTM

Just comment:
In some time, if we can pass objects as a variadic argument.
The code might be cleaner. (I don't deeply think about the possibility)

e.g. Py_INCREF(a, b, c...); Py_DECREF(a, b, c...);
@pablogsal

This comment has been minimized.

Sign in to view
Copy link
Member Author

pablogsal commented Jan 22, 2020 •
edited

e.g. Py_INCREF(a, b, c...); Py_DECREF(a, b, c...);

We would need a different function as C does not allow overloads (for va_list in this case) and I don't think is that common
@pablogsal

This comment has been minimized.

Sign in to view
Copy link
Member Author

pablogsal commented Jan 22, 2020

I would like to wait to see if @vstinner and @methane would prefer to add the check into PyObject_RichCompareBool instead although I think it would be too costly.
@corona10 corona10 requested review from vstinner and methane Jan 22, 2020
@methane
methane approved these changes Jan 23, 2020
View changes
@alex

This comment has been minimized.

Sign in to view
Copy link
Member

alex commented Jan 23, 2020

This should be backported to all branches that are still doing security releases.
@pablogsal pablogsal merged commit 79f89e6 into python:master Jan 23, 2020
9 checks passed
9 checks passed
Docs
Details
Windows (x86)
Details
Windows (x64)
Details
macOS
Details
Ubuntu
Details
Azure Pipelines PR #20200122.25 succeeded
Details
bedevere/issue-number Issue number 39421 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pablogsal pablogsal deleted the pablogsal:bpo-39421 branch Jan 23, 2020
@miss-islington

This comment has been minimized.

Sign in to view
Copy link

miss-islington commented Jan 23, 2020

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7, 3.8.
🐍🍒⛏🤖
miss-islington added a commit to miss-islington/cpython that referenced this pull request Jan 23, 2020
@pablogsal @miss-islington
bpo-39421: Fix posible crash in heapq with custom comparison operators (
826423d 
pythonGH-18118)

* bpo-39421: Fix posible crash in heapq with custom comparison operators

* fixup! bpo-39421: Fix posible crash in heapq with custom comparison operators

* fixup! fixup! bpo-39421: Fix posible crash in heapq with custom comparison operators
(cherry picked from commit 79f89e6)

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
miss-islington added a commit to miss-islington/cpython that referenced this pull request Jan 23, 2020
@pablogsal @miss-islington
bpo-39421: Fix posible crash in heapq with custom comparison operators (
7bb22f6 
pythonGH-18118)

* bpo-39421: Fix posible crash in heapq with custom comparison operators

* fixup! bpo-39421: Fix posible crash in heapq with custom comparison operators

* fixup! fixup! bpo-39421: Fix posible crash in heapq with custom comparison operators
(cherry picked from commit 79f89e6)

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

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jan 23, 2020

GH-18145 is a backport of this pull request to the 3.7 branch.
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jan 23, 2020

GH-18144 is a backport of this pull request to the 3.8 branch.
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jan 23, 2020

GH-18146 is a backport of this pull request to the 3.6 branch.
miss-islington added a commit to miss-islington/cpython that referenced this pull request Jan 23, 2020
@pablogsal @miss-islington
bpo-39421: Fix posible crash in heapq with custom comparison operators (
509a1d8 
pythonGH-18118)

* bpo-39421: Fix posible crash in heapq with custom comparison operators

* fixup! bpo-39421: Fix posible crash in heapq with custom comparison operators

* fixup! fixup! bpo-39421: Fix posible crash in heapq with custom comparison operators
(cherry picked from commit 79f89e6)

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
miss-islington added a commit that referenced this pull request Jan 23, 2020
@miss-islington @pablogsal
bpo-39421: Fix posible crash in heapq with custom comparison operators (
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
958064f 
GH-18118)

* bpo-39421: Fix posible crash in heapq with custom comparison operators

* fixup! bpo-39421: Fix posible crash in heapq with custom comparison operators

* fixup! fixup! bpo-39421: Fix posible crash in heapq with custom comparison operators
(cherry picked from commit 79f89e6)

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
ned-deily added a commit that referenced this pull request Jan 23, 2020
@miss-islington @pablogsal @ned-deily
bpo-39421: Fix posible crash in heapq with custom comparison operators (
Loading status checks…
c563f40 
GH-18118) (GH-18146)

(cherry picked from commit 79f89e6)

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
@miss-islington

This comment has been minimized.

Sign in to view
Copy link

miss-islington commented Jan 23, 2020

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖
miss-islington added a commit to miss-islington/cpython that referenced this pull request Jan 23, 2020
@pablogsal @miss-islington
bpo-39421: Fix posible crash in heapq with custom comparison operators (
194108d 
pythonGH-18118)

* bpo-39421: Fix posible crash in heapq with custom comparison operators

* fixup! bpo-39421: Fix posible crash in heapq with custom comparison operators

* fixup! fixup! bpo-39421: Fix posible crash in heapq with custom comparison operators
(cherry picked from commit 79f89e6)

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

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jan 23, 2020

GH-18149 is a backport of this pull request to the 3.8 branch.
miss-islington added a commit that referenced this pull request Jan 23, 2020
@miss-islington @pablogsal
bpo-39421: Fix posible crash in heapq with custom comparison operators (
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
993811f 
GH-18118)

* bpo-39421: Fix posible crash in heapq with custom comparison operators

* fixup! bpo-39421: Fix posible crash in heapq with custom comparison operators

* fixup! fixup! bpo-39421: Fix posible crash in heapq with custom comparison operators
(cherry picked from commit 79f89e6)

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@methane methane

@corona10 corona10

@vstinner vstinner

@rhettinger rhettinger

Assignees
No one assigned
Labels
CLA signed
Projects
None yet
Milestone
No milestone
7 participants
@pablogsal @alex @miss-islington @bedevere-bot @methane @corona10 @the-knights-who-say-ni
You can’t perform that action at this time.