Simplify and document cycle handling in os.path.realpath

zooba · Aug 12, 2019 · Aug 12, 2019 · Aug 12, 2019 · Aug 13, 2019 · Aug 13, 2019
commit 28c9bb47ebe71f6e7ba3effab61836d900a49d7c
diff --git a/Doc/library/os.path.rst b/Doc/library/os.path.rst
@@ -353,6 +353,10 @@ the :mod:`glob` module.)
    links encountered in the path (if they are supported by the operating
    system).
 
+   .. note::
+      When symbolic link cycles occur, the returned path will be one member of
+      the cycle, but no guarantee is made about which member that will be.
+
    .. versionchanged:: 3.6
       Accepts a :term:`path-like object`.
 

diff --git a/Lib/ntpath.py b/Lib/ntpath.py
@@ -520,29 +520,25 @@ def abspath(path):
             return _abspath_fallback(path)
 
 try:
-    from nt import _getfinalpathname
+    from nt import _getfinalpathname, readlink as _nt_readlink
 except ImportError:
     # realpath is a no-op on systems without _getfinalpathname support.
     realpath = abspath
 else:
-    def _readlink_deep(path, seen=None, *, _readlink=os.readlink):
+    def _readlink_deep(path, seen=None):
         if seen is None:
             seen = set()
 
-        while True:
+        while normcase(path) not in seen:
             seen.add(normcase(path))
             try:
-                next_path = _readlink(path)
-                # Break out _before_ traversing the cycle
-                if normcase(next_path) in seen:
-                    break
-                path = next_path
+                path = _nt_readlink(path)
             except OSError as ex:
                 # Stop on file (2) or directory (3) not found, or
                 # paths that are not reparse points (4390)
-                if ex.winerror not in (2, 3, 4390):
-                    raise
-                break
+                if ex.winerror in (2, 3, 4390):
+                    break
+                raise
             except ValueError:
                 # Stop on reparse points that are not symlinks
                 break
@@ -567,8 +563,7 @@ def _getfinalpathname_nonstrict(path):
                     raise
                 path, name = split(path)
                 if path and not name:
-                    tail = path + tail
-                    break
+                    return abspath(path + tail)
                 tail = join(name, tail) if tail else name
         return abspath(tail)
 

diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py
@@ -320,12 +320,16 @@ def test_realpath_symlink_loops(self):
         os.symlink(ABSTFN, ABSTFN)
         self.assertEqual(ntpath.realpath(ABSTFN), P + ABSTFN)
 
+        # cycles are non-deterministic as to which path is returned, but
+        # it will always be the fully resolved path of one member of the cycle
         os.symlink(ABSTFN + "1", ABSTFN + "2")
         os.symlink(ABSTFN + "2", ABSTFN + "1")
-        self.assertEqual(ntpath.realpath(ABSTFN + "1"), P + ABSTFN + "2")
-        self.assertEqual(ntpath.realpath(ABSTFN + "2"), P + ABSTFN + "1")
+        expected = (P + ABSTFN + "1", P + ABSTFN + "2")
+        self.assertIn(ntpath.realpath(ABSTFN + "1"), expected)
+        self.assertIn(ntpath.realpath(ABSTFN + "2"), expected)
 
-        self.assertEqual(ntpath.realpath(ABSTFN + "1\\x"), P + ABSTFN + "2\\x")
+        self.assertIn(ntpath.realpath(ABSTFN + "1\\x"),
+                      (ntpath.join(r, "x") for r in expected))
         self.assertEqual(ntpath.realpath(ABSTFN + "1\\.."),
                          ntpath.dirname(ABSTFN))
         self.assertEqual(ntpath.realpath(ABSTFN + "1\\..\\x"),
@@ -334,9 +338,9 @@ def test_realpath_symlink_loops(self):
         self.assertEqual(ntpath.realpath(ABSTFN + "1\\..\\"
                                          + ntpath.basename(ABSTFN) + "y"),
                          P + ABSTFN + "x")
-        self.assertEqual(ntpath.realpath(ABSTFN + "1\\..\\"
-                                         + ntpath.basename(ABSTFN) + "1"),
-                         P + ABSTFN + "1")
+        self.assertIn(ntpath.realpath(ABSTFN + "1\\..\\"
+                                      + ntpath.basename(ABSTFN) + "1"),
+                      expected)
 
         os.symlink(ntpath.basename(ABSTFN) + "a\\b", ABSTFN + "a")
         self.assertEqual(ntpath.realpath(ABSTFN + "a"), P + ABSTFN + "a")