gh-103333: Pickle the keyword attributes of AttributeError (#103352)

* Pickle the `name` and `args` attributes of AttributeError when present.

Co-authored-by: Gregory P. Smith <greg@krypto.org>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py
index 4ef7decfbc263e2554fce42c1e29307680040784..f3554f1c4bb3f38df0a5c3778f5051add196e795 100644 (file)
--- a/Lib/test/test_exceptions.py
+++ b/Lib/test/test_exceptions.py
@@ -417,45 +417,45 @@ class ExceptionTests(unittest.TestCase):
         # test that exception attributes are happy
 
         exceptionList = [
-            (BaseException, (), {'args' : ()}),
-            (BaseException, (1, ), {'args' : (1,)}),
-            (BaseException, ('foo',),
+            (BaseException, (), {}, {'args' : ()}),
+            (BaseException, (1, ), {}, {'args' : (1,)}),
+            (BaseException, ('foo',), {},
                 {'args' : ('foo',)}),
-            (BaseException, ('foo', 1),
+            (BaseException, ('foo', 1), {},
                 {'args' : ('foo', 1)}),
-            (SystemExit, ('foo',),
+            (SystemExit, ('foo',), {},
                 {'args' : ('foo',), 'code' : 'foo'}),
-            (OSError, ('foo',),
+            (OSError, ('foo',), {},
                 {'args' : ('foo',), 'filename' : None, 'filename2' : None,
                  'errno' : None, 'strerror' : None}),
-            (OSError, ('foo', 'bar'),
+            (OSError, ('foo', 'bar'), {},
                 {'args' : ('foo', 'bar'),
                  'filename' : None, 'filename2' : None,
                  'errno' : 'foo', 'strerror' : 'bar'}),
-            (OSError, ('foo', 'bar', 'baz'),
+            (OSError, ('foo', 'bar', 'baz'), {},
                 {'args' : ('foo', 'bar'),
                  'filename' : 'baz', 'filename2' : None,
                  'errno' : 'foo', 'strerror' : 'bar'}),
-            (OSError, ('foo', 'bar', 'baz', None, 'quux'),
+            (OSError, ('foo', 'bar', 'baz', None, 'quux'), {},
                 {'args' : ('foo', 'bar'), 'filename' : 'baz', 'filename2': 'quux'}),
-            (OSError, ('errnoStr', 'strErrorStr', 'filenameStr'),
+            (OSError, ('errnoStr', 'strErrorStr', 'filenameStr'), {},
                 {'args' : ('errnoStr', 'strErrorStr'),
                  'strerror' : 'strErrorStr', 'errno' : 'errnoStr',
                  'filename' : 'filenameStr'}),
-            (OSError, (1, 'strErrorStr', 'filenameStr'),
+            (OSError, (1, 'strErrorStr', 'filenameStr'), {},
                 {'args' : (1, 'strErrorStr'), 'errno' : 1,
                  'strerror' : 'strErrorStr',
                  'filename' : 'filenameStr', 'filename2' : None}),
-            (SyntaxError, (), {'msg' : None, 'text' : None,
+            (SyntaxError, (), {}, {'msg' : None, 'text' : None,
                 'filename' : None, 'lineno' : None, 'offset' : None,
                 'end_offset': None, 'print_file_and_line' : None}),
-            (SyntaxError, ('msgStr',),
+            (SyntaxError, ('msgStr',), {},
                 {'args' : ('msgStr',), 'text' : None,
                  'print_file_and_line' : None, 'msg' : 'msgStr',
                  'filename' : None, 'lineno' : None, 'offset' : None,
                  'end_offset': None}),
             (SyntaxError, ('msgStr', ('filenameStr', 'linenoStr', 'offsetStr',
-                           'textStr', 'endLinenoStr', 'endOffsetStr')),
+                           'textStr', 'endLinenoStr', 'endOffsetStr')), {},
                 {'offset' : 'offsetStr', 'text' : 'textStr',
                  'args' : ('msgStr', ('filenameStr', 'linenoStr',
                                       'offsetStr', 'textStr',
@@ -465,7 +465,7 @@ class ExceptionTests(unittest.TestCase):
                  'end_lineno': 'endLinenoStr', 'end_offset': 'endOffsetStr'}),
             (SyntaxError, ('msgStr', 'filenameStr', 'linenoStr', 'offsetStr',
                            'textStr', 'endLinenoStr', 'endOffsetStr',
-                           'print_file_and_lineStr'),
+                           'print_file_and_lineStr'), {},
                 {'text' : None,
                  'args' : ('msgStr', 'filenameStr', 'linenoStr', 'offsetStr',
                            'textStr', 'endLinenoStr', 'endOffsetStr',
@@ -473,38 +473,40 @@ class ExceptionTests(unittest.TestCase):
                  'print_file_and_line' : None, 'msg' : 'msgStr',
                  'filename' : None, 'lineno' : None, 'offset' : None,
                  'end_lineno': None, 'end_offset': None}),
-            (UnicodeError, (), {'args' : (),}),
+            (UnicodeError, (), {}, {'args' : (),}),
             (UnicodeEncodeError, ('ascii', 'a', 0, 1,
-                                  'ordinal not in range'),
+                                  'ordinal not in range'), {},
                 {'args' : ('ascii', 'a', 0, 1,
                                            'ordinal not in range'),
                  'encoding' : 'ascii', 'object' : 'a',
                  'start' : 0, 'reason' : 'ordinal not in range'}),
             (UnicodeDecodeError, ('ascii', bytearray(b'\xff'), 0, 1,
-                                  'ordinal not in range'),
+                                  'ordinal not in range'), {},
                 {'args' : ('ascii', bytearray(b'\xff'), 0, 1,
                                            'ordinal not in range'),
                  'encoding' : 'ascii', 'object' : b'\xff',
                  'start' : 0, 'reason' : 'ordinal not in range'}),
             (UnicodeDecodeError, ('ascii', b'\xff', 0, 1,
-                                  'ordinal not in range'),
+                                  'ordinal not in range'), {},
                 {'args' : ('ascii', b'\xff', 0, 1,
                                            'ordinal not in range'),
                  'encoding' : 'ascii', 'object' : b'\xff',
                  'start' : 0, 'reason' : 'ordinal not in range'}),
-            (UnicodeTranslateError, ("\u3042", 0, 1, "ouch"),
+            (UnicodeTranslateError, ("\u3042", 0, 1, "ouch"), {},
                 {'args' : ('\u3042', 0, 1, 'ouch'),
                  'object' : '\u3042', 'reason' : 'ouch',
                  'start' : 0, 'end' : 1}),
-            (NaiveException, ('foo',),
+            (NaiveException, ('foo',), {},
                 {'args': ('foo',), 'x': 'foo'}),
-            (SlottedNaiveException, ('foo',),
+            (SlottedNaiveException, ('foo',), {},
                 {'args': ('foo',), 'x': 'foo'}),
+            (AttributeError, ('foo',), dict(name='name', obj='obj'),
+                dict(args=('foo',), name='name', obj='obj')),
         ]
         try:
             # More tests are in test_WindowsError
             exceptionList.append(
-                (WindowsError, (1, 'strErrorStr', 'filenameStr'),
+                (WindowsError, (1, 'strErrorStr', 'filenameStr'), {},
                     {'args' : (1, 'strErrorStr'),
                      'strerror' : 'strErrorStr', 'winerror' : None,
                      'errno' : 1,
@@ -513,11 +515,11 @@ class ExceptionTests(unittest.TestCase):
         except NameError:
             pass
 
-        for exc, args, expected in exceptionList:
+        for exc, args, kwargs, expected in exceptionList:
             try:
-                e = exc(*args)
+                e = exc(*args, **kwargs)
             except:
-                print("\nexc=%r, args=%r" % (exc, args), file=sys.stderr)
+                print(f"\nexc={exc!r}, args={args!r}", file=sys.stderr)
                 # raise
             else:
                 # Verify module name
@@ -540,7 +542,12 @@ class ExceptionTests(unittest.TestCase):
                         new = p.loads(s)
                         for checkArgName in expected:
                             got = repr(getattr(new, checkArgName))
-                            want = repr(expected[checkArgName])
+                            if exc == AttributeError and checkArgName == 'obj':
+                                # See GH-103352, we're not pickling
+                                # obj at this point. So verify it's None.
+                                want = repr(None)
+                            else:
+                                want = repr(expected[checkArgName])
                             self.assertEqual(got, want,
                                              'pickled "%r", attribute "%s' %
                                              (e, checkArgName))

diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst b/Misc/NEWS.d/next/Core and Builtins/2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst
new file mode 100644 (file)
index 0000000..793f02c
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst
@@ -0,0 +1 @@
+:exc:`AttributeError` now retains the ``name`` attribute when pickled and unpickled.

diff --git a/Objects/exceptions.c b/Objects/exceptions.c
index ba5ee291f08b0c042a9e25f294e1268dd6bdea0b..59c63f4aa44958c7ac7b319907ffd339f8f0da0b 100644 (file)
--- a/Objects/exceptions.c
+++ b/Objects/exceptions.c
@@ -2287,6 +2287,46 @@ AttributeError_traverse(PyAttributeErrorObject *self, visitproc visit, void *arg
     return BaseException_traverse((PyBaseExceptionObject *)self, visit, arg);
 }
 
+/* Pickling support */
+static PyObject *
+AttributeError_getstate(PyAttributeErrorObject *self, PyObject *Py_UNUSED(ignored))
+{
+    PyObject *dict = ((PyAttributeErrorObject *)self)->dict;
+    if (self->name || self->args) {
+        dict = dict ? PyDict_Copy(dict) : PyDict_New();
+        if (dict == NULL) {
+            return NULL;
+        }
+        if (self->name && PyDict_SetItemString(dict, "name", self->name) < 0) {
+            Py_DECREF(dict);
+            return NULL;
+        }
+        /* We specifically are not pickling the obj attribute since there are many
+        cases where it is unlikely to be picklable. See GH-103352.
+        */
+        if (self->args && PyDict_SetItemString(dict, "args", self->args) < 0) {
+            Py_DECREF(dict);
+            return NULL;
+        }
+        return dict;
+    }
+    else if (dict) {
+        return Py_NewRef(dict);
+    }
+    Py_RETURN_NONE;
+}
+
+static PyObject *
+AttributeError_reduce(PyAttributeErrorObject *self, PyObject *Py_UNUSED(ignored))
+{
+    PyObject *state = AttributeError_getstate(self, NULL);
+    if (state == NULL) {
+        return NULL;
+    }
+
+    return PyTuple_Pack(3, Py_TYPE(self), self->args, state);
+}
+
 static PyMemberDef AttributeError_members[] = {
     {"name", T_OBJECT, offsetof(PyAttributeErrorObject, name), 0, PyDoc_STR("attribute name")},
     {"obj", T_OBJECT, offsetof(PyAttributeErrorObject, obj), 0, PyDoc_STR("object")},
@@ -2294,7 +2334,9 @@ static PyMemberDef AttributeError_members[] = {
 };
 
 static PyMethodDef AttributeError_methods[] = {
-    {NULL}  /* Sentinel */
+    {"__getstate__", (PyCFunction)AttributeError_getstate, METH_NOARGS},
+    {"__reduce__", (PyCFunction)AttributeError_reduce, METH_NOARGS },
+    {NULL}
 };
 
 ComplexExtendsException(PyExc_Exception, AttributeError,