bpo-38384: Fix a possible assertion failure in _pickle #16606
Conversation
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.
|
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 |
|
Minimal reproducer (but you can add class F:
@property
def read(self):
1/0
import pickle
pickle.load(F()) |
|
@ZackerySpytz Ping? |
|
Hi @ZackerySpytz! This is looking good! Make sure to also add a NEWS entry (you can use blurb-it). |
@ZackerySpytz has added a test. Shall we merge this? |
Lib/test/pickletester.py
Outdated
| @@ -3136,6 +3136,18 @@ def __init__(self): pass | |||
| self.assertRaises(pickle.PicklingError, BadPickler().dump, 0) | |||
| self.assertRaises(pickle.UnpicklingError, BadUnpickler().load) | |||
|
|
|||
| def test_load_read_exception(self): | |||
There was a problem hiding this comment.
readinto will not be tested if read raises an error. You need six tests for classes which do not have one of these attributes or raise error for it but have other two attributes.
Modules/_pickle.c
Outdated
| if (_PyObject_LookupAttrId(file, &PyId_read, &self->read) <= 0) { | ||
| goto error; | ||
| } | ||
| if (_PyObject_LookupAttrId(file, &PyId_readinto, &self->readinto) <= 0) { |
There was a problem hiding this comment.
readinto is optional now.
Ifs for read and readline can be merged.
|
LGTM. @serhiy-storchaka |
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