Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions Lib/test/test_interpreters/test_queues.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import importlib
import pickle
import sys
import threading
import types
from textwrap import dedent
import unittest

Expand Down Expand Up @@ -381,6 +383,40 @@ def test_put_get_full_fallback(self):
self.assertEqual(obj, obj2)
self.assertIsNot(obj, obj2)

def _check_unpickle_attributeerror_arg(self, arg):
# Put an object through a queue where get() must re-import its class
# via a module __getattr__ that raises AttributeError(arg).
modname = '_test_xi_attrerr'
mod = types.ModuleType(modname)
class Thing:
pass
Thing.__module__ = modname
Thing.__qualname__ = 'Thing'
mod.Thing = Thing
sys.modules[modname] = mod
self.addCleanup(sys.modules.pop, modname, None)
queue = queues.create()
queue.put(Thing())
del mod.Thing
def raise_attributeerror(name):
raise AttributeError(arg)
mod.__getattr__ = raise_attributeerror
with self.assertRaises(interpreters.NotShareableError):
queue.get()

def test_get_unpickle_fails_with_bad_attributeerror_arg(self):
# gh-151862: an AttributeError arg that can't be UTF-8 encoded used
# to crash (NULL deref); covers both non-str and surrogate-str args.
for arg in [42, b'x', None, '\ud800']:
with self.subTest(arg=arg):
self._check_unpickle_attributeerror_arg(arg)

def test_get_unpickle_fails_with_str_attributeerror_arg(self):
# Positive control: a normal str arg must not crash, locking in the
# non-NULL strncmp() path.
with self.subTest(arg='boom'):
self._check_unpickle_attributeerror_arg('boom')

def test_put_get_same_interpreter(self):
interp = interpreters.create()
interp.exec(dedent("""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fixed a crash (``NULL`` dereference) when an object passed between
interpreters via :mod:`concurrent.interpreters` fails to unpickle with an
:exc:`AttributeError` whose first argument is not a string.
5 changes: 5 additions & 0 deletions Python/crossinterp.c
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,11 @@ check_missing___main___attr(PyObject *exc)
}
}
const char *err = PyUnicode_AsUTF8(msgobj);
if (err == NULL) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should add another PyUnicode_Check before this call.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need the result check in any case, because PyUnicode_AsUTF8() can fail for other reasons -- embedded surrogates or failed memory allocation. If the errors were not silenced, PyUnicode_Check() would be more preferable, but in the current code it would not have advantage.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I proposed PyUnicode_Check not as opposite, but as an addition to current check.

PyErr_Clear();
Py_DECREF(msgobj);
return 0;
}

// Check if it's a missing __main__ attr.
int cmp = strncmp(err, "module '__main__' has no attribute '", 36);
Expand Down
Loading