Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-90208: Suppress OSError exceptions from pathlib.Path.glob() #104141

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 13 additions & 20 deletions Lib/pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,25 +142,21 @@ def _select_from(self, parent_path, scandir):
# 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):
Expand All @@ -175,28 +171,25 @@ def _iterate_directories(self, parent_path, scandir):
# 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):
Expand Down
38 changes: 12 additions & 26 deletions Lib/test/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -1949,33 +1949,19 @@ def test_glob_permissions(self):
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):
Expand Down
Original file line number Diff line number Diff line change
@@ -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.