GH-109187: Improve symlink loop handling in `pathlib.Path.resolve()` (GH-109192)

Treat symlink loops like other errors: in strict mode, raise `OSError`, and
in non-strict mode, do not raise any exception.

diff --git a/Doc/library/pathlib.rst b/Doc/library/pathlib.rst
index 22360b22fd924b2570071e813b7c8d60a215c5b8..48d6176d26bb8f4028ff11254c0e98becdb33284 100644 (file)
--- a/Doc/library/pathlib.rst
+++ b/Doc/library/pathlib.rst
@@ -1381,15 +1381,19 @@ call fails (for example because the path doesn't exist).
       >>> p.resolve()
       PosixPath('/home/antoine/pathlib/setup.py')
 
-   If the path doesn't exist and *strict* is ``True``, :exc:`FileNotFoundError`
-   is raised.  If *strict* is ``False``, the path is resolved as far as possible
-   and any remainder is appended without checking whether it exists.  If an
-   infinite loop is encountered along the resolution path, :exc:`RuntimeError`
-   is raised.
+   If a path doesn't exist or a symlink loop is encountered, and *strict* is
+   ``True``, :exc:`OSError` is raised.  If *strict* is ``False``, the path is
+   resolved as far as possible and any remainder is appended without checking
+   whether it exists.
 
    .. versionchanged:: 3.6
       The *strict* parameter was added (pre-3.6 behavior is strict).
 
+   .. versionchanged:: 3.13
+      Symlink loops are treated like other errors: :exc:`OSError` is raised in
+      strict mode, and no exception is raised in non-strict mode. In previous
+      versions, :exc:`RuntimeError` is raised no matter the value of *strict*.
+
 .. method:: Path.rglob(pattern, *, case_sensitive=None, follow_symlinks=None)
 
    Glob the given relative *pattern* recursively.  This is like calling

diff --git a/Lib/pathlib.py b/Lib/pathlib.py
index f4ec315da6b4fa4c5401c55400515f13de7b62e9..bd5f61b0b7c878489a1a62e926173350b2072b5e 100644 (file)
--- a/Lib/pathlib.py
+++ b/Lib/pathlib.py
@@ -1230,26 +1230,7 @@ class Path(PurePath):
         normalizing it.
         """
 
-        def check_eloop(e):
-            winerror = getattr(e, 'winerror', 0)
-            if e.errno == ELOOP or winerror == _WINERROR_CANT_RESOLVE_FILENAME:
-                raise RuntimeError("Symlink loop from %r" % e.filename)
-
-        try:
-            s = os.path.realpath(self, strict=strict)
-        except OSError as e:
-            check_eloop(e)
-            raise
-        p = self.with_segments(s)
-
-        # In non-strict mode, realpath() doesn't raise on symlink loops.
-        # Ensure we get an exception by calling stat()
-        if not strict:
-            try:
-                p.stat()
-            except OSError as e:
-                check_eloop(e)
-        return p
+        return self.with_segments(os.path.realpath(self, strict=strict))
 
     def owner(self):
         """

diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py
index 09df3fe471fc3ef3aa1ef3ff418600dbe605ab02..484a5e6c3bd64d4e51def5412d9c0b11fa4769e1 100644 (file)
--- a/Lib/test/test_pathlib.py
+++ b/Lib/test/test_pathlib.py
@@ -3178,10 +3178,11 @@ class PosixPathTest(PathTest):
         self.assertEqual(str(P('//a').absolute()), '//a')
         self.assertEqual(str(P('//a/b').absolute()), '//a/b')
 
-    def _check_symlink_loop(self, *args, strict=True):
+    def _check_symlink_loop(self, *args):
         path = self.cls(*args)
-        with self.assertRaises(RuntimeError):
-            print(path.resolve(strict))
+        with self.assertRaises(OSError) as cm:
+            path.resolve(strict=True)
+        self.assertEqual(cm.exception.errno, errno.ELOOP)
 
     @unittest.skipIf(
         is_emscripten or is_wasi,
@@ -3240,7 +3241,8 @@ class PosixPathTest(PathTest):
         os.symlink('linkZ/../linkZ', join('linkZ'))
         self._check_symlink_loop(BASE, 'linkZ')
         # Non-strict
-        self._check_symlink_loop(BASE, 'linkZ', 'foo', strict=False)
+        p = self.cls(BASE, 'linkZ', 'foo')
+        self.assertEqual(p.resolve(strict=False), p)
         # Loops with absolute symlinks.
         os.symlink(join('linkU/inside'), join('linkU'))
         self._check_symlink_loop(BASE, 'linkU')
@@ -3249,7 +3251,8 @@ class PosixPathTest(PathTest):
         os.symlink(join('linkW/../linkW'), join('linkW'))
         self._check_symlink_loop(BASE, 'linkW')
         # Non-strict
-        self._check_symlink_loop(BASE, 'linkW', 'foo', strict=False)
+        q = self.cls(BASE, 'linkW', 'foo')
+        self.assertEqual(q.resolve(strict=False), q)
 
     def test_glob(self):
         P = self.cls

diff --git a/Misc/NEWS.d/next/Library/2023-09-09-17-09-54.gh-issue-109187.dIayNW.rst b/Misc/NEWS.d/next/Library/2023-09-09-17-09-54.gh-issue-109187.dIayNW.rst
new file mode 100644 (file)
index 0000000..31b3ef7
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2023-09-09-17-09-54.gh-issue-109187.dIayNW.rst
@@ -0,0 +1,3 @@
+:meth:`pathlib.Path.resolve` now treats symlink loops like other errors: in
+strict mode, :exc:`OSError` is raised, and in non-strict mode, no exception
+is raised.