From 94f30c75576bb8a20724b2ac758fa33af089a522 Mon Sep 17 00:00:00 2001 From: Barney Gale Date: Thu, 11 May 2023 01:01:39 +0100 Subject: GH-90208: Suppress OSError exceptions from `pathlib.Path.glob()` (GH-104141) `pathlib.Path.glob()` now suppresses all OSError exceptions, except those raised from calling `is_dir()` on the top-level path. Previously, `glob()` suppressed ENOENT, ENOTDIR, EBADF and ELOOP errors and their Windows equivalents. PermissionError was also suppressed unless it occurred when calling `is_dir()` on the top-level path. However, the selector would abort prematurely if a PermissionError was raised, and so `glob()` could return incomplete results. --- Lib/pathlib.py | 33 ++++++++----------- Lib/test/test_pathlib.py | 38 +++++++--------------- .../2023-05-03-19-22-24.gh-issue-90208.tI00da.rst | 4 +++ 3 files changed, 29 insertions(+), 46 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-05-03-19-22-24.gh-issue-90208.tI00da.rst diff --git a/Lib/pathlib.py b/Lib/pathlib.py index 25863c7..40b7293 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -142,25 +142,21 @@ class _WildcardSelector(_Selector): # avoid exhausting file descriptors when globbing deep trees. with scandir(parent_path) as scandir_it: entries = list(scandir_it) + except OSError: + pass + else: for entry in entries: if self.dironly: try: - # "entry.is_dir()" can raise PermissionError - # in some cases (see bpo-38894), which is not - # among the errors ignored by _ignore_error() if not entry.is_dir(): continue - except OSError as e: - if not _ignore_error(e): - raise + except OSError: continue name = entry.name if self.match(name): path = parent_path._make_child_relpath(name) for p in self.successor._select_from(path, scandir): yield p - except PermissionError: - return class _RecursiveWildcardSelector(_Selector): @@ -175,28 +171,25 @@ class _RecursiveWildcardSelector(_Selector): # avoid exhausting file descriptors when globbing deep trees. with scandir(parent_path) as scandir_it: entries = list(scandir_it) + except OSError: + pass + else: for entry in entries: entry_is_dir = False try: entry_is_dir = entry.is_dir(follow_symlinks=False) - except OSError as e: - if not _ignore_error(e): - raise + except OSError: + pass if entry_is_dir: path = parent_path._make_child_relpath(entry.name) for p in self._iterate_directories(path, scandir): yield p - except PermissionError: - return def _select_from(self, parent_path, scandir): - try: - successor_select = self.successor._select_from - for starting_point in self._iterate_directories(parent_path, scandir): - for p in successor_select(starting_point, scandir): - yield p - except PermissionError: - return + successor_select = self.successor._select_from + for starting_point in self._iterate_directories(parent_path, scandir): + for p in successor_select(starting_point, scandir): + yield p class _DoubleRecursiveWildcardSelector(_RecursiveWildcardSelector): diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index 10dd604..67ca479 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -1949,33 +1949,19 @@ class _BasePathTest(object): P = self.cls base = P(BASE) / 'permissions' base.mkdir() + self.addCleanup(os_helper.rmtree, base) - file1 = base / "file1" - file1.touch() - file2 = base / "file2" - file2.touch() - - subdir = base / "subdir" - - file3 = base / "file3" - file3.symlink_to(subdir / "other") - - # Patching is needed to avoid relying on the filesystem - # to return the order of the files as the error will not - # happen if the symlink is the last item. - real_scandir = os.scandir - def my_scandir(path): - with real_scandir(path) as scandir_it: - entries = list(scandir_it) - entries.sort(key=lambda entry: entry.name) - return contextlib.nullcontext(entries) - - with mock.patch("os.scandir", my_scandir): - self.assertEqual(len(set(base.glob("*"))), 3) - subdir.mkdir() - self.assertEqual(len(set(base.glob("*"))), 4) - subdir.chmod(000) - self.assertEqual(len(set(base.glob("*"))), 4) + for i in range(100): + link = base / f"link{i}" + if i % 2: + link.symlink_to(P(BASE, "dirE", "nonexistent")) + else: + link.symlink_to(P(BASE, "dirC")) + + self.assertEqual(len(set(base.glob("*"))), 100) + self.assertEqual(len(set(base.glob("*/"))), 50) + self.assertEqual(len(set(base.glob("*/fileC"))), 50) + self.assertEqual(len(set(base.glob("*/file*"))), 50) @os_helper.skip_unless_symlink def test_glob_long_symlink(self): diff --git a/Misc/NEWS.d/next/Library/2023-05-03-19-22-24.gh-issue-90208.tI00da.rst b/Misc/NEWS.d/next/Library/2023-05-03-19-22-24.gh-issue-90208.tI00da.rst new file mode 100644 index 0000000..1fd9588 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-05-03-19-22-24.gh-issue-90208.tI00da.rst @@ -0,0 +1,4 @@ +Fixed issue where :meth:`pathlib.Path.glob` returned incomplete results when +it encountered a :exc:`PermissionError`. This method now suppresses all +:exc:`OSError` exceptions, except those raised from calling +:meth:`~pathlib.Path.is_dir` on the top-level path. -- cgit v0.12