From 99ce6b9fa3e10494e68ca7a0ab6cdf343a38bf50 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Fri, 1 Sep 2017 00:43:39 +0300 Subject: [PATCH 1/5] init commit --- Lib/test/test_imp.py | 16 ++++++++++++++++ .../2017-09-01-00-40-58.bpo-31315.ZX20bl.rst | 2 ++ Python/importdl.c | 5 +++++ 3 files changed, 23 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-09-01-00-40-58.bpo-31315.ZX20bl.rst diff --git a/Lib/test/test_imp.py b/Lib/test/test_imp.py index 6f35f49487ce7f9..e05897342862ab6 100644 --- a/Lib/test/test_imp.py +++ b/Lib/test/test_imp.py @@ -21,6 +21,12 @@ def requires_load_dynamic(meth): meth = support.cpython_only(meth) return unittest.skipIf(not hasattr(imp, 'load_dynamic'), 'imp.load_dynamic() required')(meth) +def requires_create_dynamic(meth): + """Decorator to skip a test if not running under CPython or lacking + imp.create_dynamic().""" + meth = support.cpython_only(meth) + return unittest.skipIf(not hasattr(imp, 'create_dynamic'), + 'imp.create_dynamic() required')(meth) @unittest.skipIf(_thread is None, '_thread module is required') @@ -318,6 +324,16 @@ def test_load_source(self): with self.assertRaisesRegex(ValueError, 'embedded null'): imp.load_source(__name__, __file__ + "\0") + @requires_create_dynamic + def test_issue31315(self): + # There shouldn't be an assertion failure in imp.create_dynamic(), + # when spec.name is not a string. + class BadSpec: + name = 42 + origin = 'foo' + with self.assertRaises(TypeError): + imp.create_dynamic(BadSpec()) + class ReloadTests(unittest.TestCase): diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-09-01-00-40-58.bpo-31315.ZX20bl.rst b/Misc/NEWS.d/next/Core and Builtins/2017-09-01-00-40-58.bpo-31315.ZX20bl.rst new file mode 100644 index 000000000000000..d13badbb358ff13 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-09-01-00-40-58.bpo-31315.ZX20bl.rst @@ -0,0 +1,2 @@ +Fix an assertion failure in imp.create_dynamic(), when spec.name is not a +string. Patch by Oren Milman. diff --git a/Python/importdl.c b/Python/importdl.c index d8656b943336ec1..ba43af65e766050 100644 --- a/Python/importdl.c +++ b/Python/importdl.c @@ -103,6 +103,11 @@ _PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp) if (name_unicode == NULL) { return NULL; } + if (!PyUnicode_Check(name_unicode)) { + PyErr_SetString(PyExc_TypeError, + "spec.name must be a string"); + goto error; + } name = get_encoded_name(name_unicode, &hook_prefix); if (name == NULL) { From 854d84b4862dcc52f46fbb63cda5f38b6ac9d424 Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Fri, 1 Sep 2017 14:40:57 +1000 Subject: [PATCH 2/5] Add missing blank line --- Lib/test/test_imp.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_imp.py b/Lib/test/test_imp.py index e05897342862ab6..41c44ee3fead733 100644 --- a/Lib/test/test_imp.py +++ b/Lib/test/test_imp.py @@ -21,6 +21,7 @@ def requires_load_dynamic(meth): meth = support.cpython_only(meth) return unittest.skipIf(not hasattr(imp, 'load_dynamic'), 'imp.load_dynamic() required')(meth) + def requires_create_dynamic(meth): """Decorator to skip a test if not running under CPython or lacking imp.create_dynamic().""" From b65be79a2d0f009609cd5d232a80958148b93780 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Fri, 1 Sep 2017 20:38:55 +0300 Subject: [PATCH 3/5] improve the test and its doc --- Lib/test/test_imp.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_imp.py b/Lib/test/test_imp.py index 41c44ee3fead733..c047a160da6dea2 100644 --- a/Lib/test/test_imp.py +++ b/Lib/test/test_imp.py @@ -23,11 +23,16 @@ def requires_load_dynamic(meth): 'imp.load_dynamic() required')(meth) def requires_create_dynamic(meth): - """Decorator to skip a test if not running under CPython or lacking - imp.create_dynamic().""" + """A decorator to skip tests that rely on imp.create_dynamic(). + + Some Python implementations might not provide imp.create_dynamic(). In + such implementations, this decorator causes decorated tests to be skipped + through the normal unittest mechanism. + """ meth = support.cpython_only(meth) - return unittest.skipIf(not hasattr(imp, 'create_dynamic'), - 'imp.create_dynamic() required')(meth) + supported = hasattr(imp, 'create_dynamic') + deco = unittest.skipIf(not supported, 'imp.create_dynamic() required') + return deco(meth) @unittest.skipIf(_thread is None, '_thread module is required') From 9107d1467bd61504a7653344867f297f3891638f Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Fri, 1 Sep 2017 22:22:09 +0300 Subject: [PATCH 4/5] improve the docstring --- Lib/test/test_imp.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_imp.py b/Lib/test/test_imp.py index c047a160da6dea2..f74c3958575cd32 100644 --- a/Lib/test/test_imp.py +++ b/Lib/test/test_imp.py @@ -23,11 +23,11 @@ def requires_load_dynamic(meth): 'imp.load_dynamic() required')(meth) def requires_create_dynamic(meth): - """A decorator to skip tests that rely on imp.create_dynamic(). + """A decorator for CPython-only tests that rely on imp.create_dynamic(). - Some Python implementations might not provide imp.create_dynamic(). In - such implementations, this decorator causes decorated tests to be skipped - through the normal unittest mechanism. + On some platforms, CPython might not provide imp.create_dynamic(). + In such cases and in other implementations, this decorator causes + decorated tests to be skipped through the normal unittest mechanism. """ meth = support.cpython_only(meth) supported = hasattr(imp, 'create_dynamic') From 64e68ff9380e17c7093989c480302ae0e34299d0 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sat, 9 Sep 2017 18:03:52 +0300 Subject: [PATCH 5/5] remove requires_create_dynamic(), and make the test more realistic --- Lib/test/test_imp.py | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_imp.py b/Lib/test/test_imp.py index f74c3958575cd32..d513eedc7bb4aa5 100644 --- a/Lib/test/test_imp.py +++ b/Lib/test/test_imp.py @@ -22,18 +22,6 @@ def requires_load_dynamic(meth): return unittest.skipIf(not hasattr(imp, 'load_dynamic'), 'imp.load_dynamic() required')(meth) -def requires_create_dynamic(meth): - """A decorator for CPython-only tests that rely on imp.create_dynamic(). - - On some platforms, CPython might not provide imp.create_dynamic(). - In such cases and in other implementations, this decorator causes - decorated tests to be skipped through the normal unittest mechanism. - """ - meth = support.cpython_only(meth) - supported = hasattr(imp, 'create_dynamic') - deco = unittest.skipIf(not supported, 'imp.create_dynamic() required') - return deco(meth) - @unittest.skipIf(_thread is None, '_thread module is required') class LockTests(unittest.TestCase): @@ -330,15 +318,16 @@ def test_load_source(self): with self.assertRaisesRegex(ValueError, 'embedded null'): imp.load_source(__name__, __file__ + "\0") - @requires_create_dynamic + @support.cpython_only def test_issue31315(self): # There shouldn't be an assertion failure in imp.create_dynamic(), # when spec.name is not a string. + create_dynamic = support.get_attribute(imp, 'create_dynamic') class BadSpec: - name = 42 + name = None origin = 'foo' with self.assertRaises(TypeError): - imp.create_dynamic(BadSpec()) + create_dynamic(BadSpec()) class ReloadTests(unittest.TestCase):