bug #990669: os.path.realpath() will resolve symlinks before normalizing the

path, as normalizing the path may alter the meaning of the path if it contains
symlinks.

Also add tests for infinite symlink loops and parent symlinks that need to be
resolved.

diff --git a/Lib/posixpath.py b/Lib/posixpath.py
index 557f8f21cbfcdc4631dd870802d179b315d4dd97..345a74268cbc243049b4e7e56a8aec6588cbfb7e 100644 (file)
--- a/Lib/posixpath.py
+++ b/Lib/posixpath.py
@@ -400,9 +400,11 @@ def abspath(path):
 def realpath(filename):
     """Return the canonical path of the specified filename, eliminating any
 symbolic links encountered in the path."""
-    filename = abspath(filename)
-
-    bits = ['/'] + filename.split('/')[1:]
+    if isabs(filename):
+        bits = ['/'] + filename.split('/')[1:]
+    else:
+        bits = filename.split('/')
+        
     for i in range(2, len(bits)+1):
         component = join(*bits[0:i])
         # Resolve symbolic links.
@@ -410,13 +412,13 @@ symbolic links encountered in the path."""
             resolved = _resolve_link(component)
             if resolved is None:
                 # Infinite loop -- return original component + rest of the path
-                return join(*([component] + bits[i:]))
+                return abspath(join(*([component] + bits[i:])))
             else:
                 newpath = join(*([resolved] + bits[i:]))
-                return realpath(newpath)
-
-    return filename
+                return realpath(newpath)        
 
+    return abspath(filename)
+    
 
 def _resolve_link(path):
     """Internal helper function.  Takes a path and follows symlinks

diff --git a/Lib/test/test_posixpath.py b/Lib/test/test_posixpath.py
index fcf2ef56c6e69dcfdf6ddd5842fdbd8fa357a8f9..75cd50b5872e6f33641b1059aac66365753b8733 100644 (file)
--- a/Lib/test/test_posixpath.py
+++ b/Lib/test/test_posixpath.py
@@ -2,6 +2,12 @@ import unittest
 from test import test_support
 
 import posixpath, os
+from posixpath import realpath, abspath, join, dirname, basename
+
+# An absolute path to a temporary filename for testing. We can't rely on TESTFN
+# being an absolute path, so we need this.
+
+ABSTFN = abspath(test_support.TESTFN)
 
 class PosixPathTest(unittest.TestCase):
 
@@ -389,9 +395,96 @@ class PosixPathTest(unittest.TestCase):
         self.assertRaises(TypeError, posixpath.abspath)
 
     def test_realpath(self):
-        self.assert_("foo" in posixpath.realpath("foo"))
-
+        self.assert_("foo" in realpath("foo"))
         self.assertRaises(TypeError, posixpath.realpath)
+          
+    if hasattr(os, "symlink"):
+        def test_realpath_basic(self):
+            # Basic operation.
+            try:
+                os.symlink(ABSTFN+"1", ABSTFN)
+                self.assertEqual(realpath(ABSTFN), ABSTFN+"1")
+            finally:
+                self.safe_remove(ABSTFN)
+        
+        def test_realpath_symlink_loops(self):
+            # Bug #930024, return the path unchanged if we get into an infinite
+            # symlink loop.
+            try:
+                old_path = abspath('.')
+                os.symlink(ABSTFN, ABSTFN)
+                self.assertEqual(realpath(ABSTFN), ABSTFN)
+
+                os.symlink(ABSTFN+"1", ABSTFN+"2")
+                os.symlink(ABSTFN+"2", ABSTFN+"1")
+                self.assertEqual(realpath(ABSTFN+"1"), ABSTFN+"1")
+                self.assertEqual(realpath(ABSTFN+"2"), ABSTFN+"2")
+
+                # Test using relative path as well.
+                os.chdir(dirname(ABSTFN))
+                self.assertEqual(realpath(basename(ABSTFN)), ABSTFN)
+            finally:
+                os.chdir(old_path)
+                self.safe_remove(ABSTFN)
+                self.safe_remove(ABSTFN+"1")
+                self.safe_remove(ABSTFN+"2")
+
+        def test_realpath_resolve_parents(self):                        
+            # We also need to resolve any symlinks in the parents of a relative
+            # path passed to realpath. E.g.: current working directory is
+            # /usr/doc with 'doc' being a symlink to /usr/share/doc. We call
+            # realpath("a"). This should return /usr/share/doc/a/.
+            try:
+                old_path = abspath('.')
+                os.mkdir(ABSTFN)
+                os.mkdir(ABSTFN + "/y")
+                os.symlink(ABSTFN + "/y", ABSTFN + "/k")
+
+                os.chdir(ABSTFN + "/k")
+                self.assertEqual(realpath("a"), ABSTFN + "/y/a")
+            finally:
+                os.chdir(old_path)
+                self.safe_remove(ABSTFN + "/k")
+                self.safe_rmdir(ABSTFN + "/y")
+                self.safe_rmdir(ABSTFN)
+
+        def test_realpath_resolve_before_normalizing(self):
+            # Bug #990669: Symbolic links should be resolved before we
+            # normalize the path. E.g.: if we have directories 'a', 'k' and 'y'
+            # in the following hierarchy:
+            # a/k/y
+            #
+            # and a symbolic link 'link-y' pointing to 'y' in directory 'a',
+            # then realpath("link-y/..") should return 'k', not 'a'.
+            try:
+                old_path = abspath('.')
+                os.mkdir(ABSTFN)
+                os.mkdir(ABSTFN + "/k")
+                os.mkdir(ABSTFN + "/k/y")
+                os.symlink(ABSTFN + "/k/y", ABSTFN + "/link-y")
+
+                # Absolute path.
+                self.assertEqual(realpath(ABSTFN + "/link-y/.."), ABSTFN + "/k")
+                # Relative path.
+                os.chdir(dirname(ABSTFN))
+                self.assertEqual(realpath(basename(ABSTFN) + "/link-y/.."), ABSTFN + "/k")
+            finally:
+                os.chdir(old_path)
+                self.safe_remove(ABSTFN + "/link-y")
+                self.safe_rmdir(ABSTFN + "/k/y")
+                self.safe_rmdir(ABSTFN + "/k")
+                self.safe_rmdir(ABSTFN)
+
+        # Convenience functions for removing temporary files.
+        def pass_os_error(self, func, filename):
+            try: func(filename)
+            except OSError: pass
+
+        def safe_remove(self, filename):
+            self.pass_os_error(os.remove, filename)
+
+        def safe_rmdir(self, dirname):
+            self.pass_os_error(os.rmdir, dirname)
 
 def test_main():
     test_support.run_unittest(PosixPathTest)

diff --git a/Misc/NEWS b/Misc/NEWS
index 04ab6dd6a4728a223e5db85b8f0fb61a0365483e..ab83648035018c964a11a51f41d14a36f630d554 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -41,6 +41,10 @@ Extension modules
 Library
 -------
 
+- bug #990669: os.path.realpath() will resolve symlinks before normalizing the
+  path, as normalizing the path may alter the meaning of the path if it
+  contains symlinks.
+
 - bug #851123: shutil.copyfile will raise an exception when trying to copy a
   file onto a link to itself. Thanks Gregory Ball.
 
@@ -77,7 +81,7 @@ C API
 Documentation
 -------------
 
-- bug 990669: os.path.normpath may alter the meaning of a path if it contains
+- bug #990669: os.path.normpath may alter the meaning of a path if it contains
 symbolic links. This has been documented in a comment since 1992, but is now in
 the library reference as well.