From 214b7515d87e1d9d747c5b047a95ddf4a09d1cd4 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 16 Jan 2025 12:59:12 +0100 Subject: [PATCH 01/10] gh-128911: Add tests on the PyImport C API * Add Modules/_testlimitedcapi/import.c * Add Lib/test/test_capi/test_import.py * Remove _testcapi.check_pyimport_addmodule(): tests already covered by newly added tests. --- Lib/test/test_capi/test_import.py | 125 ++++++++++++++++++++++++++ Lib/test/test_import/__init__.py | 24 ----- Modules/Setup.stdlib.in | 2 +- Modules/_testcapimodule.c | 47 ---------- Modules/_testlimitedcapi.c | 3 + Modules/_testlimitedcapi/import.c | 140 ++++++++++++++++++++++++++++++ Modules/_testlimitedcapi/parts.h | 1 + 7 files changed, 270 insertions(+), 72 deletions(-) create mode 100644 Lib/test/test_capi/test_import.py create mode 100644 Modules/_testlimitedcapi/import.c diff --git a/Lib/test/test_capi/test_import.py b/Lib/test/test_capi/test_import.py new file mode 100644 index 00000000000000..9f00b367d4a0f6 --- /dev/null +++ b/Lib/test/test_capi/test_import.py @@ -0,0 +1,125 @@ +import importlib.util +import sys +import types +import unittest +from test.support import import_helper +from test.support.warnings_helper import check_warnings + +_testlimitedcapi = import_helper.import_module('_testlimitedcapi') + + +class ImportTests(unittest.TestCase): + def test_getmagicnumber(self): + # Test PyImport_GetMagicNumber() + magic = _testlimitedcapi.PyImport_GetMagicNumber() + self.assertEqual(magic, + int.from_bytes(importlib.util.MAGIC_NUMBER, 'little')) + + def test_getmagictag(self): + # Test PyImport_GetMagicTag() + tag = _testlimitedcapi.PyImport_GetMagicTag() + self.assertEqual(tag, sys.implementation.cache_tag) + + def test_getmoduledict(self): + # Test PyImport_GetModuleDict() + modules = _testlimitedcapi.PyImport_GetModuleDict() + self.assertIs(modules, sys.modules) + + def check_import_loaded_module(self, import_module): + for name in ('os', 'sys', 'test', 'unittest'): + with self.subTest(name=name): + self.assertIn(name, sys.modules) + module = import_module(name) + self.assertIsInstance(module, types.ModuleType) + self.assertIs(module, sys.modules[name]) + + def check_import_fresh_module(self, import_module): + old_modules = dict(sys.modules) + try: + for name in ('asyncio', 'colorsys', 'datetime'): + with self.subTest(name=name): + sys.modules.pop(name, None) + module = import_module(name) + self.assertIsInstance(module, types.ModuleType) + self.assertIs(module, sys.modules[name]) + self.assertEqual(module.__name__, name) + finally: + sys.modules.clear() + sys.modules.update(old_modules) + + def test_getmodule(self): + # Test PyImport_GetModule() + self.check_import_loaded_module(_testlimitedcapi.PyImport_GetModule) + + nonexistent = 'nonexistent' + self.assertNotIn(nonexistent, sys.modules) + self.assertIsNone(_testlimitedcapi.PyImport_GetModule(nonexistent)) + self.assertIsNone(_testlimitedcapi.PyImport_GetModule('')) + self.assertIsNone(_testlimitedcapi.PyImport_GetModule(object())) + + def check_addmodule(self, add_module): + # create a new module + name = 'nonexistent' + self.assertNotIn(name, sys.modules) + try: + module = add_module(name) + self.assertIsInstance(module, types.ModuleType) + self.assertIs(module, sys.modules[name]) + finally: + sys.modules.pop(name, None) + + # get an existing module + self.check_import_loaded_module(add_module) + + def test_addmoduleobject(self): + # Test PyImport_AddModuleObject() + self.check_addmodule(_testlimitedcapi.PyImport_AddModuleObject) + + def test_addmodule(self): + # Test PyImport_AddModule() + self.check_addmodule(_testlimitedcapi.PyImport_AddModule) + + def test_addmoduleref(self): + # Test PyImport_AddModuleRef() + self.check_addmodule(_testlimitedcapi.PyImport_AddModuleRef) + + def check_import_func(self, import_module): + self.check_import_loaded_module(import_module) + self.check_import_fresh_module(import_module) + + # Invalid module name types + with self.assertRaises(TypeError): + import_module(123) + with self.assertRaises(TypeError): + import_module(object()) + + def test_import(self): + # Test PyImport_Import() + self.check_import_func(_testlimitedcapi.PyImport_Import) + + def test_importmodule(self): + # Test PyImport_ImportModule() + self.check_import_func(_testlimitedcapi.PyImport_ImportModule) + + def test_importmodulenoblock(self): + # Test deprecated PyImport_ImportModuleNoBlock() + with check_warnings(('', DeprecationWarning)): + self.check_import_func(_testlimitedcapi.PyImport_ImportModuleNoBlock) + + # TODO: test PyImport_ExecCodeModule() + # TODO: test PyImport_ExecCodeModuleEx() + # TODO: test PyImport_ExecCodeModuleWithPathnames() + # TODO: test PyImport_ExecCodeModuleObject() + # TODO: test PyImport_ImportModuleLevel() + # TODO: test PyImport_ImportModuleLevelObject() + # TODO: test PyImport_ImportModuleEx() + # TODO: test PyImport_GetImporter() + # TODO: test PyImport_ReloadModule() + # TODO: test PyImport_ImportFrozenModuleObject() + # TODO: test PyImport_ImportFrozenModule() + # TODO: test PyImport_AppendInittab() + # TODO: test PyImport_ExtendInittab() + + +if __name__ == "__main__": + unittest.main() diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index c2cec6444cb43a..1e706023c795b6 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -3311,30 +3311,6 @@ def test_basic_multiple_interpreters_reset_each(self): # * module's global state was initialized, not reset -@cpython_only -class CAPITests(unittest.TestCase): - def test_pyimport_addmodule(self): - # gh-105922: Test PyImport_AddModuleRef(), PyImport_AddModule() - # and PyImport_AddModuleObject() - _testcapi = import_module("_testcapi") - for name in ( - 'sys', # frozen module - 'test', # package - __name__, # package.module - ): - _testcapi.check_pyimport_addmodule(name) - - def test_pyimport_addmodule_create(self): - # gh-105922: Test PyImport_AddModuleRef(), create a new module - _testcapi = import_module("_testcapi") - name = 'dontexist' - self.assertNotIn(name, sys.modules) - self.addCleanup(unload, name) - - mod = _testcapi.check_pyimport_addmodule(name) - self.assertIs(mod, sys.modules[name]) - - @cpython_only class TestMagicNumber(unittest.TestCase): def test_magic_number_endianness(self): diff --git a/Modules/Setup.stdlib.in b/Modules/Setup.stdlib.in index b7357f41768a2f..6b6a8ae57a5119 100644 --- a/Modules/Setup.stdlib.in +++ b/Modules/Setup.stdlib.in @@ -163,7 +163,7 @@ @MODULE__TESTBUFFER_TRUE@_testbuffer _testbuffer.c @MODULE__TESTINTERNALCAPI_TRUE@_testinternalcapi _testinternalcapi.c _testinternalcapi/test_lock.c _testinternalcapi/pytime.c _testinternalcapi/set.c _testinternalcapi/test_critical_sections.c @MODULE__TESTCAPI_TRUE@_testcapi _testcapimodule.c _testcapi/vectorcall.c _testcapi/heaptype.c _testcapi/abstract.c _testcapi/unicode.c _testcapi/dict.c _testcapi/set.c _testcapi/list.c _testcapi/tuple.c _testcapi/getargs.c _testcapi/datetime.c _testcapi/docstring.c _testcapi/mem.c _testcapi/watchers.c _testcapi/long.c _testcapi/float.c _testcapi/complex.c _testcapi/numbers.c _testcapi/structmember.c _testcapi/exceptions.c _testcapi/code.c _testcapi/buffer.c _testcapi/pyatomic.c _testcapi/run.c _testcapi/file.c _testcapi/codec.c _testcapi/immortal.c _testcapi/gc.c _testcapi/hash.c _testcapi/time.c _testcapi/bytes.c _testcapi/object.c _testcapi/monitoring.c _testcapi/config.c -@MODULE__TESTLIMITEDCAPI_TRUE@_testlimitedcapi _testlimitedcapi.c _testlimitedcapi/abstract.c _testlimitedcapi/bytearray.c _testlimitedcapi/bytes.c _testlimitedcapi/codec.c _testlimitedcapi/complex.c _testlimitedcapi/dict.c _testlimitedcapi/eval.c _testlimitedcapi/float.c _testlimitedcapi/heaptype_relative.c _testlimitedcapi/list.c _testlimitedcapi/long.c _testlimitedcapi/object.c _testlimitedcapi/pyos.c _testlimitedcapi/set.c _testlimitedcapi/sys.c _testlimitedcapi/tuple.c _testlimitedcapi/unicode.c _testlimitedcapi/vectorcall_limited.c _testlimitedcapi/version.c +@MODULE__TESTLIMITEDCAPI_TRUE@_testlimitedcapi _testlimitedcapi.c _testlimitedcapi/abstract.c _testlimitedcapi/bytearray.c _testlimitedcapi/bytes.c _testlimitedcapi/codec.c _testlimitedcapi/complex.c _testlimitedcapi/dict.c _testlimitedcapi/eval.c _testlimitedcapi/float.c _testlimitedcapi/heaptype_relative.c _testlimitedcapi/import.c _testlimitedcapi/list.c _testlimitedcapi/long.c _testlimitedcapi/object.c _testlimitedcapi/pyos.c _testlimitedcapi/set.c _testlimitedcapi/sys.c _testlimitedcapi/tuple.c _testlimitedcapi/unicode.c _testlimitedcapi/vectorcall_limited.c _testlimitedcapi/version.c @MODULE__TESTCLINIC_TRUE@_testclinic _testclinic.c @MODULE__TESTCLINIC_LIMITED_TRUE@_testclinic_limited _testclinic_limited.c diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index a0a1f8af6710a3..c4e83417956929 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3059,52 +3059,6 @@ function_set_closure(PyObject *self, PyObject *args) Py_RETURN_NONE; } -static PyObject * -check_pyimport_addmodule(PyObject *self, PyObject *args) -{ - const char *name; - if (!PyArg_ParseTuple(args, "s", &name)) { - return NULL; - } - - // test PyImport_AddModuleRef() - PyObject *module = PyImport_AddModuleRef(name); - if (module == NULL) { - return NULL; - } - assert(PyModule_Check(module)); - // module is a strong reference - - // test PyImport_AddModule() - PyObject *module2 = PyImport_AddModule(name); - if (module2 == NULL) { - goto error; - } - assert(PyModule_Check(module2)); - assert(module2 == module); - // module2 is a borrowed ref - - // test PyImport_AddModuleObject() - PyObject *name_obj = PyUnicode_FromString(name); - if (name_obj == NULL) { - goto error; - } - PyObject *module3 = PyImport_AddModuleObject(name_obj); - Py_DECREF(name_obj); - if (module3 == NULL) { - goto error; - } - assert(PyModule_Check(module3)); - assert(module3 == module); - // module3 is a borrowed ref - - return module; - -error: - Py_DECREF(module); - return NULL; -} - static PyObject * test_weakref_capi(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) @@ -3570,7 +3524,6 @@ static PyMethodDef TestMethods[] = { {"function_set_kw_defaults", function_set_kw_defaults, METH_VARARGS, NULL}, {"function_get_closure", function_get_closure, METH_O, NULL}, {"function_set_closure", function_set_closure, METH_VARARGS, NULL}, - {"check_pyimport_addmodule", check_pyimport_addmodule, METH_VARARGS}, {"test_weakref_capi", test_weakref_capi, METH_NOARGS}, {"function_set_warning", function_set_warning, METH_NOARGS}, {"test_critical_sections", test_critical_sections, METH_NOARGS}, diff --git a/Modules/_testlimitedcapi.c b/Modules/_testlimitedcapi.c index bcc69a339ec5c4..82dac1c999470f 100644 --- a/Modules/_testlimitedcapi.c +++ b/Modules/_testlimitedcapi.c @@ -56,6 +56,9 @@ PyInit__testlimitedcapi(void) if (_PyTestLimitedCAPI_Init_HeaptypeRelative(mod) < 0) { return NULL; } + if (_PyTestLimitedCAPI_Init_Import(mod) < 0) { + return NULL; + } if (_PyTestLimitedCAPI_Init_List(mod) < 0) { return NULL; } diff --git a/Modules/_testlimitedcapi/import.c b/Modules/_testlimitedcapi/import.c new file mode 100644 index 00000000000000..01c2574d17bbfa --- /dev/null +++ b/Modules/_testlimitedcapi/import.c @@ -0,0 +1,140 @@ +// Need limited C API version 3.7 for PyImport_GetModule() +#include "pyconfig.h" // Py_GIL_DISABLED +#if !defined(Py_GIL_DISABLED) && !defined(Py_LIMITED_API) +# define Py_LIMITED_API 0x030d0000 +#endif + +#include "parts.h" +#include "util.h" + + +/* Test PyImport_GetMagicNumber() */ +static PyObject * +pyimport_getmagicnumber(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) +{ + long magic = PyImport_GetMagicNumber(); + return PyLong_FromLong(magic); +} + + +/* Test PyImport_GetMagicTag() */ +static PyObject * +pyimport_getmagictag(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) +{ + const char *tag = PyImport_GetMagicTag(); + return PyUnicode_FromString(tag); +} + + +/* Test PyImport_GetModuleDict() */ +static PyObject * +pyimport_getmoduledict(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) +{ + return Py_XNewRef(PyImport_GetModuleDict()); +} + + +/* Test PyImport_GetModule() */ +static PyObject * +pyimport_getmodule(PyObject *Py_UNUSED(module), PyObject *name) +{ + assert(!PyErr_Occurred()); + PyObject *module = PyImport_GetModule(name); + if (module == NULL && !PyErr_Occurred()) { + Py_RETURN_NONE; + } + return module; +} + + +/* Test PyImport_AddModuleObject() */ +static PyObject * +pyimport_addmoduleobject(PyObject *Py_UNUSED(module), PyObject *name) +{ + return Py_XNewRef(PyImport_AddModuleObject(name)); +} + + +/* Test PyImport_AddModule() */ +static PyObject * +pyimport_addmodule(PyObject *Py_UNUSED(module), PyObject *args) +{ + const char *name; + if (!PyArg_ParseTuple(args, "s", &name)) { + return NULL; + } + + return Py_XNewRef(PyImport_AddModule(name)); +} + + +/* Test PyImport_AddModuleRef() */ +static PyObject * +pyimport_addmoduleref(PyObject *Py_UNUSED(module), PyObject *args) +{ + const char *name; + if (!PyArg_ParseTuple(args, "s", &name)) { + return NULL; + } + + return PyImport_AddModuleRef(name); +} + + +/* Test PyImport_Import() */ +static PyObject * +pyimport_import(PyObject *Py_UNUSED(module), PyObject *name) +{ + return PyImport_Import(name); +} + + +/* Test PyImport_ImportModule() */ +static PyObject * +pyimport_importmodule(PyObject *Py_UNUSED(module), PyObject *args) +{ + const char *name; + if (!PyArg_ParseTuple(args, "s", &name)) { + return NULL; + } + + return PyImport_ImportModule(name); +} + + +/* Test PyImport_ImportModuleNoBlock() */ +static PyObject * +pyimport_importmodulenoblock(PyObject *Py_UNUSED(module), PyObject *args) +{ + const char *name; + if (!PyArg_ParseTuple(args, "s", &name)) { + return NULL; + } + + _Py_COMP_DIAG_PUSH + _Py_COMP_DIAG_IGNORE_DEPR_DECLS + return PyImport_ImportModuleNoBlock(name); + _Py_COMP_DIAG_POP +} + + +static PyMethodDef test_methods[] = { + {"PyImport_GetMagicNumber", pyimport_getmagicnumber, METH_NOARGS}, + {"PyImport_GetMagicTag", pyimport_getmagictag, METH_NOARGS}, + {"PyImport_GetModuleDict", pyimport_getmoduledict, METH_NOARGS}, + {"PyImport_GetModule", pyimport_getmodule, METH_O}, + {"PyImport_AddModuleObject", pyimport_addmoduleobject, METH_O}, + {"PyImport_AddModule", pyimport_addmodule, METH_VARARGS}, + {"PyImport_AddModuleRef", pyimport_addmoduleref, METH_VARARGS}, + {"PyImport_Import", pyimport_import, METH_O}, + {"PyImport_ImportModule", pyimport_importmodule, METH_VARARGS}, + {"PyImport_ImportModuleNoBlock", pyimport_importmodulenoblock, METH_VARARGS}, + {NULL}, +}; + + +int +_PyTestLimitedCAPI_Init_Import(PyObject *module) +{ + return PyModule_AddFunctions(module, test_methods); +} diff --git a/Modules/_testlimitedcapi/parts.h b/Modules/_testlimitedcapi/parts.h index 56d566b66565a3..9efcd8dcb71e5b 100644 --- a/Modules/_testlimitedcapi/parts.h +++ b/Modules/_testlimitedcapi/parts.h @@ -31,6 +31,7 @@ int _PyTestLimitedCAPI_Init_Dict(PyObject *module); int _PyTestLimitedCAPI_Init_Eval(PyObject *module); int _PyTestLimitedCAPI_Init_Float(PyObject *module); int _PyTestLimitedCAPI_Init_HeaptypeRelative(PyObject *module); +int _PyTestLimitedCAPI_Init_Import(PyObject *module); int _PyTestLimitedCAPI_Init_Object(PyObject *module); int _PyTestLimitedCAPI_Init_List(PyObject *module); int _PyTestLimitedCAPI_Init_Long(PyObject *module); From 482471f27615772c87efa5a284594c7db83eeb67 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 16 Jan 2025 14:18:48 +0100 Subject: [PATCH 02/10] Avoid importing asyncio package --- Lib/test/test_capi/test_import.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_capi/test_import.py b/Lib/test/test_capi/test_import.py index 9f00b367d4a0f6..bf38f8fbd15d31 100644 --- a/Lib/test/test_capi/test_import.py +++ b/Lib/test/test_capi/test_import.py @@ -36,7 +36,7 @@ def check_import_loaded_module(self, import_module): def check_import_fresh_module(self, import_module): old_modules = dict(sys.modules) try: - for name in ('asyncio', 'colorsys', 'datetime'): + for name in ('colorsys', 'datetime', 'math'): with self.subTest(name=name): sys.modules.pop(name, None) module = import_module(name) From fc2f8cdfdd04c3816c3a8fcb8898c0163608f40e Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 16 Jan 2025 14:23:59 +0100 Subject: [PATCH 03/10] Update Windows build --- PCbuild/_testlimitedcapi.vcxproj | 1 + PCbuild/_testlimitedcapi.vcxproj.filters | 1 + 2 files changed, 2 insertions(+) diff --git a/PCbuild/_testlimitedcapi.vcxproj b/PCbuild/_testlimitedcapi.vcxproj index 0ea5edba3aa9a7..87abff52493098 100644 --- a/PCbuild/_testlimitedcapi.vcxproj +++ b/PCbuild/_testlimitedcapi.vcxproj @@ -103,6 +103,7 @@ + diff --git a/PCbuild/_testlimitedcapi.vcxproj.filters b/PCbuild/_testlimitedcapi.vcxproj.filters index b379090eb599f5..a975a508506905 100644 --- a/PCbuild/_testlimitedcapi.vcxproj.filters +++ b/PCbuild/_testlimitedcapi.vcxproj.filters @@ -18,6 +18,7 @@ + From 33b8507a941caff39ce329a33fdea0c019c8c9ca Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 16 Jan 2025 15:00:55 +0100 Subject: [PATCH 04/10] Add tests Avoid the datetime module. --- Lib/test/test_capi/test_import.py | 157 ++++++++++++++++++++++++++---- Modules/_testlimitedcapi/import.c | 147 +++++++++++++++++++++++++++- 2 files changed, 282 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_capi/test_import.py b/Lib/test/test_capi/test_import.py index bf38f8fbd15d31..fb3eaa47175395 100644 --- a/Lib/test/test_capi/test_import.py +++ b/Lib/test/test_capi/test_import.py @@ -1,4 +1,5 @@ import importlib.util +import os.path import sys import types import unittest @@ -6,6 +7,7 @@ from test.support.warnings_helper import check_warnings _testlimitedcapi = import_helper.import_module('_testlimitedcapi') +NULL = None class ImportTests(unittest.TestCase): @@ -36,7 +38,7 @@ def check_import_loaded_module(self, import_module): def check_import_fresh_module(self, import_module): old_modules = dict(sys.modules) try: - for name in ('colorsys', 'datetime', 'math'): + for name in ('colorsys', 'math'): with self.subTest(name=name): sys.modules.pop(name, None) module = import_module(name) @@ -57,32 +59,43 @@ def test_getmodule(self): self.assertIsNone(_testlimitedcapi.PyImport_GetModule('')) self.assertIsNone(_testlimitedcapi.PyImport_GetModule(object())) - def check_addmodule(self, add_module): + def check_addmodule(self, add_module, accept_nonstr=False): # create a new module - name = 'nonexistent' - self.assertNotIn(name, sys.modules) - try: - module = add_module(name) - self.assertIsInstance(module, types.ModuleType) - self.assertIs(module, sys.modules[name]) - finally: - sys.modules.pop(name, None) + names = ['nonexistent'] + if accept_nonstr: + # PyImport_AddModuleObject() accepts non-string names + names.append(object()) + for name in names: + with self.subTest(name=name): + self.assertNotIn(name, sys.modules) + try: + module = add_module(name) + self.assertIsInstance(module, types.ModuleType) + self.assertEqual(module.__name__, name) + self.assertIs(module, sys.modules[name]) + finally: + sys.modules.pop(name, None) # get an existing module self.check_import_loaded_module(add_module) def test_addmoduleobject(self): # Test PyImport_AddModuleObject() - self.check_addmodule(_testlimitedcapi.PyImport_AddModuleObject) + self.check_addmodule(_testlimitedcapi.PyImport_AddModuleObject, + accept_nonstr=True) def test_addmodule(self): # Test PyImport_AddModule() self.check_addmodule(_testlimitedcapi.PyImport_AddModule) + # CRASHES PyImport_AddModule(NULL) + def test_addmoduleref(self): # Test PyImport_AddModuleRef() self.check_addmodule(_testlimitedcapi.PyImport_AddModuleRef) + # CRASHES PyImport_AddModuleRef(NULL) + def check_import_func(self, import_module): self.check_import_loaded_module(import_module) self.check_import_fresh_module(import_module) @@ -106,19 +119,121 @@ def test_importmodulenoblock(self): with check_warnings(('', DeprecationWarning)): self.check_import_func(_testlimitedcapi.PyImport_ImportModuleNoBlock) - # TODO: test PyImport_ExecCodeModule() - # TODO: test PyImport_ExecCodeModuleEx() - # TODO: test PyImport_ExecCodeModuleWithPathnames() - # TODO: test PyImport_ExecCodeModuleObject() - # TODO: test PyImport_ImportModuleLevel() - # TODO: test PyImport_ImportModuleLevelObject() - # TODO: test PyImport_ImportModuleEx() + def check_frozen_import(self, import_frozen_module): + # Importing a frozen module executes its code, so starts by unloading + # the module to execute the code in a new (temporary) module. + old_zipimport = sys.modules.pop('zipimport') + try: + self.assertEqual(import_frozen_module('zipimport'), 1) + finally: + sys.modules['zipimport'] = old_zipimport + + # not a frozen module + self.assertEqual(import_frozen_module('sys'), 0) + + def test_importfrozenmodule(self): + # Test PyImport_ImportFrozenModule() + self.check_frozen_import(_testlimitedcapi.PyImport_ImportFrozenModule) + + # CRASHES PyImport_ImportFrozenModule(NULL) + + def test_importfrozenmoduleobject(self): + # Test PyImport_ImportFrozenModuleObject() + PyImport_ImportFrozenModuleObject = _testlimitedcapi.PyImport_ImportFrozenModuleObject + self.check_frozen_import(PyImport_ImportFrozenModuleObject) + + # Bad name is treated as "not found" + self.assertEqual(PyImport_ImportFrozenModuleObject(None), 0) + + def test_importmoduleex(self): + # Test PyImport_ImportModuleEx() + def import_module(name): + return _testlimitedcapi.PyImport_ImportModuleEx( + name, globals(), {}, []) + + self.check_import_func(import_module) + + def test_importmodulelevel(self): + # Test PyImport_ImportModuleLevel() + def import_module(name): + return _testlimitedcapi.PyImport_ImportModuleLevel( + name, globals(), {}, [], 0) + + self.check_import_func(import_module) + + def test_importmodulelevelobject(self): + # Test PyImport_ImportModuleLevelObject() + def import_module(name): + return _testlimitedcapi.PyImport_ImportModuleLevelObject( + name, globals(), {}, [], 0) + + self.check_import_func(import_module) + + def check_executecodemodule(self, execute_code, pathname=None): + name = 'test_import_executecode' + try: + # Create a temporary module where the code will be executed + self.assertNotIn(name, sys.modules) + module = _testlimitedcapi.PyImport_AddModuleRef(name) + self.assertNotHasAttr(module, 'attr') + + # Execute the code + if pathname is not None: + code_filename = pathname + else: + code_filename = '' + code = compile('attr = 1', code_filename, 'exec') + module2 = execute_code(name, code) + self.assertIs(module2, module) + + # Check the function side effects + self.assertEqual(module.attr, 1) + if pathname is not None: + self.assertEqual(module.__spec__.origin, pathname) + finally: + sys.modules.pop(name, None) + + def test_executecodemodule(self): + # Test PyImport_ExecCodeModule() + self.check_executecodemodule(_testlimitedcapi.PyImport_ExecCodeModule) + + def test_executecodemoduleex(self): + # Test PyImport_ExecCodeModuleEx() + pathname = os.path.abspath('pathname') + + def execute_code(name, code): + return _testlimitedcapi.PyImport_ExecCodeModuleEx(name, code, + pathname) + self.check_executecodemodule(execute_code, pathname) + + def check_executecode_pathnames(self, execute_code_func): + # Test non-NULL pathname and NULL cpathname + pathname = os.path.abspath('pathname') + + def execute_code1(name, code): + return execute_code_func(name, code, pathname, NULL) + self.check_executecodemodule(execute_code1, pathname) + + # Test NULL pathname and non-NULL cpathname + pyc_filename = importlib.util.cache_from_source(__file__) + py_filename = importlib.util.source_from_cache(pyc_filename) + + def execute_code2(name, code): + return execute_code_func(name, code, NULL, pyc_filename) + self.check_executecodemodule(execute_code2, py_filename) + + def test_executecodemodulewithpathnames(self): + # Test PyImport_ExecCodeModuleWithPathnames() + self.check_executecode_pathnames(_testlimitedcapi.PyImport_ExecCodeModuleWithPathnames) + + def test_executecodemoduleobject(self): + # Test PyImport_ExecCodeModuleObject() + self.check_executecode_pathnames(_testlimitedcapi.PyImport_ExecCodeModuleObject) + # TODO: test PyImport_GetImporter() # TODO: test PyImport_ReloadModule() - # TODO: test PyImport_ImportFrozenModuleObject() - # TODO: test PyImport_ImportFrozenModule() - # TODO: test PyImport_AppendInittab() # TODO: test PyImport_ExtendInittab() + # PyImport_AppendInittab() is tested by test_embed if __name__ == "__main__": diff --git a/Modules/_testlimitedcapi/import.c b/Modules/_testlimitedcapi/import.c index 01c2574d17bbfa..fc2b581115e11f 100644 --- a/Modules/_testlimitedcapi/import.c +++ b/Modules/_testlimitedcapi/import.c @@ -1,4 +1,4 @@ -// Need limited C API version 3.7 for PyImport_GetModule() +// Need limited C API version 3.13 for PyImport_AddModuleRef() #include "pyconfig.h" // Py_GIL_DISABLED #if !defined(Py_GIL_DISABLED) && !defined(Py_LIMITED_API) # define Py_LIMITED_API 0x030d0000 @@ -118,6 +118,142 @@ pyimport_importmodulenoblock(PyObject *Py_UNUSED(module), PyObject *args) } +/* Test PyImport_ImportModuleEx() */ +static PyObject * +pyimport_importmoduleex(PyObject *Py_UNUSED(module), PyObject *args) +{ + const char *name; + PyObject *globals, *locals, *fromlist; + if (!PyArg_ParseTuple(args, "sOOO", + &name, &globals, &locals, &fromlist)) { + return NULL; + } + + return PyImport_ImportModuleEx(name, globals, locals, fromlist); +} + + +/* Test PyImport_ImportModuleLevel() */ +static PyObject * +pyimport_importmodulelevel(PyObject *Py_UNUSED(module), PyObject *args) +{ + const char *name; + PyObject *globals, *locals, *fromlist; + int level; + if (!PyArg_ParseTuple(args, "sOOOi", + &name, &globals, &locals, &fromlist, &level)) { + return NULL; + } + + return PyImport_ImportModuleLevel(name, globals, locals, fromlist, level); +} + + +/* Test PyImport_ImportModuleLevelObject() */ +static PyObject * +pyimport_importmodulelevelobject(PyObject *Py_UNUSED(module), PyObject *args) +{ + PyObject *name, *globals, *locals, *fromlist; + int level; + if (!PyArg_ParseTuple(args, "OOOOi", + &name, &globals, &locals, &fromlist, &level)) { + return NULL; + } + + return PyImport_ImportModuleLevelObject(name, globals, locals, fromlist, level); +} + + +/* Test PyImport_ImportFrozenModule() */ +static PyObject * +pyimport_importfrozenmodule(PyObject *Py_UNUSED(module), PyObject *args) +{ + const char *name; + if (!PyArg_ParseTuple(args, "s", &name)) { + return NULL; + } + + int res = PyImport_ImportFrozenModule(name); + if (res < 0) { + return NULL; + } + return PyLong_FromLong(res); +} + + +/* Test PyImport_ImportFrozenModuleObject() */ +static PyObject * +pyimport_importfrozenmoduleobject(PyObject *Py_UNUSED(module), PyObject *name) +{ + int res = PyImport_ImportFrozenModuleObject(name); + if (res < 0) { + return NULL; + } + return PyLong_FromLong(res); +} + + +/* Test PyImport_ExecCodeModule() */ +static PyObject * +pyimport_executecodemodule(PyObject *Py_UNUSED(module), PyObject *args) +{ + const char *name; + PyObject *code; + if (!PyArg_ParseTuple(args, "sO", &name, &code)) { + return NULL; + } + + return PyImport_ExecCodeModule(name, code); +} + + +/* Test PyImport_ExecCodeModuleEx() */ +static PyObject * +pyimport_executecodemoduleex(PyObject *Py_UNUSED(module), PyObject *args) +{ + const char *name; + PyObject *code; + const char *pathname; + if (!PyArg_ParseTuple(args, "sOs", &name, &code, &pathname)) { + return NULL; + } + + return PyImport_ExecCodeModuleEx(name, code, pathname); +} + + +/* Test PyImport_ExecCodeModuleWithPathnames() */ +static PyObject * +pyimport_executecodemodulewithpathnames(PyObject *Py_UNUSED(module), PyObject *args) +{ + const char *name; + PyObject *code; + const char *pathname; + const char *cpathname; + if (!PyArg_ParseTuple(args, "sOzz", &name, &code, &pathname, &cpathname)) { + return NULL; + } + + return PyImport_ExecCodeModuleWithPathnames(name, code, + pathname, cpathname); +} + + +/* Test PyImport_ExecCodeModuleObject() */ +static PyObject * +pyimport_executecodemoduleobject(PyObject *Py_UNUSED(module), PyObject *args) +{ + PyObject *name, *code, *pathname, *cpathname; + if (!PyArg_ParseTuple(args, "OOOO", &name, &code, &pathname, &cpathname)) { + return NULL; + } + NULLABLE(pathname); + NULLABLE(cpathname); + + return PyImport_ExecCodeModuleObject(name, code, pathname, cpathname); +} + + static PyMethodDef test_methods[] = { {"PyImport_GetMagicNumber", pyimport_getmagicnumber, METH_NOARGS}, {"PyImport_GetMagicTag", pyimport_getmagictag, METH_NOARGS}, @@ -129,6 +265,15 @@ static PyMethodDef test_methods[] = { {"PyImport_Import", pyimport_import, METH_O}, {"PyImport_ImportModule", pyimport_importmodule, METH_VARARGS}, {"PyImport_ImportModuleNoBlock", pyimport_importmodulenoblock, METH_VARARGS}, + {"PyImport_ImportModuleEx", pyimport_importmoduleex, METH_VARARGS}, + {"PyImport_ImportModuleLevel", pyimport_importmodulelevel, METH_VARARGS}, + {"PyImport_ImportModuleLevelObject", pyimport_importmodulelevelobject, METH_VARARGS}, + {"PyImport_ImportFrozenModule", pyimport_importfrozenmodule, METH_VARARGS}, + {"PyImport_ImportFrozenModuleObject", pyimport_importfrozenmoduleobject, METH_O}, + {"PyImport_ExecCodeModule", pyimport_executecodemodule, METH_VARARGS}, + {"PyImport_ExecCodeModuleEx", pyimport_executecodemoduleex, METH_VARARGS}, + {"PyImport_ExecCodeModuleWithPathnames", pyimport_executecodemodulewithpathnames, METH_VARARGS}, + {"PyImport_ExecCodeModuleObject", pyimport_executecodemoduleobject, METH_VARARGS}, {NULL}, }; From 6a7d1defcba43412066bfd31421c3f7d66be0b7d Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 17 Jan 2025 11:06:16 +0100 Subject: [PATCH 05/10] Fix typo --- Lib/test/test_capi/test_import.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_capi/test_import.py b/Lib/test/test_capi/test_import.py index fb3eaa47175395..4512c59f014b75 100644 --- a/Lib/test/test_capi/test_import.py +++ b/Lib/test/test_capi/test_import.py @@ -120,7 +120,7 @@ def test_importmodulenoblock(self): self.check_import_func(_testlimitedcapi.PyImport_ImportModuleNoBlock) def check_frozen_import(self, import_frozen_module): - # Importing a frozen module executes its code, so starts by unloading + # Importing a frozen module executes its code, so start by unloading # the module to execute the code in a new (temporary) module. old_zipimport = sys.modules.pop('zipimport') try: From 08e48103fb73434457480d29bb54531e3f3adc0f Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 17 Jan 2025 11:37:37 +0100 Subject: [PATCH 06/10] Address Serhiy's review --- Lib/test/test_capi/test_import.py | 7 ++++--- Modules/_testlimitedcapi/import.c | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_capi/test_import.py b/Lib/test/test_capi/test_import.py index 4512c59f014b75..c38ea383873208 100644 --- a/Lib/test/test_capi/test_import.py +++ b/Lib/test/test_capi/test_import.py @@ -55,9 +55,10 @@ def test_getmodule(self): nonexistent = 'nonexistent' self.assertNotIn(nonexistent, sys.modules) - self.assertIsNone(_testlimitedcapi.PyImport_GetModule(nonexistent)) - self.assertIsNone(_testlimitedcapi.PyImport_GetModule('')) - self.assertIsNone(_testlimitedcapi.PyImport_GetModule(object())) + for name in (nonexistent, '', object()): + with self.subTest(name=name): + with self.assertRaises(KeyError): + _testlimitedcapi.PyImport_GetModule(name) def check_addmodule(self, add_module, accept_nonstr=False): # create a new module diff --git a/Modules/_testlimitedcapi/import.c b/Modules/_testlimitedcapi/import.c index fc2b581115e11f..1d1dc0f44cdc26 100644 --- a/Modules/_testlimitedcapi/import.c +++ b/Modules/_testlimitedcapi/import.c @@ -41,7 +41,8 @@ pyimport_getmodule(PyObject *Py_UNUSED(module), PyObject *name) assert(!PyErr_Occurred()); PyObject *module = PyImport_GetModule(name); if (module == NULL && !PyErr_Occurred()) { - Py_RETURN_NONE; + PyErr_SetString(PyExc_KeyError, "PyImport_GetModule() returned NULL"); + return NULL; } return module; } From 1599ab643e34f35f588caf9571d1647ee69af27b Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 17 Jan 2025 11:59:26 +0100 Subject: [PATCH 07/10] Document crashes when passing NULL --- Lib/test/test_capi/test_import.py | 24 +++++++++++++++++++++--- Modules/_testlimitedcapi/import.c | 28 +++++++++++++++++++--------- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_capi/test_import.py b/Lib/test/test_capi/test_import.py index c38ea383873208..bd56416916eb7f 100644 --- a/Lib/test/test_capi/test_import.py +++ b/Lib/test/test_capi/test_import.py @@ -60,6 +60,8 @@ def test_getmodule(self): with self.assertRaises(KeyError): _testlimitedcapi.PyImport_GetModule(name) + # CRASHES PyImport_GetModule(NULL) + def check_addmodule(self, add_module, accept_nonstr=False): # create a new module names = ['nonexistent'] @@ -85,6 +87,8 @@ def test_addmoduleobject(self): self.check_addmodule(_testlimitedcapi.PyImport_AddModuleObject, accept_nonstr=True) + # CRASHES PyImport_AddModuleObject(NULL) + def test_addmodule(self): # Test PyImport_AddModule() self.check_addmodule(_testlimitedcapi.PyImport_AddModule) @@ -111,15 +115,22 @@ def test_import(self): # Test PyImport_Import() self.check_import_func(_testlimitedcapi.PyImport_Import) + with self.assertRaises(SystemError): + _testlimitedcapi.PyImport_Import(NULL) + def test_importmodule(self): # Test PyImport_ImportModule() self.check_import_func(_testlimitedcapi.PyImport_ImportModule) + # CRASHES PyImport_ImportModule(NULL) + def test_importmodulenoblock(self): # Test deprecated PyImport_ImportModuleNoBlock() with check_warnings(('', DeprecationWarning)): self.check_import_func(_testlimitedcapi.PyImport_ImportModuleNoBlock) + # CRASHES PyImport_ImportModuleNoBlock(NULL) + def check_frozen_import(self, import_frozen_module): # Importing a frozen module executes its code, so start by unloading # the module to execute the code in a new (temporary) module. @@ -200,12 +211,19 @@ def test_executecodemodule(self): def test_executecodemoduleex(self): # Test PyImport_ExecCodeModuleEx() - pathname = os.path.abspath('pathname') - def execute_code(name, code): + # Test NULL path (it should not crash) + def execute_code1(name, code): + return _testlimitedcapi.PyImport_ExecCodeModuleEx(name, code, + NULL) + self.check_executecodemodule(execute_code1) + + # Test non-NULL path + pathname = os.path.abspath('pathname') + def execute_code2(name, code): return _testlimitedcapi.PyImport_ExecCodeModuleEx(name, code, pathname) - self.check_executecodemodule(execute_code, pathname) + self.check_executecodemodule(execute_code2, pathname) def check_executecode_pathnames(self, execute_code_func): # Test non-NULL pathname and NULL cpathname diff --git a/Modules/_testlimitedcapi/import.c b/Modules/_testlimitedcapi/import.c index 1d1dc0f44cdc26..c4bdea8fbc5cdc 100644 --- a/Modules/_testlimitedcapi/import.c +++ b/Modules/_testlimitedcapi/import.c @@ -39,6 +39,7 @@ static PyObject * pyimport_getmodule(PyObject *Py_UNUSED(module), PyObject *name) { assert(!PyErr_Occurred()); + NULLABLE(name); PyObject *module = PyImport_GetModule(name); if (module == NULL && !PyErr_Occurred()) { PyErr_SetString(PyExc_KeyError, "PyImport_GetModule() returned NULL"); @@ -52,6 +53,7 @@ pyimport_getmodule(PyObject *Py_UNUSED(module), PyObject *name) static PyObject * pyimport_addmoduleobject(PyObject *Py_UNUSED(module), PyObject *name) { + NULLABLE(name); return Py_XNewRef(PyImport_AddModuleObject(name)); } @@ -61,7 +63,7 @@ static PyObject * pyimport_addmodule(PyObject *Py_UNUSED(module), PyObject *args) { const char *name; - if (!PyArg_ParseTuple(args, "s", &name)) { + if (!PyArg_ParseTuple(args, "z", &name)) { return NULL; } @@ -86,6 +88,7 @@ pyimport_addmoduleref(PyObject *Py_UNUSED(module), PyObject *args) static PyObject * pyimport_import(PyObject *Py_UNUSED(module), PyObject *name) { + NULLABLE(name); return PyImport_Import(name); } @@ -95,7 +98,7 @@ static PyObject * pyimport_importmodule(PyObject *Py_UNUSED(module), PyObject *args) { const char *name; - if (!PyArg_ParseTuple(args, "s", &name)) { + if (!PyArg_ParseTuple(args, "z", &name)) { return NULL; } @@ -108,7 +111,7 @@ static PyObject * pyimport_importmodulenoblock(PyObject *Py_UNUSED(module), PyObject *args) { const char *name; - if (!PyArg_ParseTuple(args, "s", &name)) { + if (!PyArg_ParseTuple(args, "z", &name)) { return NULL; } @@ -129,6 +132,9 @@ pyimport_importmoduleex(PyObject *Py_UNUSED(module), PyObject *args) &name, &globals, &locals, &fromlist)) { return NULL; } + NULLABLE(globals); + NULLABLE(locals); + NULLABLE(fromlist); return PyImport_ImportModuleEx(name, globals, locals, fromlist); } @@ -145,6 +151,9 @@ pyimport_importmodulelevel(PyObject *Py_UNUSED(module), PyObject *args) &name, &globals, &locals, &fromlist, &level)) { return NULL; } + NULLABLE(globals); + NULLABLE(locals); + NULLABLE(fromlist); return PyImport_ImportModuleLevel(name, globals, locals, fromlist, level); } @@ -160,6 +169,10 @@ pyimport_importmodulelevelobject(PyObject *Py_UNUSED(module), PyObject *args) &name, &globals, &locals, &fromlist, &level)) { return NULL; } + NULLABLE(name); + NULLABLE(globals); + NULLABLE(locals); + NULLABLE(fromlist); return PyImport_ImportModuleLevelObject(name, globals, locals, fromlist, level); } @@ -175,10 +188,7 @@ pyimport_importfrozenmodule(PyObject *Py_UNUSED(module), PyObject *args) } int res = PyImport_ImportFrozenModule(name); - if (res < 0) { - return NULL; - } - return PyLong_FromLong(res); + RETURN_INT(res); } @@ -215,7 +225,7 @@ pyimport_executecodemoduleex(PyObject *Py_UNUSED(module), PyObject *args) const char *name; PyObject *code; const char *pathname; - if (!PyArg_ParseTuple(args, "sOs", &name, &code, &pathname)) { + if (!PyArg_ParseTuple(args, "zOz", &name, &code, &pathname)) { return NULL; } @@ -231,7 +241,7 @@ pyimport_executecodemodulewithpathnames(PyObject *Py_UNUSED(module), PyObject *a PyObject *code; const char *pathname; const char *cpathname; - if (!PyArg_ParseTuple(args, "sOzz", &name, &code, &pathname, &cpathname)) { + if (!PyArg_ParseTuple(args, "zOzz", &name, &code, &pathname, &cpathname)) { return NULL; } From b5b8558099611209f4feab9e85a503dfa371d6a9 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 17 Jan 2025 13:25:45 +0100 Subject: [PATCH 08/10] PyImport_GetModule() returns KeyError, instead of raising KeyError --- Lib/test/test_capi/test_import.py | 4 ++-- Modules/_testlimitedcapi/import.c | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_capi/test_import.py b/Lib/test/test_capi/test_import.py index bd56416916eb7f..cfc35c2081bf88 100644 --- a/Lib/test/test_capi/test_import.py +++ b/Lib/test/test_capi/test_import.py @@ -57,8 +57,8 @@ def test_getmodule(self): self.assertNotIn(nonexistent, sys.modules) for name in (nonexistent, '', object()): with self.subTest(name=name): - with self.assertRaises(KeyError): - _testlimitedcapi.PyImport_GetModule(name) + self.assertEqual(_testlimitedcapi.PyImport_GetModule(name), + KeyError) # CRASHES PyImport_GetModule(NULL) diff --git a/Modules/_testlimitedcapi/import.c b/Modules/_testlimitedcapi/import.c index c4bdea8fbc5cdc..70719c3b8fc70c 100644 --- a/Modules/_testlimitedcapi/import.c +++ b/Modules/_testlimitedcapi/import.c @@ -42,8 +42,7 @@ pyimport_getmodule(PyObject *Py_UNUSED(module), PyObject *name) NULLABLE(name); PyObject *module = PyImport_GetModule(name); if (module == NULL && !PyErr_Occurred()) { - PyErr_SetString(PyExc_KeyError, "PyImport_GetModule() returned NULL"); - return NULL; + return Py_NewRef(PyExc_KeyError); } return module; } From e5ad39a2180f39f1fa08cb53e765a2c6ecac53e2 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 17 Jan 2025 18:38:37 +0200 Subject: [PATCH 09/10] Test more corner cases and errors. --- Lib/test/test_capi/test_import.py | 228 ++++++++++++++++++++---------- Modules/_testlimitedcapi/import.c | 49 ++++--- 2 files changed, 180 insertions(+), 97 deletions(-) diff --git a/Lib/test/test_capi/test_import.py b/Lib/test/test_capi/test_import.py index cfc35c2081bf88..c5d3640a162876 100644 --- a/Lib/test/test_capi/test_import.py +++ b/Lib/test/test_capi/test_import.py @@ -3,6 +3,7 @@ import sys import types import unittest +from test.support import os_helper from test.support import import_helper from test.support.warnings_helper import check_warnings @@ -31,9 +32,10 @@ def check_import_loaded_module(self, import_module): for name in ('os', 'sys', 'test', 'unittest'): with self.subTest(name=name): self.assertIn(name, sys.modules) + old_module = sys.modules[name] module = import_module(name) self.assertIsInstance(module, types.ModuleType) - self.assertIs(module, sys.modules[name]) + self.assertIs(module, old_module) def check_import_fresh_module(self, import_module): old_modules = dict(sys.modules) @@ -51,23 +53,25 @@ def check_import_fresh_module(self, import_module): def test_getmodule(self): # Test PyImport_GetModule() - self.check_import_loaded_module(_testlimitedcapi.PyImport_GetModule) + getmodule = _testlimitedcapi.PyImport_GetModule + self.check_import_loaded_module(getmodule) nonexistent = 'nonexistent' self.assertNotIn(nonexistent, sys.modules) - for name in (nonexistent, '', object()): - with self.subTest(name=name): - self.assertEqual(_testlimitedcapi.PyImport_GetModule(name), - KeyError) + self.assertIs(getmodule(nonexistent), KeyError) + self.assertIs(getmodule(''), KeyError) + self.assertIs(getmodule(object()), KeyError) - # CRASHES PyImport_GetModule(NULL) + self.assertRaises(TypeError, getmodule, []) # unhashable + # CRASHES getmodule(NULL) def check_addmodule(self, add_module, accept_nonstr=False): # create a new module names = ['nonexistent'] if accept_nonstr: + names.append(b'\xff') # non-UTF-8 # PyImport_AddModuleObject() accepts non-string names - names.append(object()) + names.append(tuple(['hashable non-string'])) for name in names: with self.subTest(name=name): self.assertNotIn(name, sys.modules) @@ -84,52 +88,58 @@ def check_addmodule(self, add_module, accept_nonstr=False): def test_addmoduleobject(self): # Test PyImport_AddModuleObject() - self.check_addmodule(_testlimitedcapi.PyImport_AddModuleObject, - accept_nonstr=True) + addmoduleobject = _testlimitedcapi.PyImport_AddModuleObject + self.check_addmodule(addmoduleobject, accept_nonstr=True) - # CRASHES PyImport_AddModuleObject(NULL) + self.assertRaises(TypeError, addmoduleobject, []) # unhashable + # CRASHES addmoduleobject(NULL) def test_addmodule(self): # Test PyImport_AddModule() - self.check_addmodule(_testlimitedcapi.PyImport_AddModule) + addmodule = _testlimitedcapi.PyImport_AddModule + self.check_addmodule(addmodule) - # CRASHES PyImport_AddModule(NULL) + self.assertRaises(UnicodeDecodeError, addmodule, b'\xff') + # CRASHES addmodule(NULL) def test_addmoduleref(self): # Test PyImport_AddModuleRef() - self.check_addmodule(_testlimitedcapi.PyImport_AddModuleRef) + addmoduleref = _testlimitedcapi.PyImport_AddModuleRef + self.check_addmodule(addmoduleref) - # CRASHES PyImport_AddModuleRef(NULL) + self.assertRaises(UnicodeDecodeError, addmoduleref, b'\xff') + # CRASHES addmoduleref(NULL) def check_import_func(self, import_module): self.check_import_loaded_module(import_module) self.check_import_fresh_module(import_module) - - # Invalid module name types - with self.assertRaises(TypeError): - import_module(123) - with self.assertRaises(TypeError): - import_module(object()) + self.assertRaises(ModuleNotFoundError, import_module, 'nonexistent') + self.assertRaises(ValueError, import_module, '') def test_import(self): # Test PyImport_Import() - self.check_import_func(_testlimitedcapi.PyImport_Import) + import_ = _testlimitedcapi.PyImport_Import + self.check_import_func(import_) - with self.assertRaises(SystemError): - _testlimitedcapi.PyImport_Import(NULL) + self.assertRaises(TypeError, import_, b'os') + self.assertRaises(SystemError, import_, NULL) def test_importmodule(self): # Test PyImport_ImportModule() - self.check_import_func(_testlimitedcapi.PyImport_ImportModule) + importmodule = _testlimitedcapi.PyImport_ImportModule + self.check_import_func(importmodule) - # CRASHES PyImport_ImportModule(NULL) + self.assertRaises(UnicodeDecodeError, importmodule, b'\xff') + # CRASHES importmodule(NULL) def test_importmodulenoblock(self): # Test deprecated PyImport_ImportModuleNoBlock() + importmodulenoblock = _testlimitedcapi.PyImport_ImportModuleNoBlock with check_warnings(('', DeprecationWarning)): - self.check_import_func(_testlimitedcapi.PyImport_ImportModuleNoBlock) + self.check_import_func(importmodulenoblock) + self.assertRaises(UnicodeDecodeError, importmodulenoblock, b'\xff') - # CRASHES PyImport_ImportModuleNoBlock(NULL) + # CRASHES importmodulenoblock(NULL) def check_frozen_import(self, import_frozen_module): # Importing a frozen module executes its code, so start by unloading @@ -137,51 +147,73 @@ def check_frozen_import(self, import_frozen_module): old_zipimport = sys.modules.pop('zipimport') try: self.assertEqual(import_frozen_module('zipimport'), 1) + self.assertEqual(import_frozen_module('zipimport'), 1) finally: sys.modules['zipimport'] = old_zipimport # not a frozen module self.assertEqual(import_frozen_module('sys'), 0) + self.assertEqual(import_frozen_module('nonexistent'), 0) + self.assertEqual(import_frozen_module(''), 0) def test_importfrozenmodule(self): # Test PyImport_ImportFrozenModule() - self.check_frozen_import(_testlimitedcapi.PyImport_ImportFrozenModule) + importfrozenmodule = _testlimitedcapi.PyImport_ImportFrozenModule + self.check_frozen_import(importfrozenmodule) - # CRASHES PyImport_ImportFrozenModule(NULL) + self.assertRaises(UnicodeDecodeError, importfrozenmodule, b'\xff') + # CRASHES importfrozenmodule(NULL) def test_importfrozenmoduleobject(self): # Test PyImport_ImportFrozenModuleObject() - PyImport_ImportFrozenModuleObject = _testlimitedcapi.PyImport_ImportFrozenModuleObject - self.check_frozen_import(PyImport_ImportFrozenModuleObject) - - # Bad name is treated as "not found" - self.assertEqual(PyImport_ImportFrozenModuleObject(None), 0) + importfrozenmoduleobject = _testlimitedcapi.PyImport_ImportFrozenModuleObject + self.check_frozen_import(importfrozenmoduleobject) + self.assertEqual(importfrozenmoduleobject(b'zipimport'), 0) + self.assertEqual(importfrozenmoduleobject(NULL), 0) def test_importmoduleex(self): # Test PyImport_ImportModuleEx() - def import_module(name): - return _testlimitedcapi.PyImport_ImportModuleEx( - name, globals(), {}, []) - - self.check_import_func(import_module) + importmoduleex = _testlimitedcapi.PyImport_ImportModuleEx + self.check_import_func(lambda name: importmoduleex(name, NULL, NULL, NULL)) + + self.assertRaises(ModuleNotFoundError, importmoduleex, 'nonexistent', NULL, NULL, NULL) + self.assertRaises(ValueError, importmoduleex, '', NULL, NULL, NULL) + self.assertRaises(UnicodeDecodeError, importmoduleex, b'\xff', NULL, NULL, NULL) + # CRASHES importmoduleex(NULL, NULL, NULL, NULL) + + def check_importmodulelevel(self, importmodulelevel): + self.check_import_func(lambda name: importmodulelevel(name, NULL, NULL, NULL, 0)) + + self.assertRaises(ModuleNotFoundError, importmodulelevel, 'nonexistent', NULL, NULL, NULL, 0) + self.assertRaises(ValueError, importmodulelevel, '', NULL, NULL, NULL, 0) + + if __package__: + self.assertIs(importmodulelevel('test_import', globals(), NULL, NULL, 1), + sys.modules['test.test_capi.test_import']) + self.assertIs(importmodulelevel('test_capi', globals(), NULL, NULL, 2), + sys.modules['test.test_capi']) + self.assertRaises(ValueError, importmodulelevel, 'os', NULL, NULL, NULL, -1) + with self.assertWarns(ImportWarning): + self.assertRaises(KeyError, importmodulelevel, 'test_import', {}, NULL, NULL, 1) + self.assertRaises(TypeError, importmodulelevel, 'test_import', [], NULL, NULL, 1) def test_importmodulelevel(self): # Test PyImport_ImportModuleLevel() - def import_module(name): - return _testlimitedcapi.PyImport_ImportModuleLevel( - name, globals(), {}, [], 0) + importmodulelevel = _testlimitedcapi.PyImport_ImportModuleLevel + self.check_importmodulelevel(importmodulelevel) - self.check_import_func(import_module) + self.assertRaises(UnicodeDecodeError, importmodulelevel, b'\xff', NULL, NULL, NULL, 0) + # CRASHES importmodulelevel(NULL, NULL, NULL, NULL, 0) def test_importmodulelevelobject(self): # Test PyImport_ImportModuleLevelObject() - def import_module(name): - return _testlimitedcapi.PyImport_ImportModuleLevelObject( - name, globals(), {}, [], 0) + importmodulelevel = _testlimitedcapi.PyImport_ImportModuleLevelObject + self.check_importmodulelevel(importmodulelevel) - self.check_import_func(import_module) + self.assertRaises(TypeError, importmodulelevel, b'os', NULL, NULL, NULL, 0) + self.assertRaises(ValueError, importmodulelevel, NULL, NULL, NULL, NULL, 0) - def check_executecodemodule(self, execute_code, pathname=None): + def check_executecodemodule(self, execute_code, *args): name = 'test_import_executecode' try: # Create a temporary module where the code will be executed @@ -190,64 +222,104 @@ def check_executecodemodule(self, execute_code, pathname=None): self.assertNotHasAttr(module, 'attr') # Execute the code - if pathname is not None: - code_filename = pathname - else: - code_filename = '' - code = compile('attr = 1', code_filename, 'exec') - module2 = execute_code(name, code) + code = compile('attr = 1', '', 'exec') + module2 = execute_code(name, code, *args) self.assertIs(module2, module) # Check the function side effects self.assertEqual(module.attr, 1) - if pathname is not None: - self.assertEqual(module.__spec__.origin, pathname) finally: sys.modules.pop(name, None) + #print() + #print(module.__spec__) + #from pprint import pp + #pp(vars(module)) + #print(vars(module)) + return module.__spec__.origin def test_executecodemodule(self): # Test PyImport_ExecCodeModule() - self.check_executecodemodule(_testlimitedcapi.PyImport_ExecCodeModule) + execcodemodule = _testlimitedcapi.PyImport_ExecCodeModule + self.check_executecodemodule(execcodemodule) + + code = compile('attr = 1', '', 'exec') + self.assertRaises(UnicodeDecodeError, execcodemodule, b'\xff', code) + # CRASHES execcodemodule(NULL, code) + # CRASHES execcodemodule(name, NULL) def test_executecodemoduleex(self): # Test PyImport_ExecCodeModuleEx() + execcodemoduleex = _testlimitedcapi.PyImport_ExecCodeModuleEx # Test NULL path (it should not crash) - def execute_code1(name, code): - return _testlimitedcapi.PyImport_ExecCodeModuleEx(name, code, - NULL) - self.check_executecodemodule(execute_code1) + self.check_executecodemodule(execcodemoduleex, NULL) # Test non-NULL path - pathname = os.path.abspath('pathname') - def execute_code2(name, code): - return _testlimitedcapi.PyImport_ExecCodeModuleEx(name, code, - pathname) - self.check_executecodemodule(execute_code2, pathname) + pathname = b'pathname' + origin = self.check_executecodemodule(execcodemoduleex, pathname) + self.assertEqual(origin, os.path.abspath(os.fsdecode(pathname))) - def check_executecode_pathnames(self, execute_code_func): + pathname = os_helper.TESTFN_UNDECODABLE + if pathname: + origin = self.check_executecodemodule(execcodemoduleex, pathname) + self.assertEqual(origin, os.path.abspath(os.fsdecode(pathname))) + + code = compile('attr = 1', '', 'exec') + self.assertRaises(UnicodeDecodeError, execcodemoduleex, b'\xff', code, NULL) + # CRASHES execcodemoduleex(NULL, code, NULL) + # CRASHES execcodemoduleex(name, NULL, NULL) + + def check_executecode_pathnames(self, execute_code_func, object=False): # Test non-NULL pathname and NULL cpathname - pathname = os.path.abspath('pathname') - def execute_code1(name, code): - return execute_code_func(name, code, pathname, NULL) - self.check_executecodemodule(execute_code1, pathname) + # Test NULL paths (it should not crash) + self.check_executecodemodule(execute_code_func, NULL, NULL) + + pathname = 'pathname' + origin = self.check_executecodemodule(execute_code_func, pathname, NULL) + self.assertEqual(origin, os.path.abspath(os.fsdecode(pathname))) + origin = self.check_executecodemodule(execute_code_func, NULL, pathname) + if not object: + self.assertEqual(origin, os.path.abspath(os.fsdecode(pathname))) + + pathname = os_helper.TESTFN_UNDECODABLE + if pathname: + if object: + pathname = os.fsdecode(pathname) + origin = self.check_executecodemodule(execute_code_func, pathname, NULL) + self.assertEqual(origin, os.path.abspath(os.fsdecode(pathname))) + self.check_executecodemodule(execute_code_func, NULL, pathname) # Test NULL pathname and non-NULL cpathname pyc_filename = importlib.util.cache_from_source(__file__) py_filename = importlib.util.source_from_cache(pyc_filename) - - def execute_code2(name, code): - return execute_code_func(name, code, NULL, pyc_filename) - self.check_executecodemodule(execute_code2, py_filename) + origin = self.check_executecodemodule(execute_code_func, NULL, pyc_filename) + if not object: + self.assertEqual(origin, py_filename) def test_executecodemodulewithpathnames(self): # Test PyImport_ExecCodeModuleWithPathnames() - self.check_executecode_pathnames(_testlimitedcapi.PyImport_ExecCodeModuleWithPathnames) + execute_code_func = _testlimitedcapi.PyImport_ExecCodeModuleWithPathnames + self.check_executecode_pathnames(execute_code_func) + + code = compile('attr = 1', '', 'exec') + self.assertRaises(UnicodeDecodeError, execute_code_func, b'\xff', code, NULL, NULL) + # CRASHES execute_code_func(NULL, code, NULL, NULL) + # CRASHES execute_code_func(name, NULL, NULL, NULL) def test_executecodemoduleobject(self): # Test PyImport_ExecCodeModuleObject() - self.check_executecode_pathnames(_testlimitedcapi.PyImport_ExecCodeModuleObject) + execute_code_func = _testlimitedcapi.PyImport_ExecCodeModuleObject + self.check_executecode_pathnames(execute_code_func, object=True) + + #self.check_executecodemodule(execute_code_func, [], NULL) + code = compile('attr = 1', '', 'exec') + self.assertRaises(TypeError, execute_code_func, [], code, NULL, NULL) + nonstring = tuple(['hashable non-string']) + self.assertRaises(AttributeError, execute_code_func, nonstring, code, NULL, NULL) + sys.modules.pop(nonstring, None) + # CRASHES execute_code_func(NULL, code, NULL, NULL) + # CRASHES execute_code_func(name, NULL, NULL, NULL) # TODO: test PyImport_GetImporter() # TODO: test PyImport_ReloadModule() diff --git a/Modules/_testlimitedcapi/import.c b/Modules/_testlimitedcapi/import.c index 70719c3b8fc70c..3707dbedeea0d9 100644 --- a/Modules/_testlimitedcapi/import.c +++ b/Modules/_testlimitedcapi/import.c @@ -62,7 +62,8 @@ static PyObject * pyimport_addmodule(PyObject *Py_UNUSED(module), PyObject *args) { const char *name; - if (!PyArg_ParseTuple(args, "z", &name)) { + Py_ssize_t size; + if (!PyArg_ParseTuple(args, "z#", &name, &size)) { return NULL; } @@ -75,7 +76,8 @@ static PyObject * pyimport_addmoduleref(PyObject *Py_UNUSED(module), PyObject *args) { const char *name; - if (!PyArg_ParseTuple(args, "s", &name)) { + Py_ssize_t size; + if (!PyArg_ParseTuple(args, "z#", &name, &size)) { return NULL; } @@ -97,7 +99,8 @@ static PyObject * pyimport_importmodule(PyObject *Py_UNUSED(module), PyObject *args) { const char *name; - if (!PyArg_ParseTuple(args, "z", &name)) { + Py_ssize_t size; + if (!PyArg_ParseTuple(args, "z#", &name, &size)) { return NULL; } @@ -110,7 +113,8 @@ static PyObject * pyimport_importmodulenoblock(PyObject *Py_UNUSED(module), PyObject *args) { const char *name; - if (!PyArg_ParseTuple(args, "z", &name)) { + Py_ssize_t size; + if (!PyArg_ParseTuple(args, "z#", &name, &size)) { return NULL; } @@ -126,9 +130,10 @@ static PyObject * pyimport_importmoduleex(PyObject *Py_UNUSED(module), PyObject *args) { const char *name; + Py_ssize_t size; PyObject *globals, *locals, *fromlist; - if (!PyArg_ParseTuple(args, "sOOO", - &name, &globals, &locals, &fromlist)) { + if (!PyArg_ParseTuple(args, "z#OOO", + &name, &size, &globals, &locals, &fromlist)) { return NULL; } NULLABLE(globals); @@ -144,10 +149,11 @@ static PyObject * pyimport_importmodulelevel(PyObject *Py_UNUSED(module), PyObject *args) { const char *name; + Py_ssize_t size; PyObject *globals, *locals, *fromlist; int level; - if (!PyArg_ParseTuple(args, "sOOOi", - &name, &globals, &locals, &fromlist, &level)) { + if (!PyArg_ParseTuple(args, "z#OOOi", + &name, &size, &globals, &locals, &fromlist, &level)) { return NULL; } NULLABLE(globals); @@ -182,12 +188,12 @@ static PyObject * pyimport_importfrozenmodule(PyObject *Py_UNUSED(module), PyObject *args) { const char *name; - if (!PyArg_ParseTuple(args, "s", &name)) { + Py_ssize_t size; + if (!PyArg_ParseTuple(args, "z#", &name, &size)) { return NULL; } - int res = PyImport_ImportFrozenModule(name); - RETURN_INT(res); + RETURN_INT(PyImport_ImportFrozenModule(name)); } @@ -195,11 +201,8 @@ pyimport_importfrozenmodule(PyObject *Py_UNUSED(module), PyObject *args) static PyObject * pyimport_importfrozenmoduleobject(PyObject *Py_UNUSED(module), PyObject *name) { - int res = PyImport_ImportFrozenModuleObject(name); - if (res < 0) { - return NULL; - } - return PyLong_FromLong(res); + NULLABLE(name); + RETURN_INT(PyImport_ImportFrozenModuleObject(name)); } @@ -208,10 +211,12 @@ static PyObject * pyimport_executecodemodule(PyObject *Py_UNUSED(module), PyObject *args) { const char *name; + Py_ssize_t size; PyObject *code; - if (!PyArg_ParseTuple(args, "sO", &name, &code)) { + if (!PyArg_ParseTuple(args, "z#O", &name, &size, &code)) { return NULL; } + NULLABLE(code); return PyImport_ExecCodeModule(name, code); } @@ -222,11 +227,13 @@ static PyObject * pyimport_executecodemoduleex(PyObject *Py_UNUSED(module), PyObject *args) { const char *name; + Py_ssize_t size; PyObject *code; const char *pathname; - if (!PyArg_ParseTuple(args, "zOz", &name, &code, &pathname)) { + if (!PyArg_ParseTuple(args, "z#Oz#", &name, &size, &code, &pathname, &size)) { return NULL; } + NULLABLE(code); return PyImport_ExecCodeModuleEx(name, code, pathname); } @@ -237,12 +244,14 @@ static PyObject * pyimport_executecodemodulewithpathnames(PyObject *Py_UNUSED(module), PyObject *args) { const char *name; + Py_ssize_t size; PyObject *code; const char *pathname; const char *cpathname; - if (!PyArg_ParseTuple(args, "zOzz", &name, &code, &pathname, &cpathname)) { + if (!PyArg_ParseTuple(args, "z#Oz#z#", &name, &size, &code, &pathname, &size, &cpathname, &size)) { return NULL; } + NULLABLE(code); return PyImport_ExecCodeModuleWithPathnames(name, code, pathname, cpathname); @@ -257,6 +266,8 @@ pyimport_executecodemoduleobject(PyObject *Py_UNUSED(module), PyObject *args) if (!PyArg_ParseTuple(args, "OOOO", &name, &code, &pathname, &cpathname)) { return NULL; } + NULLABLE(name); + NULLABLE(code); NULLABLE(pathname); NULLABLE(cpathname); From e4c7ff75987744f64f570fc791338f77c689f16e Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 17 Jan 2025 18:28:18 +0100 Subject: [PATCH 10/10] Cleanup --- Lib/test/test_capi/test_import.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_capi/test_import.py b/Lib/test/test_capi/test_import.py index c5d3640a162876..94f96728d9174b 100644 --- a/Lib/test/test_capi/test_import.py +++ b/Lib/test/test_capi/test_import.py @@ -147,6 +147,8 @@ def check_frozen_import(self, import_frozen_module): old_zipimport = sys.modules.pop('zipimport') try: self.assertEqual(import_frozen_module('zipimport'), 1) + + # import zipimport again self.assertEqual(import_frozen_module('zipimport'), 1) finally: sys.modules['zipimport'] = old_zipimport @@ -230,11 +232,6 @@ def check_executecodemodule(self, execute_code, *args): self.assertEqual(module.attr, 1) finally: sys.modules.pop(name, None) - #print() - #print(module.__spec__) - #from pprint import pp - #pp(vars(module)) - #print(vars(module)) return module.__spec__.origin def test_executecodemodule(self): @@ -312,7 +309,6 @@ def test_executecodemoduleobject(self): execute_code_func = _testlimitedcapi.PyImport_ExecCodeModuleObject self.check_executecode_pathnames(execute_code_func, object=True) - #self.check_executecodemodule(execute_code_func, [], NULL) code = compile('attr = 1', '', 'exec') self.assertRaises(TypeError, execute_code_func, [], code, NULL, NULL) nonstring = tuple(['hashable non-string'])