From d4336d8a6d8d9f90aaa4097299621a2197a8bc75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 29 Jul 2024 11:19:16 +0200 Subject: [PATCH 01/17] handle ValueError --- Lib/importlib/_bootstrap_external.py | 40 +++++++++++++++++----------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index 2bb44b290e4a84..aa67f36a77dc27 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -148,6 +148,7 @@ def _path_stat(path): Made a separate function to make it easier to override in experiments (e.g. cache stat results). + This function may raise OSError or ValueError. """ return _os.stat(path) @@ -156,7 +157,7 @@ def _path_is_mode_type(path, mode): """Test whether the path is the specified mode type.""" try: stat_info = _path_stat(path) - except OSError: + except (OSError, ValueError): return False return (stat_info.st_mode & 0o170000) == mode @@ -211,10 +212,10 @@ def _write_atomic(path, data, mode=0o666): with _io.FileIO(fd, 'wb') as file: file.write(data) _os.replace(path_tmp, path) - except OSError: + except (OSError, ValueError): try: _os.unlink(path_tmp) - except OSError: + except (OSError, ValueError): pass raise @@ -654,7 +655,7 @@ def _calc_mode(path): """Calculate the mode permissions for a bytecode file.""" try: mode = _path_stat(path).st_mode - except OSError: + except (OSError, ValueError): mode = 0o666 # We always ensure write access so we can update cached files # later even when the source files are read-only on Windows (#6074) @@ -990,7 +991,7 @@ def find_spec(cls, fullname, path=None, target=None): return None try: _path_stat(filepath) - except OSError: + except (OSError, ValueError): return None for loader, suffixes in _get_supported_file_loaders(): if filepath.endswith(tuple(suffixes)): @@ -1036,7 +1037,7 @@ def path_mtime(self, path): """Optional method that returns the modification time (an int) for the specified path (a str). - Raises OSError when the path cannot be handled. + Raises OSError or ValueError when the path cannot be handled. """ raise OSError @@ -1050,7 +1051,7 @@ def path_stats(self, path): - 'size' (optional) is the size in bytes of the source code. Implementing this method allows the loader to read bytecode files. - Raises OSError when the path cannot be handled. + Raises OSError or ValueError when the path cannot be handled. """ return {'mtime': self.path_mtime(path)} @@ -1076,7 +1077,7 @@ def get_source(self, fullname): path = self.get_filename(fullname) try: source_bytes = self.get_data(path) - except OSError as exc: + except (OSError, ValueError) as exc: raise ImportError('source not available through get_data()', name=fullname) from exc return decode_source(source_bytes) @@ -1109,13 +1110,13 @@ def get_code(self, fullname): else: try: st = self.path_stats(source_path) - except OSError: + except (OSError, ValueError): pass else: source_mtime = int(st['mtime']) try: data = self.get_data(bytecode_path) - except OSError: + except (OSError, ValueError): pass else: exc_details = { @@ -1211,7 +1212,10 @@ def get_filename(self, fullname): return self.path def get_data(self, path): - """Return the data from path as raw bytes.""" + """Return the data from path as raw bytes. + + This may raise an OSError or a ValueError if the path is invalid. + """ if isinstance(self, (SourceLoader, ExtensionFileLoader)): with _io.open_code(str(path)) as file: return file.read() @@ -1255,19 +1259,20 @@ def set_data(self, path, data, *, _mode=0o666): except FileExistsError: # Probably another Python process already created the dir. continue - except OSError as exc: - # Could be a permission error, read-only filesystem: just forget - # about writing the data. + except (OSError, ValueError) as exc: + # Could be a permission error, read-only filesystem, or + # an invalid path name: just forget about writing the data. _bootstrap._verbose_message('could not create {!r}: {!r}', parent, exc) return try: _write_atomic(path, data, _mode) - _bootstrap._verbose_message('created {!r}', path) - except OSError as exc: + except (OSError, ValueError) as exc: # Same as above: just don't write the bytecode. _bootstrap._verbose_message('could not create {!r}: {!r}', path, exc) + else: + _bootstrap._verbose_message('created {!r}', path) class SourcelessFileLoader(FileLoader, _LoaderBasics): @@ -1631,6 +1636,9 @@ def find_spec(self, fullname, target=None): mtime = _path_stat(self.path or _os.getcwd()).st_mtime except OSError: mtime = -1 + except ValueError: + # Invalid paths will never be usable even if mtime = -1. + return None if mtime != self._path_mtime: self._fill_cache() self._path_mtime = mtime From a7e34cae34f0e9109f773b8dfe1c3ce3c98d0213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 29 Jul 2024 11:19:19 +0200 Subject: [PATCH 02/17] add tests --- Lib/test/test_importlib/import_/test_path.py | 27 ++++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_importlib/import_/test_path.py b/Lib/test/test_importlib/import_/test_path.py index 89b52fbd1e1aff..cbf6994947a744 100644 --- a/Lib/test/test_importlib/import_/test_path.py +++ b/Lib/test/test_importlib/import_/test_path.py @@ -93,13 +93,30 @@ def test_path_importer_cache_empty_string(self): self.check_found(found, importer) self.assertIn(os.getcwd(), sys.path_importer_cache) - def test_None_on_sys_path(self): - # Putting None in sys.path[0] caused an import regression from Python - # 3.2: http://bugs.python.org/issue16514 + def test_invalid_names_in_sys_path(self): + for name, desc in [ + # Putting None in sys.path[0] caused an import regression from + # Python 3.2: http://bugs.python.org/issue16514 + (None, 'None in sys.path[0]'), + # embedded NUL characters raise ValueError in os.stat() + ('\x00', 'NUL bytes path'), + (f'Top{os.sep}Mid\x00', 'path with embedded NUL bytes'), + # A filename with surrogate codes. A UnicodeEncodeError is raised + # by os.stat() upon querying, which is a subclass of ValueError. + ("\uD834\uDD1E.py", 'surrogate codes (MUSICAL SYMBOL G CLEF)'), + # For POSIX platforms, an OSError will be raised but for Windows + # platforms, a ValueError is raised due to the path_t converter. + # See: https://github.com/python/cpython/issues/122353 + ('a' * 1_000_000, 'very long path'), + ]: + with self.subTest(desc): + self._test_invalid_name_in_sys_path(name) + + def _test_invalid_name_in_sys_path(self, name): new_path = sys.path[:] - new_path.insert(0, None) + new_path.insert(0, name) new_path_importer_cache = sys.path_importer_cache.copy() - new_path_importer_cache.pop(None, None) + new_path_importer_cache.pop(name, None) new_path_hooks = [zipimport.zipimporter, self.machinery.FileFinder.path_hook( *self.importlib._bootstrap_external._get_supported_file_loaders())] From a14a08a6e29d4e0315b431a237551853c47aa8b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 29 Jul 2024 11:21:49 +0200 Subject: [PATCH 03/17] blurb --- .../next/Library/2024-07-29-11-21-39.gh-issue-122353.ESrsyI.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2024-07-29-11-21-39.gh-issue-122353.ESrsyI.rst diff --git a/Misc/NEWS.d/next/Library/2024-07-29-11-21-39.gh-issue-122353.ESrsyI.rst b/Misc/NEWS.d/next/Library/2024-07-29-11-21-39.gh-issue-122353.ESrsyI.rst new file mode 100644 index 00000000000000..f9a42c1c8a0509 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-07-29-11-21-39.gh-issue-122353.ESrsyI.rst @@ -0,0 +1,2 @@ +Handle :exc:`ValueError`\s raised by OS-related functions during import if +``sys.path`` contains invalid path names. Patch by Bénédikt Tran. From b4619f0caa281c58bae9726811c48eb73ef826e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 31 Jul 2024 12:31:11 +0200 Subject: [PATCH 04/17] relax guards --- Lib/importlib/_bootstrap_external.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index aa67f36a77dc27..ef845853677afb 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -212,10 +212,10 @@ def _write_atomic(path, data, mode=0o666): with _io.FileIO(fd, 'wb') as file: file.write(data) _os.replace(path_tmp, path) - except (OSError, ValueError): + except OSError: try: _os.unlink(path_tmp) - except (OSError, ValueError): + except OSError: pass raise @@ -1267,7 +1267,7 @@ def set_data(self, path, data, *, _mode=0o666): return try: _write_atomic(path, data, _mode) - except (OSError, ValueError) as exc: + except OSError as exc: # Same as above: just don't write the bytecode. _bootstrap._verbose_message('could not create {!r}: {!r}', path, exc) From 16c22b9fde4b0b82ca706e02f9eb0916094f7af0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 31 Jul 2024 12:32:09 +0200 Subject: [PATCH 05/17] use a (safe) bare except --- Lib/importlib/_bootstrap_external.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index ef845853677afb..f6c33d76201042 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -212,7 +212,7 @@ def _write_atomic(path, data, mode=0o666): with _io.FileIO(fd, 'wb') as file: file.write(data) _os.replace(path_tmp, path) - except OSError: + except: try: _os.unlink(path_tmp) except OSError: From 9aa048d5169cece8e2121d4c03b6b04f1dde57e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 31 Jul 2024 12:33:49 +0200 Subject: [PATCH 06/17] remove file extension in test --- Lib/test/test_importlib/import_/test_path.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_importlib/import_/test_path.py b/Lib/test/test_importlib/import_/test_path.py index cbf6994947a744..3851bbc09bb264 100644 --- a/Lib/test/test_importlib/import_/test_path.py +++ b/Lib/test/test_importlib/import_/test_path.py @@ -103,7 +103,7 @@ def test_invalid_names_in_sys_path(self): (f'Top{os.sep}Mid\x00', 'path with embedded NUL bytes'), # A filename with surrogate codes. A UnicodeEncodeError is raised # by os.stat() upon querying, which is a subclass of ValueError. - ("\uD834\uDD1E.py", 'surrogate codes (MUSICAL SYMBOL G CLEF)'), + ("\uD834\uDD1E", 'surrogate codes (MUSICAL SYMBOL G CLEF)'), # For POSIX platforms, an OSError will be raised but for Windows # platforms, a ValueError is raised due to the path_t converter. # See: https://github.com/python/cpython/issues/122353 From b554b2c05332373d711ea4f12e772f6a569a3382 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 31 Jul 2024 12:50:24 +0200 Subject: [PATCH 07/17] update docs --- Doc/library/importlib.rst | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst index 1206a2d94d22a3..2a0600364eeaac 100644 --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -416,12 +416,16 @@ ABC hierarchy:: Loaders that have a file-like storage back-end that allows storing arbitrary data can implement this abstract method to give direct access - to the data stored. :exc:`OSError` is to be raised if the *path* cannot - be found. The *path* is expected to be constructed using a module's - :attr:`__file__` attribute or an item from a package's :attr:`__path__`. + to the data stored. + + An :exc:`OSError` is to be raised if the *path* cannot be found, and + a :exc:`ValueError` is raised if the *path* cannot be handled (e.g., + the *path* contains null characters or the *path* is too long). The + *path* is expected to be constructed using a module's :attr:`__file__` + attribute or an item from a package's :attr:`__path__`. .. versionchanged:: 3.4 - Raises :exc:`OSError` instead of :exc:`NotImplementedError`. + Raise :exc:`OSError` by default instead of :exc:`NotImplementedError`. .. class:: InspectLoader @@ -551,6 +555,9 @@ ABC hierarchy:: Reads *path* as a binary file and returns the bytes from it. + An :exc:`OSError` is to be raised if the *path* cannot be found, and + a :exc:`ValueError` is raised if the *path* is invalid (e.g., *path* + contains null characters or is too long). .. class:: SourceLoader @@ -582,12 +589,13 @@ ABC hierarchy:: - ``'size'`` (optional): the size in bytes of the source code. Any other keys in the dictionary are ignored, to allow for future - extensions. If the path cannot be handled, :exc:`OSError` is raised. + extensions. If the path cannot be handled, raises an :exc:`OSError` + or a :exc:`ValueError` depending on the reason. .. versionadded:: 3.3 .. versionchanged:: 3.4 - Raise :exc:`OSError` instead of :exc:`NotImplementedError`. + Raise :exc:`OSError` by default instead of :exc:`NotImplementedError`. .. method:: path_mtime(path) @@ -597,10 +605,11 @@ ABC hierarchy:: .. deprecated:: 3.3 This method is deprecated in favour of :meth:`path_stats`. You don't have to implement it, but it is still available for compatibility - purposes. Raise :exc:`OSError` if the path cannot be handled. + purposes. If the path cannot be handled, raises an :exc:`OSError` + or a :exc:`ValueError` depending on the reason. .. versionchanged:: 3.4 - Raise :exc:`OSError` instead of :exc:`NotImplementedError`. + Raise :exc:`OSError` by default instead of :exc:`NotImplementedError`. .. method:: set_data(path, data) @@ -608,9 +617,9 @@ ABC hierarchy:: path. Any intermediate directories which do not exist are to be created automatically. - When writing to the path fails because the path is read-only - (:const:`errno.EACCES`/:exc:`PermissionError`), do not propagate the - exception. + When writing to the path fails by raising an :class:`OSError` (e.g., + the path is read-only (:const:`errno.EACCES`/:exc:`PermissionError`) + or a :class:`ValueError`, the exception is not propagated. .. versionchanged:: 3.4 No longer raises :exc:`NotImplementedError` when called. From f0fb424108560f54b046633756e3c4575dee6f0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 31 Jul 2024 13:41:48 +0200 Subject: [PATCH 08/17] keep the correct guard! --- Lib/importlib/_bootstrap_external.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index f6c33d76201042..3e8d656877cc48 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -1267,7 +1267,7 @@ def set_data(self, path, data, *, _mode=0o666): return try: _write_atomic(path, data, _mode) - except OSError as exc: + except (OSError, ValueError) as exc: # Same as above: just don't write the bytecode. _bootstrap._verbose_message('could not create {!r}: {!r}', path, exc) From 622d3a15c2c566daa4ce1c83b91e87190a45e2a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 31 Jul 2024 13:49:12 +0200 Subject: [PATCH 09/17] simplify docs --- Doc/library/importlib.rst | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst index 2a0600364eeaac..2917c7cd021469 100644 --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -417,12 +417,12 @@ ABC hierarchy:: that allows storing arbitrary data can implement this abstract method to give direct access to the data stored. + The *path* is expected to be constructed using a module's + :attr:`__file__` attribute or an item from a package's + :attr:`__path__` attribute. - An :exc:`OSError` is to be raised if the *path* cannot be found, and - a :exc:`ValueError` is raised if the *path* cannot be handled (e.g., - the *path* contains null characters or the *path* is too long). The - *path* is expected to be constructed using a module's :attr:`__file__` - attribute or an item from a package's :attr:`__path__`. + If the *path* cannot be handled, this raises an :exc:`OSError` + or a :exc:`ValueError` depending on the reason. .. versionchanged:: 3.4 Raise :exc:`OSError` by default instead of :exc:`NotImplementedError`. @@ -555,9 +555,8 @@ ABC hierarchy:: Reads *path* as a binary file and returns the bytes from it. - An :exc:`OSError` is to be raised if the *path* cannot be found, and - a :exc:`ValueError` is raised if the *path* is invalid (e.g., *path* - contains null characters or is too long). + If the *path* cannot be handled, this raises an :exc:`OSError` + or a :exc:`ValueError` depending on the reason. .. class:: SourceLoader @@ -589,8 +588,8 @@ ABC hierarchy:: - ``'size'`` (optional): the size in bytes of the source code. Any other keys in the dictionary are ignored, to allow for future - extensions. If the path cannot be handled, raises an :exc:`OSError` - or a :exc:`ValueError` depending on the reason. + extensions. If the *path* cannot be handled, this raises an + :exc:`OSError` or a :exc:`ValueError` depending on the reason. .. versionadded:: 3.3 @@ -605,8 +604,8 @@ ABC hierarchy:: .. deprecated:: 3.3 This method is deprecated in favour of :meth:`path_stats`. You don't have to implement it, but it is still available for compatibility - purposes. If the path cannot be handled, raises an :exc:`OSError` - or a :exc:`ValueError` depending on the reason. + purposes. If the *path* cannot be handled, this raises an + :exc:`OSError` or a :exc:`ValueError` depending on the reason. .. versionchanged:: 3.4 Raise :exc:`OSError` by default instead of :exc:`NotImplementedError`. From b2b119039fb57948b675b801a2a5cc4055138711 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 31 Jul 2024 13:51:04 +0200 Subject: [PATCH 10/17] simplify --- Doc/library/importlib.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst index 2917c7cd021469..6aa18c32c23a1a 100644 --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -616,8 +616,7 @@ ABC hierarchy:: path. Any intermediate directories which do not exist are to be created automatically. - When writing to the path fails by raising an :class:`OSError` (e.g., - the path is read-only (:const:`errno.EACCES`/:exc:`PermissionError`) + When writing to the path fails by raising an :class:`OSError` or a :class:`ValueError`, the exception is not propagated. .. versionchanged:: 3.4 From ef21e28da634ca9d9678bc855bf64ea9c03094c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 31 Jul 2024 13:57:06 +0200 Subject: [PATCH 11/17] fixup! comment --- Lib/test/test_importlib/import_/test_path.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_importlib/import_/test_path.py b/Lib/test/test_importlib/import_/test_path.py index 3851bbc09bb264..f967ad4108d74a 100644 --- a/Lib/test/test_importlib/import_/test_path.py +++ b/Lib/test/test_importlib/import_/test_path.py @@ -101,7 +101,7 @@ def test_invalid_names_in_sys_path(self): # embedded NUL characters raise ValueError in os.stat() ('\x00', 'NUL bytes path'), (f'Top{os.sep}Mid\x00', 'path with embedded NUL bytes'), - # A filename with surrogate codes. A UnicodeEncodeError is raised + # A directory with surrogate codes. A UnicodeEncodeError is raised # by os.stat() upon querying, which is a subclass of ValueError. ("\uD834\uDD1E", 'surrogate codes (MUSICAL SYMBOL G CLEF)'), # For POSIX platforms, an OSError will be raised but for Windows From aa993976b327a362850520cf591877d99c388862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 31 Jul 2024 13:57:41 +0200 Subject: [PATCH 12/17] fixup! even better --- Lib/test/test_importlib/import_/test_path.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_importlib/import_/test_path.py b/Lib/test/test_importlib/import_/test_path.py index f967ad4108d74a..30d5558e3f01ba 100644 --- a/Lib/test/test_importlib/import_/test_path.py +++ b/Lib/test/test_importlib/import_/test_path.py @@ -101,7 +101,7 @@ def test_invalid_names_in_sys_path(self): # embedded NUL characters raise ValueError in os.stat() ('\x00', 'NUL bytes path'), (f'Top{os.sep}Mid\x00', 'path with embedded NUL bytes'), - # A directory with surrogate codes. A UnicodeEncodeError is raised + # A path with surrogate codes. A UnicodeEncodeError is raised # by os.stat() upon querying, which is a subclass of ValueError. ("\uD834\uDD1E", 'surrogate codes (MUSICAL SYMBOL G CLEF)'), # For POSIX platforms, an OSError will be raised but for Windows From b039c622d6653b663e1adaa644ffcfcc9897bbaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 12 Aug 2024 17:34:13 +0200 Subject: [PATCH 13/17] use imperative style --- Doc/library/importlib.rst | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst index 6aa18c32c23a1a..483325e344a48a 100644 --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -421,8 +421,12 @@ ABC hierarchy:: :attr:`__file__` attribute or an item from a package's :attr:`__path__` attribute. - If the *path* cannot be handled, this raises an :exc:`OSError` - or a :exc:`ValueError` depending on the reason. + If the *path* cannot be handled, either :exc:`OSError` or :exc:`ValueError` + is to be raised. In most cases, these will be raised by the underlying + operating system interfaces rather than directly (e.g., :exc:`OSError` + would be raised when attempting to access a valid but nonexistent + filesystem path, while attempting to access a path containing a NULL + byte would raise :exc:`ValueError`). .. versionchanged:: 3.4 Raise :exc:`OSError` by default instead of :exc:`NotImplementedError`. @@ -588,8 +592,10 @@ ABC hierarchy:: - ``'size'`` (optional): the size in bytes of the source code. Any other keys in the dictionary are ignored, to allow for future - extensions. If the *path* cannot be handled, this raises an - :exc:`OSError` or a :exc:`ValueError` depending on the reason. + extensions. + + As for :meth:`ResourecLoader.get_data`, either :exc:`OSError` or + :exc:`ValueError` is to be raised if the *path* cannot be handled. .. versionadded:: 3.3 @@ -604,8 +610,7 @@ ABC hierarchy:: .. deprecated:: 3.3 This method is deprecated in favour of :meth:`path_stats`. You don't have to implement it, but it is still available for compatibility - purposes. If the *path* cannot be handled, this raises an - :exc:`OSError` or a :exc:`ValueError` depending on the reason. + purposes. .. versionchanged:: 3.4 Raise :exc:`OSError` by default instead of :exc:`NotImplementedError`. From 3a27cb031c2ed227474999ae2622a5ed43b57c31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 12 Aug 2024 17:34:39 +0200 Subject: [PATCH 14/17] narrow exceptions guards in `SourceFileLoader.set_data` --- Doc/library/importlib.rst | 5 +++-- Lib/importlib/_bootstrap_external.py | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst index 483325e344a48a..90a2de1f385f73 100644 --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -621,8 +621,9 @@ ABC hierarchy:: path. Any intermediate directories which do not exist are to be created automatically. - When writing to the path fails by raising an :class:`OSError` - or a :class:`ValueError`, the exception is not propagated. + When writing to the path fails because the path is read-only + (:const:`errno.EACCES`/:exc:`PermissionError`), do not propagate the + exception. Other exceptions should still be propagated. .. versionchanged:: 3.4 No longer raises :exc:`NotImplementedError` when called. diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index 3e8d656877cc48..6a270069a3154d 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -1259,15 +1259,15 @@ def set_data(self, path, data, *, _mode=0o666): except FileExistsError: # Probably another Python process already created the dir. continue - except (OSError, ValueError) as exc: - # Could be a permission error, read-only filesystem, or - # an invalid path name: just forget about writing the data. + except PermissionError as exc: + # Could be a permission error, read-only filesystem: + # just forget about writing the data. _bootstrap._verbose_message('could not create {!r}: {!r}', parent, exc) return try: _write_atomic(path, data, _mode) - except (OSError, ValueError) as exc: + except PermissionError as exc: # Same as above: just don't write the bytecode. _bootstrap._verbose_message('could not create {!r}: {!r}', path, exc) From f2b269a7eb237b01d0b3919c35dd2da47bcf49ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 12 Aug 2024 18:06:15 +0200 Subject: [PATCH 15/17] guard against permissions and EROFS errors --- Lib/importlib/_bootstrap_external.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index 6a270069a3154d..f68ac4d9dac600 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -1259,18 +1259,25 @@ def set_data(self, path, data, *, _mode=0o666): except FileExistsError: # Probably another Python process already created the dir. continue - except PermissionError as exc: - # Could be a permission error, read-only filesystem: + except OSError as exc: + # Could be a permission error or read-only filesystem (EROFS): # just forget about writing the data. - _bootstrap._verbose_message('could not create {!r}: {!r}', - parent, exc) - return + from errno import EROFS + if isinstance(exc, PermissionError) or exc.errno == EROFS: + _bootstrap._verbose_message('could not create {!r}: {!r}', + parent, exc) + return + raise try: _write_atomic(path, data, _mode) - except PermissionError as exc: + except OSError as exc: # Same as above: just don't write the bytecode. - _bootstrap._verbose_message('could not create {!r}: {!r}', path, - exc) + from errno import EROFS + if isinstance(exc, PermissionError) or exc.errno == EROFS: + _bootstrap._verbose_message('could not create {!r}: {!r}', + path, exc) + return + raise else: _bootstrap._verbose_message('created {!r}', path) From cbce6d3a9898122bebde78e7454de7a884f1a902 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 12 Aug 2024 18:11:09 +0200 Subject: [PATCH 16/17] guard against permissions and EROFS errors --- Lib/importlib/_bootstrap_external.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index 3f4c8631cdc8d0..7ef29a238ec81d 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -989,18 +989,26 @@ def set_data(self, path, data, *, _mode=0o666): except OSError as exc: # Could be a permission error or read-only filesystem (EROFS): # just forget about writing the data. - from errno import EROFS - if isinstance(exc, PermissionError) or exc.errno == EROFS: + from errno import EACCES, EROFS + + if ( + isinstance(exc, PermissionError) + or exc.errno in {EACCES, EROFS} + ): _bootstrap._verbose_message('could not create {!r}: {!r}', - parent, exc) + parent, exc) return raise try: _write_atomic(path, data, _mode) except OSError as exc: # Same as above: just don't write the bytecode. - from errno import EROFS - if isinstance(exc, PermissionError) or exc.errno == EROFS: + from errno import EACCES, EROFS + + if ( + isinstance(exc, PermissionError) + or exc.errno in {EACCES, EROFS} + ): _bootstrap._verbose_message('could not create {!r}: {!r}', path, exc) return From 24c95a591b889a4703e9f2d562c54cd48aa2ea9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 12 Aug 2024 18:11:50 +0200 Subject: [PATCH 17/17] fixup indentation --- Lib/importlib/_bootstrap_external.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index 7ef29a238ec81d..dd155c2c901ea9 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -996,7 +996,7 @@ def set_data(self, path, data, *, _mode=0o666): or exc.errno in {EACCES, EROFS} ): _bootstrap._verbose_message('could not create {!r}: {!r}', - parent, exc) + parent, exc) return raise try: