GH-82565: Fixed a possible assertion error

Lib/test/pickletester.py

@@ -3482,6 +3482,79 @@ def __init__(self): pass
         self.assertRaises(pickle.PicklingError, BadPickler().dump, 0)
         self.assertRaises(pickle.UnpicklingError, BadUnpickler().load)
 
+    def test_unpickler_bad_file(self):
+        # bpo-38384: Crash in _pickle if the read attribute raises an error.
+        def raises_oserror(self, *args, **kwargs):
+            raise OSError
+        @property
+        def bad_property(self):
+            1/0
+
+        # File without read
+        class F:
+            readline = raises_oserror
+        self.assertRaises((AttributeError, TypeError), self.Unpickler, F())
+
+        # File without readline
+        class F:
+            read = raises_oserror
+        self.assertRaises((AttributeError, TypeError), self.Unpickler, F())
+
+        # File with bad read
+        class F:
+            read = bad_property
+            readline = raises_oserror
+        self.assertRaises(ZeroDivisionError, self.Unpickler, F())
+
+        # File with bad readline
+        class F:
+            readline = bad_property
+            read = raises_oserror
+        self.assertRaises(ZeroDivisionError, self.Unpickler, F())
+
+        # File with bad read and without readline
+        class F: 
+            read = bad_property
+        self.assertRaises(ZeroDivisionError, self.Unpickler, F())
+
+        # File with bad readline and without read
+        class F: 
+            readline = bad_property
+        self.assertRaises(ZeroDivisionError, self.Unpickler, F())
+
+        # File with bad peek
+        class F:
+            peek = bad_property
+            read = raises_oserror
+            readline = raises_oserror
+        try:
+            self.Unpickler(F())
+        except ZeroDivisionError:
+            pass
+
+        # File with bad readinto
+        class F:
+            readinto = bad_property
+            read = raises_oserror
+            readline = raises_oserror
+        try:
+            self.Unpickler(F())
+        except ZeroDivisionError:
+            pass
+
+    def test_pickler_bad_file(self):
+        # File without write
+        class F:
+            pass
+        self.assertRaises(TypeError, self.Pickler, F())
+
+        # File with bad write
+        class F:
+            @property
+            def write(self):
+                1/0
+        self.assertRaises(ZeroDivisionError, self.Pickler, F())
+
     def check_dumps_loads_oob_buffers(self, dumps, loads):
         # No need to do the full gamut of tests here, just enough to
         # check that dumps() and loads() redirect their arguments

Misc/NEWS.d/next/Tests/2023-04-03-06-16-37.gh-issue-82565.6SpJ0f.rst

@@ -0,0 +1,2 @@
+Fix a crash in the C implementation of :mod:`pickle` if an error occurred
+while looking up the ``read``  and ``readline`` attribute of the file.

Modules/_pickle.c

@@ -1632,16 +1632,21 @@ _Unpickler_New(void)
 static int
 _Unpickler_SetInputStream(UnpicklerObject *self, PyObject *file)
 {
+    _Py_IDENTIFIER(peek);
+    _Py_IDENTIFIER(read);
+    _Py_IDENTIFIER(readinto);
+    _Py_IDENTIFIER(readline);
+
     /* Optional file methods */
     if (_PyObject_LookupAttr(file, &_Py_ID(peek), &self->peek) < 0) {
         return -1;
     }
     if (_PyObject_LookupAttr(file, &_Py_ID(readinto), &self->readinto) < 0) {
         return -1;
     }
-    (void)_PyObject_LookupAttr(file, &_Py_ID(read), &self->read);
-    (void)_PyObject_LookupAttr(file, &_Py_ID(readline), &self->readline);
-    if (!self->readline || !self->read) {
+    if (_PyObject_LookupAttrId(file, &PyId_read, &self->read) <= 0 ||
+        _PyObject_LookupAttrId(file, &PyId_readline, &self->readline) <= 0) 
+    {
         if (!PyErr_Occurred()) {
             PyErr_SetString(PyExc_TypeError,
                             "file must have 'read' and 'readline' attributes");