bpo-38894: Fix pathlib.Path.glob in the presence of symlinks and insufficient permissions (GH-18815)

pablogsal · godlygeek · web-flow · commit eb7560a73d46 · 2020-03-07T17:53:20.000Z
Co-authored-by: Matt Wozniski &lt;mwozniski@bloomberg.net&gt;
diff --git a/Lib/pathlib.py b/Lib/pathlib.py
@@ -529,23 +529,26 @@ def _select_from(self, parent_path, is_dir, exists, scandir):
         try:
             entries = list(scandir(parent_path))
             for entry in entries:
-                entry_is_dir = False
-                try:
-                    entry_is_dir = entry.is_dir()
-                except OSError as e:
-                    if not _ignore_error(e):
-                        raise
-                if not self.dironly or entry_is_dir:
-                    name = entry.name
-                    if self.match(name):
-                        path = parent_path._make_child_relpath(name)
-                        for p in self.successor._select_from(path, is_dir, exists, scandir):
-                            yield p
+                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
+                        continue
+                name = entry.name
+                if self.match(name):
+                    path = parent_path._make_child_relpath(name)
+                    for p in self.successor._select_from(path, is_dir, exists, scandir):
+                        yield p
         except PermissionError:
             return
 
 
-
 class _RecursiveWildcardSelector(_Selector):
 
     def __init__(self, pat, child_parts, flavour):
diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py
@@ -1595,6 +1595,42 @@ def test_glob_dotdot(self):
         self.assertEqual(set(p.glob("dirA/../file*")), { P(BASE, "dirA/../fileA") })
         self.assertEqual(set(p.glob("../xyzzy")), set())
 
+    @support.skip_unless_symlink
+    def test_glob_permissions(self):
+        # See bpo-38894
+        P = self.cls
+        base = P(BASE) / 'permissions'
+        base.mkdir()
+
+        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.
+
+        with mock.patch("os.scandir") as scandir:
+            scandir.return_value = sorted(os.scandir(base))
+            self.assertEqual(len(set(base.glob("*"))), 3)
+
+        subdir.mkdir()
+
+        with mock.patch("os.scandir") as scandir:
+            scandir.return_value = sorted(os.scandir(base))
+            self.assertEqual(len(set(base.glob("*"))), 4)
+
+        subdir.chmod(000)
+
+        with mock.patch("os.scandir") as scandir:
+            scandir.return_value = sorted(os.scandir(base))
+            self.assertEqual(len(set(base.glob("*"))), 4)
 
     def _check_resolve(self, p, expected, strict=True):
         q = p.resolve(strict)
diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-03-06-21-04-39.bpo-38894.nfcGKv.rst b/Misc/NEWS.d/next/Core and Builtins/2020-03-06-21-04-39.bpo-38894.nfcGKv.rst
@@ -0,0 +1,4 @@
+Fix a bug that was causing incomplete results when calling
+``pathlib.Path.glob`` in the presence of symlinks that point
+to files where the user does not have read access. Patch by Pablo
+Galindo and Matt Wozniski.
