Issue #19887: Improve the Path.resolve() algorithm to support certain symlink chains.

Original patch by Serhiy.

3 files changed, 96 insertions, 35 deletions

diff --git a/Lib/pathlib.py b/Lib/pathlib.py
index b404c1f..9b4fde1 100644
--- a/Lib/pathlib.py
+++ b/Lib/pathlib.py
@@ -254,42 +254,47 @@ class _PosixFlavour(_Flavour):
 
     def resolve(self, path):
         sep = self.sep
-        def split(p):
-            return [x for x in p.split(sep) if x]
-        def absparts(p):
-            # Our own abspath(), since the posixpath one makes
-            # the mistake of "normalizing" the path without resolving the
-            # symlinks first.
-            if not p.startswith(sep):
-                return split(os.getcwd()) + split(p)
-            else:
-                return split(p)
-        parts = absparts(str(path))[::-1]
         accessor = path._accessor
-        resolved = cur = ""
-        symlinks = {}
-        while parts:
-            part = parts.pop()
-            cur = resolved + sep + part
-            if cur in symlinks and symlinks[cur] <= len(parts):
-                # We've already seen the symlink and there's not less
-                # work to do than the last time.
-                raise RuntimeError("Symlink loop from %r" % cur)
-            try:
-                target = accessor.readlink(cur)
-            except OSError as e:
-                if e.errno != EINVAL:
-                    raise
-                # Not a symlink
-                resolved = cur
-            else:
-                # Take note of remaining work from this symlink
-                symlinks[cur] = len(parts)
-                if target.startswith(sep):
-                    # Symlink points to absolute path
-                    resolved = ""
-                parts.extend(split(target)[::-1])
-        return resolved or sep
+        seen = {}
+        def _resolve(path, rest):
+            if rest.startswith(sep):
+                path = ''
+
+            for name in rest.split(sep):
+                if not name or name == '.':
+                    # current dir
+                    continue
+                if name == '..':
+                    # parent dir
+                    path, _, _ = path.rpartition(sep)
+                    continue
+                newpath = path + sep + name
+                if newpath in seen:
+                    # Already seen this path
+                    path = seen[newpath]
+                    if path is not None:
+                        # use cached value
+                        continue
+                    # The symlink is not resolved, so we must have a symlink loop.
+                    raise RuntimeError("Symlink loop from %r" % newpath)
+                # Resolve the symbolic link
+                try:
+                    target = accessor.readlink(newpath)
+                except OSError as e:
+                    if e.errno != EINVAL:
+                        raise
+                    # Not a symlink
+                    path = newpath
+                else:
+                    seen[newpath] = None # not resolved symlink
+                    path = _resolve(path, target)
+                    seen[newpath] = path # resolved symlink
+
+            return path
+        # NOTE: according to POSIX, getcwd() cannot contain path components
+        # which are symlinks.
+        base = '' if path.is_absolute() else os.getcwd()
+        return _resolve(base, str(path)) or sep
 
     def is_reserved(self, parts):
         return False
diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py
index a8d740f..0ad77e0 100755
--- a/Lib/test/test_pathlib.py
+++ b/Lib/test/test_pathlib.py
@@ -1620,6 +1620,59 @@ class _BasePathTest(object):
         # 'bin'
         self.assertIs(p.parts[2], q.parts[3])
 
+    def _check_complex_symlinks(self, link0_target):
+        # Test solving a non-looping chain of symlinks (issue #19887)
+        P = self.cls(BASE)
+        self.dirlink(os.path.join('link0', 'link0'), join('link1'))
+        self.dirlink(os.path.join('link1', 'link1'), join('link2'))
+        self.dirlink(os.path.join('link2', 'link2'), join('link3'))
+        self.dirlink(link0_target, join('link0'))
+
+        # Resolve absolute paths
+        p = (P / 'link0').resolve()
+        self.assertEqual(p, P)
+        self.assertEqual(str(p), BASE)
+        p = (P / 'link1').resolve()
+        self.assertEqual(p, P)
+        self.assertEqual(str(p), BASE)
+        p = (P / 'link2').resolve()
+        self.assertEqual(p, P)
+        self.assertEqual(str(p), BASE)
+        p = (P / 'link3').resolve()
+        self.assertEqual(p, P)
+        self.assertEqual(str(p), BASE)
+
+        # Resolve relative paths
+        old_path = os.getcwd()
+        os.chdir(BASE)
+        try:
+            p = self.cls('link0').resolve()
+            self.assertEqual(p, P)
+            self.assertEqual(str(p), BASE)
+            p = self.cls('link1').resolve()
+            self.assertEqual(p, P)
+            self.assertEqual(str(p), BASE)
+            p = self.cls('link2').resolve()
+            self.assertEqual(p, P)
+            self.assertEqual(str(p), BASE)
+            p = self.cls('link3').resolve()
+            self.assertEqual(p, P)
+            self.assertEqual(str(p), BASE)
+        finally:
+            os.chdir(old_path)
+
+    @with_symlinks
+    def test_complex_symlinks_absolute(self):
+        self._check_complex_symlinks(BASE)
+
+    @with_symlinks
+    def test_complex_symlinks_relative(self):
+        self._check_complex_symlinks('.')
+
+    @with_symlinks
+    def test_complex_symlinks_relative_dot_dot(self):
+        self._check_complex_symlinks(os.path.join('dirA', '..'))
+
 
 class PathTest(_BasePathTest, unittest.TestCase):
     cls = pathlib.Path
diff --git a/Misc/NEWS b/Misc/NEWS
index 628c2fe..52b0a3d 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -44,6 +44,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #19887: Improve the Path.resolve() algorithm to support certain
+  symlink chains.
+
 - Issue #19912: Fixed numerous bugs in ntpath.splitunc().
 
 - Issue #19911: ntpath.splitdrive() now correctly processes the 'İ' character