gh-82565: Fix a possible assertion failure in _pickle by ZackerySpytz · Pull Request #16606 · python/cpython

ZackerySpytz · 2019-10-06T13:41:57Z

In _Unpickler_SetInputStream(), _PyObject_LookupAttrId() is called
three times in a row without any error handling. If an exception
occurs during the first or second call, _PyObject_LookupAttrId()
will be called with a live exception.

https://bugs.python.org/issue38384

Issue: An assertion failure in test_pickle #82565

In _Unpickler_SetInputStream(), _PyObject_LookupAttrId() is called three times in a row without any error handling. If an exception occurs during the first or second call, _PyObject_LookupAttrId() will be called with a live exception.

serhiy-storchaka

Please add a test.

bedevere-bot · 2019-10-06T16:36:04Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

serhiy-storchaka · 2019-10-21T06:44:32Z

Minimal reproducer (but you can add readline and readinto):

classF:
@propertydefread(self):
1/0importpicklepickle.load(F())

taleinat · 2019-11-13T22:47:13Z

@ZackerySpytz Ping?

…assert

taleinat · 2019-11-22T23:12:18Z

Hi @ZackerySpytz! This is looking good!

Make sure to also add a NEWS entry (you can use blurb-it).

iritkatriel · 2021-10-17T23:01:00Z

Please add a test.

@ZackerySpytz has added a test. Shall we merge this?

furkanonder · 2022-12-22T12:19:53Z

LGTM. @serhiy-storchaka

serhiy-storchaka

LGTM.

Please add few more test cases:

no read, no readline
read = bad_property, no readline
readline = bad_property, no read

serhiy-storchaka · 2023-12-01T12:35:39Z

Sorry, I forget to merge this PR at its time, so the original issue was fixed in #105667.

But the tests may still be useful.

miss-islington-app · 2023-12-01T13:36:41Z

Thanks @ZackerySpytz for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

…honGH-16606) (cherry picked from commit 0584443) Co-authored-by: Zackery Spytz <zspytz@gmail.com>

bedevere-app · 2023-12-01T13:36:49Z

GH-112591 is a backport of this pull request to the 3.12 branch.

bedevere-app · 2023-12-01T13:36:53Z

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

…honGH-16606) (cherry picked from commit 0584443) Co-authored-by: Zackery Spytz <zspytz@gmail.com>

…-16606) (GH-112591) (cherry picked from commit 0584443) Co-authored-by: Zackery Spytz <zspytz@gmail.com>

…-16606) (GH-112592) (cherry picked from commit 0584443) Co-authored-by: Zackery Spytz <zspytz@gmail.com>

…honGH-16606)

the-knights-who-say-ni added the CLA signed label Oct 6, 2019

bedevere-bot added the awaiting review label Oct 6, 2019

serhiy-storchaka requested changes Oct 6, 2019
View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Oct 6, 2019

ZackerySpytz added 2 commits November 22, 2019 09:10

Merge remote-tracking branch 'upstream/master' into bpo-38384-pickle-…
0898bd6
…assert

Add a test.
b81c706

serhiy-storchaka reviewed Oct 19, 2021
View reviewed changes

Comment threadLib/test/pickletester.py Outdated
Comment threadModules/_pickle.c Outdated

serhiy-storchaka added 3 commits October 20, 2021 21:25

Merge branch 'main' into bpo-38384-pickle-assert
d739b61

Rewrite test.
ccaeb97

Add a NEWS entry and a test for pickler
bf18cec

ZackerySpytzmannequin mentioned this pull request Apr 10, 2022
An assertion failure in test_pickle #82565
Closed

ezio-melotti removed the CLA signed label Jul 13, 2022

bumper-furkan-onder approved these changes Dec 22, 2022
View reviewed changes

Modify a test and the news entry (which weren't written by me).
6e24487

furkanonder approved these changes Dec 22, 2022
View reviewed changes

serhiy-storchaka reviewed Dec 22, 2022
View reviewed changes

SahillMulani mentioned this pull request Apr 3, 2023
GH-82565: Fixed a possible assertion error #103206
Closed

hauntsaninja changed the title ~~bpo-38384: Fix a possible assertion failure in _pickle~~gh-82565: Fix a possible assertion failure in _pickleApr 3, 2023

arhadthedev added 2 commits April 20, 2023 22:55

Merge branch 'main' into bpo-38384-pickle-assert
cca3253

Address the comment of Serhiy
cd5f794

arhadthedev added extension-modules C modules in the Modules dir 3.11 only security fixes labels Apr 20, 2023

arhadthedev added the 3.12 only security fixes label Apr 20, 2023

arhadthedevand others added 2 commits April 21, 2023 06:40

_PyObject_LookupAttrId -> _PyObject_LookupAttr
dd343e1

Merge branch 'main' into bpo-38384-pickle-assert
12aeed8

serhiy-storchaka added 2 commits December 1, 2023 14:35

Fix tests.
644a1cf

Remove a NEWS entry.
57f6095

serhiy-storchaka approved these changes Dec 1, 2023
View reviewed changes

bedevere-appBot added awaiting merge and removed awaiting changes labels Dec 1, 2023

serhiy-storchaka enabled auto-merge (squash) December 1, 2023 12:39

serhiy-storchaka added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes tests Tests in the Lib/test dir skip news and removed 3.11 only security fixes 3.12 only security fixes extension-modules C modules in the Modules dir labels Dec 1, 2023

serhiy-storchaka merged commit 0584443 into python:mainDec 1, 2023

bedevere-appBot removed the awaiting merge label Dec 1, 2023

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 1, 2023
pythongh-82565: Add tests for pickle and unpickle with bad files (pyt…
c3caf2b
…honGH-16606) (cherry picked from commit 0584443) Co-authored-by: Zackery Spytz <zspytz@gmail.com>

bedevere-appBot removed the needs backport to 3.12 only security fixes label Dec 1, 2023

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 1, 2023
pythongh-82565: Add tests for pickle and unpickle with bad files (pyt…
f860fd7
…honGH-16606) (cherry picked from commit 0584443) Co-authored-by: Zackery Spytz <zspytz@gmail.com>

bedevere-appBot removed the needs backport to 3.11 only security fixes label Dec 1, 2023

serhiy-storchaka pushed a commit that referenced this pull request Dec 1, 2023
[3.12]gh-82565: Add tests for pickle and unpickle with bad files (GH…
edce0c4
…-16606) (GH-112591) (cherry picked from commit 0584443) Co-authored-by: Zackery Spytz <zspytz@gmail.com>

serhiy-storchaka pushed a commit that referenced this pull request Dec 1, 2023
[3.11]gh-82565: Add tests for pickle and unpickle with bad files (GH…
d838c7a
…-16606) (GH-112592) (cherry picked from commit 0584443) Co-authored-by: Zackery Spytz <zspytz@gmail.com>

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
pythongh-82565: Add tests for pickle and unpickle with bad files (pyt…
1fd2c8a
…honGH-16606)

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
pythongh-82565: Add tests for pickle and unpickle with bad files (pyt…
ffe1825
…honGH-16606)

Uh oh!

Uh oh!

Conversation

ZackerySpytz commented Oct 6, 2019• edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Oct 6, 2019

Uh oh!

serhiy-storchaka commented Oct 21, 2019

Uh oh!

taleinat commented Nov 13, 2019

Uh oh!

taleinat commented Nov 22, 2019

Uh oh!

iritkatriel commented Oct 17, 2021

Uh oh!

Uh oh!

Uh oh!

furkanonder commented Dec 22, 2022

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Dec 1, 2023

Uh oh!

miss-islington-appBot commented Dec 1, 2023

Uh oh!

bedevere-appBot commented Dec 1, 2023

Uh oh!

bedevere-appBot commented Dec 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

ZackerySpytz commented Oct 6, 2019•
edited by bedevere-bot
Loading