From aa340b85e27580cb29fb8498078a966ae2cead75 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Thu, 15 Dec 2022 09:40:46 -0500 Subject: [PATCH 1/7] bpo-34816: Raise AttributeError if loading fails in ctypes.LibraryLoader.__get_attr__ --- Lib/ctypes/__init__.py | 5 +- Lib/ctypes/test/test_loading.py | 186 ++++++++++++++++++ .../2021-04-08-00-36-37.bpo-34816.4Xe0id.rst | 1 + 3 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 Lib/ctypes/test/test_loading.py create mode 100644 Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst diff --git a/Lib/ctypes/__init__.py b/Lib/ctypes/__init__.py index b94b337a3157b7..2e9d4c5e7238e9 100644 --- a/Lib/ctypes/__init__.py +++ b/Lib/ctypes/__init__.py @@ -444,7 +444,10 @@ def __init__(self, dlltype): def __getattr__(self, name): if name[0] == '_': raise AttributeError(name) - dll = self._dlltype(name) + try: + dll = self._dlltype(name) + except OSError: + raise AttributeError(name) setattr(self, name, dll) return dll diff --git a/Lib/ctypes/test/test_loading.py b/Lib/ctypes/test/test_loading.py new file mode 100644 index 00000000000000..e7ebfa55d2e7bb --- /dev/null +++ b/Lib/ctypes/test/test_loading.py @@ -0,0 +1,186 @@ +from ctypes import * +import os +import shutil +import subprocess +import sys +import unittest +import test.support +from ctypes.util import find_library + +libc_name = None + +def setUpModule(): + global libc_name + if os.name == "nt": + libc_name = find_library("c") + elif sys.platform == "cygwin": + libc_name = "cygwin1.dll" + else: + libc_name = find_library("c") + + if test.support.verbose: + print("libc_name is", libc_name) + +class LoaderTest(unittest.TestCase): + + unknowndll = "xxrandomnamexx" + + def test_load(self): + if libc_name is None: + self.skipTest('could not find libc') + CDLL(libc_name) + CDLL(os.path.basename(libc_name)) + self.assertRaises(OSError, CDLL, self.unknowndll) + + def test_load_version(self): + if libc_name is None: + self.skipTest('could not find libc') + if os.path.basename(libc_name) != 'libc.so.6': + self.skipTest('wrong libc path for test') + cdll.LoadLibrary("libc.so.6") + # linux uses version, libc 9 should not exist + self.assertRaises(OSError, cdll.LoadLibrary, "libc.so.9") + self.assertRaises(OSError, cdll.LoadLibrary, self.unknowndll) + + def test_find(self): + for name in ("c", "m"): + lib = find_library(name) + if lib: + cdll.LoadLibrary(lib) + CDLL(lib) + + @unittest.skipUnless(os.name == "nt", + 'test specific to Windows') + def test_load_library(self): + # CRT is no longer directly loadable. See issue23606 for the + # discussion about alternative approaches. + #self.assertIsNotNone(libc_name) + if test.support.verbose: + print(find_library("kernel32")) + print(find_library("user32")) + + if os.name == "nt": + windll.kernel32.GetModuleHandleW + windll["kernel32"].GetModuleHandleW + windll.LoadLibrary("kernel32").GetModuleHandleW + WinDLL("kernel32").GetModuleHandleW + # embedded null character + self.assertRaises(ValueError, windll.LoadLibrary, "kernel32\0") + + @unittest.skipUnless(os.name == "nt", + 'test specific to Windows') + def test_load_ordinal_functions(self): + import _ctypes_test + dll = WinDLL(_ctypes_test.__file__) + # We load the same function both via ordinal and name + func_ord = dll[2] + func_name = dll.GetString + # addressof gets the address where the function pointer is stored + a_ord = addressof(func_ord) + a_name = addressof(func_name) + f_ord_addr = c_void_p.from_address(a_ord).value + f_name_addr = c_void_p.from_address(a_name).value + self.assertEqual(hex(f_ord_addr), hex(f_name_addr)) + + self.assertRaises(AttributeError, dll.__getitem__, 1234) + + @unittest.skipUnless(os.name == "nt", 'Windows-specific test') + def test_1703286_A(self): + from _ctypes import LoadLibrary, FreeLibrary + # On winXP 64-bit, advapi32 loads at an address that does + # NOT fit into a 32-bit integer. FreeLibrary must be able + # to accept this address. + + # These are tests for http://www.python.org/sf/1703286 + handle = LoadLibrary("advapi32") + FreeLibrary(handle) + + @unittest.skipUnless(os.name == "nt", 'Windows-specific test') + def test_1703286_B(self): + # Since on winXP 64-bit advapi32 loads like described + # above, the (arbitrarily selected) CloseEventLog function + # also has a high address. 'call_function' should accept + # addresses so large. + from _ctypes import call_function + advapi32 = windll.advapi32 + # Calling CloseEventLog with a NULL argument should fail, + # but the call should not segfault or so. + self.assertEqual(0, advapi32.CloseEventLog(None)) + windll.kernel32.GetProcAddress.argtypes = c_void_p, c_char_p + windll.kernel32.GetProcAddress.restype = c_void_p + proc = windll.kernel32.GetProcAddress(advapi32._handle, + b"CloseEventLog") + self.assertTrue(proc) + # This is the real test: call the function via 'call_function' + self.assertEqual(0, call_function(proc, (None,))) + + @unittest.skipUnless(os.name == "nt", + 'test specific to Windows') + def test_load_hasattr(self): + # bpo-34816: shouldn't raise OSError + self.assertFalse(hasattr(windll, 'test')) + + @unittest.skipUnless(os.name == "nt", + 'test specific to Windows') + def test_load_dll_with_flags(self): + _sqlite3 = test.support.import_module("_sqlite3") + src = _sqlite3.__file__ + if src.lower().endswith("_d.pyd"): + ext = "_d.dll" + else: + ext = ".dll" + + with test.support.temp_dir() as tmp: + # We copy two files and load _sqlite3.dll (formerly .pyd), + # which has a dependency on sqlite3.dll. Then we test + # loading it in subprocesses to avoid it starting in memory + # for each test. + target = os.path.join(tmp, "_sqlite3.dll") + shutil.copy(src, target) + shutil.copy(os.path.join(os.path.dirname(src), "sqlite3" + ext), + os.path.join(tmp, "sqlite3" + ext)) + + def should_pass(command): + with self.subTest(command): + subprocess.check_output( + [sys.executable, "-c", + "from ctypes import *; import nt;" + command], + cwd=tmp + ) + + def should_fail(command): + with self.subTest(command): + with self.assertRaises(subprocess.CalledProcessError): + subprocess.check_output( + [sys.executable, "-c", + "from ctypes import *; import nt;" + command], + cwd=tmp, stderr=subprocess.STDOUT, + ) + + # Default load should not find this in CWD + should_fail("WinDLL('_sqlite3.dll')") + + # Relative path (but not just filename) should succeed + should_pass("WinDLL('./_sqlite3.dll')") + + # Insecure load flags should succeed + # Clear the DLL directory to avoid safe search settings propagating + should_pass("windll.kernel32.SetDllDirectoryW(None); WinDLL('_sqlite3.dll', winmode=0)") + + # Full path load without DLL_LOAD_DIR shouldn't find dependency + should_fail("WinDLL(nt._getfullpathname('_sqlite3.dll'), " + + "winmode=nt._LOAD_LIBRARY_SEARCH_SYSTEM32)") + + # Full path load with DLL_LOAD_DIR should succeed + should_pass("WinDLL(nt._getfullpathname('_sqlite3.dll'), " + + "winmode=nt._LOAD_LIBRARY_SEARCH_SYSTEM32|" + + "nt._LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR)") + + # User-specified directory should succeed + should_pass("import os; p = os.add_dll_directory(os.getcwd());" + + "WinDLL('_sqlite3.dll'); p.close()") + + + +if __name__ == "__main__": + unittest.main() diff --git a/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst b/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst new file mode 100644 index 00000000000000..3b901a5f00da31 --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst @@ -0,0 +1 @@ +Fix issue where hasattr(ctypes.windll, 'somefunction') raises OSError instead of the expected behavior: returning False \ No newline at end of file From b8aaba9126ce84e4371f49d644743213d798ba9f Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Thu, 15 Dec 2022 11:20:26 -0500 Subject: [PATCH 2/7] merging in 3.11 updates --- Lib/ctypes/test/test_loading.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Lib/ctypes/test/test_loading.py b/Lib/ctypes/test/test_loading.py index e7ebfa55d2e7bb..b61d6fa2912ac4 100644 --- a/Lib/ctypes/test/test_loading.py +++ b/Lib/ctypes/test/test_loading.py @@ -5,6 +5,8 @@ import sys import unittest import test.support +from test.support import import_helper +from test.support import os_helper from ctypes.util import find_library libc_name = None @@ -91,7 +93,7 @@ def test_1703286_A(self): # NOT fit into a 32-bit integer. FreeLibrary must be able # to accept this address. - # These are tests for http://www.python.org/sf/1703286 + # These are tests for https://www.python.org/sf/1703286 handle = LoadLibrary("advapi32") FreeLibrary(handle) @@ -123,14 +125,14 @@ def test_load_hasattr(self): @unittest.skipUnless(os.name == "nt", 'test specific to Windows') def test_load_dll_with_flags(self): - _sqlite3 = test.support.import_module("_sqlite3") + _sqlite3 = import_helper.import_module("_sqlite3") src = _sqlite3.__file__ if src.lower().endswith("_d.pyd"): ext = "_d.dll" else: ext = ".dll" - with test.support.temp_dir() as tmp: + with os_helper.temp_dir() as tmp: # We copy two files and load _sqlite3.dll (formerly .pyd), # which has a dependency on sqlite3.dll. Then we test # loading it in subprocesses to avoid it starting in memory From f7487c421b016151cbc0d069d471b46fad82f5c4 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Thu, 15 Dec 2022 11:26:27 -0500 Subject: [PATCH 3/7] adding new line at end of file for docs --- .../next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst b/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst index 3b901a5f00da31..084d197e3a0a7d 100644 --- a/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst +++ b/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst @@ -1 +1 @@ -Fix issue where hasattr(ctypes.windll, 'somefunction') raises OSError instead of the expected behavior: returning False \ No newline at end of file +Fix issue where hasattr(ctypes.windll, 'somefunction') raises OSError instead of the expected behavior: returning False From 76f0039522c97c684684bcb171cb4ce177a02589 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Thu, 15 Dec 2022 16:28:12 -0500 Subject: [PATCH 4/7] updating news per feedback --- .../next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst b/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst index 084d197e3a0a7d..3bb8de8936b816 100644 --- a/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst +++ b/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst @@ -1 +1 @@ -Fix issue where hasattr(ctypes.windll, 'somefunction') raises OSError instead of the expected behavior: returning False +`hasattr(ctypes.windll, 'nonexistant')` now returns `False` instead of raising `OSError`. From 12d77d2a829cb9f30a599a70965cc793c1a0f056 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filipe=20La=C3=ADns?= Date: Thu, 15 Dec 2022 21:33:20 +0000 Subject: [PATCH 5/7] Update Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst --- .../next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst b/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst index 3bb8de8936b816..15bb20d2030491 100644 --- a/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst +++ b/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst @@ -1 +1,2 @@ -`hasattr(ctypes.windll, 'nonexistant')` now returns `False` instead of raising `OSError`. +``hasattr(ctypes.windll, 'nonexistant')`` now returns `False` instead of raising exc:`OSError`. + From 6c6690407aea1e3c41f11f2f39c96312570f6a34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filipe=20La=C3=ADns?= Date: Thu, 15 Dec 2022 22:06:44 +0000 Subject: [PATCH 6/7] Update Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst --- .../next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst b/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst index 15bb20d2030491..6485e1a21bc3d4 100644 --- a/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst +++ b/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst @@ -1,2 +1,2 @@ -``hasattr(ctypes.windll, 'nonexistant')`` now returns `False` instead of raising exc:`OSError`. +``hasattr(ctypes.windll, 'nonexistant')`` now returns `False` instead of raising `exc:`OSError`. From f151341139bcc73ef12adf66713ef3c974073051 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filipe=20La=C3=ADns?= Date: Thu, 15 Dec 2022 22:11:22 +0000 Subject: [PATCH 7/7] Update Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst Co-authored-by: Zachary Ware --- .../next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst b/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst index 6485e1a21bc3d4..6fe5f12cc9d9d4 100644 --- a/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst +++ b/Misc/NEWS.d/next/Windows/2021-04-08-00-36-37.bpo-34816.4Xe0id.rst @@ -1,2 +1,3 @@ -``hasattr(ctypes.windll, 'nonexistant')`` now returns `False` instead of raising `exc:`OSError`. +``hasattr(ctypes.windll, 'nonexistant')`` now returns ``False`` instead of raising :exc:`OSError`. +