From b65f06f56e2b73f831ca4f95d33b6b9b3d2d5af6 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 Aug 2019 12:44:15 -0700 Subject: [PATCH 01/12] [WIP] bpo-9949: Enable symlink traversal for ntpath.realpath --- Doc/library/os.path.rst | 5 +- Lib/ntpath.py | 73 +++++++++++++++----- Lib/test/test_ntpath.py | 149 ++++++++++++++++++++++++++++++++++++++++ Lib/test/test_os.py | 5 +- 4 files changed, 208 insertions(+), 24 deletions(-) diff --git a/Doc/library/os.path.rst b/Doc/library/os.path.rst index a673b81278ea74..5f11d896915a0a 100644 --- a/Doc/library/os.path.rst +++ b/Doc/library/os.path.rst @@ -350,11 +350,14 @@ the :mod:`glob` module.) .. function:: realpath(path) Return the canonical path of the specified filename, eliminating any symbolic - links encountered in the path (if they are supported by the operating system). + links encountered in the path (if they are supported by the operating + system). .. versionchanged:: 3.6 Accepts a :term:`path-like object`. + .. versionchanged:: 3.8 + Symbolic links are now resolved on Windows. .. function:: relpath(path, start=os.curdir) diff --git a/Lib/ntpath.py b/Lib/ntpath.py index f3cfabf0238ef5..ef03c7880f558e 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -519,8 +519,60 @@ def abspath(path): except (OSError, ValueError): return _abspath_fallback(path) -# realpath is a no-op on systems without islink support -realpath = abspath +try: + from nt import _getfinalpathname +except ImportError: + # realpath is a no-op on systems without _getfinalpathname support. + realpath = abspath +else: + def _getfinalpathname_nonstrict(path): + # Fast-path is to find the target directly + try: + return _getfinalpathname(path) + except OSError: + pass + + # Non-strict algorithm is to find as much of the target directory + # as we can and join the rest. + path, tail = split(path) + while path: + try: + return join(_getfinalpathname(path), tail) + except OSError: + path, name = split(path) + if path and not name: + tail = path + tail + break + tail = join(name, tail) + return tail + + def realpath(path): + path = os.fspath(path) + if isinstance(path, bytes): + prefix = b'\\\\?\\' + unc_prefix = b'\\\\?\\UNC\\' + new_unc_prefix = b'\\\\' + cwd = os.getcwdb() + else: + prefix = '\\\\?\\' + unc_prefix = '\\\\?\\UNC\\' + new_unc_prefix = '\\\\' + cwd = os.getcwd() + had_prefix = path.startswith(prefix) + path = _getfinalpathname_nonstrict(path) + # The path returned by _getfinalpathname will always start with \\?\ - + # strip off that prefix unless it was already provided on the original + # path. + if not had_prefix and path.startswith(prefix): + # For UNC paths, the prefix will actually be \\?\UNC\ + # Handle that case as well. + if path.startswith(unc_prefix): + path = new_unc_prefix + path[len(unc_prefix):] + else: + path = path[len(prefix):] + return path + + # Win9x family and earlier have no Unicode filename support. supports_unicode_filenames = (hasattr(sys, "getwindowsversion") and sys.getwindowsversion()[3] >= 2) @@ -633,23 +685,6 @@ def commonpath(paths): raise -# determine if two files are in fact the same file -try: - # GetFinalPathNameByHandle is available starting with Windows 6.0. - # Windows XP and non-Windows OS'es will mock _getfinalpathname. - if sys.getwindowsversion()[:2] >= (6, 0): - from nt import _getfinalpathname - else: - raise ImportError -except (AttributeError, ImportError): - # On Windows XP and earlier, two files are the same if their absolute - # pathnames are the same. - # Non-Windows operating systems fake this method with an XP - # approximation. - def _getfinalpathname(f): - return normcase(abspath(f)) - - try: # The genericpath.isdir implementation uses os.stat and checks the mode # attribute to tell whether or not the path is a directory. diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index 92d85ecbc4fcfe..97a154792ab922 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -7,6 +7,7 @@ from test import support, test_genericpath from tempfile import TemporaryFile + try: import nt except ImportError: @@ -14,6 +15,14 @@ # but for those that require it we import here. nt = None +try: + ntpath._getfinalpathname +except AttributeError: + HAVE_GETFINALPATHNAME = False +else: + HAVE_GETFINALPATHNAME = True + + def tester(fn, wantResult): fn = fn.replace("\\", "\\\\") gotResult = eval(fn) @@ -194,6 +203,146 @@ def test_normpath(self): tester("ntpath.normpath('\\\\.\\NUL')", r'\\.\NUL') tester("ntpath.normpath('\\\\?\\D:/XY\\Z')", r'\\?\D:/XY\Z') + def test_realpath_curdir(self): + tester("ntpath.realpath('.')", os.getcwd()) + tester("ntpath.realpath('./.')", os.getcwd()) + tester("ntpath.realpath('/'.join(['.'] * 100))", os.getcwd()) + tester("ntpath.realpath('.\\.')", os.getcwd()) + tester("ntpath.realpath('\\'.join(['.'] * 100))", os.getcwd()) + + def test_realpath_pardir(self): + tester("ntpath.realpath('..')", os.path.dirname(os.getcwd())) + tester("ntpath.realpath('../..')", + os.path.dirname(os.path.dirname(os.getcwd()))) + tester("ntpath.realpath('/'.join(['..'] * 50))", + os.path.splitdrive(os.getcwd())[0] + '\\') + tester("ntpath.realpath('..\\..')", + os.path.dirname(os.path.dirname(os.getcwd()))) + tester("ntpath.realpath('\\'.join(['..'] * 50))", + os.path.splitdrive(os.getcwd())[0] + '\\') + + @support.skip_unless_symlink + @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') + def test_realpath_basic(self): + ABSTFN = ntpath.abspath(support.TESTFN) + self.addCleanup(support.unlink, ABSTFN) + self.addCleanup(support.unlink, ABSTFN + "1") + + os.symlink(ABSTFN, ABSTFN + "1") + self.assertEqual(ntpath.realpath(ABSTFN + "1"), ABSTFN) + self.assertEqual(ntpath.realpath(os.fsencode(ABSTFN + "1")), + os.fsencode(ABSTFN)) + + @support.skip_unless_symlink + @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') + def test_realpath_relative(self): + ABSTFN = ntpath.abspath(support.TESTFN) + self.addCleanup(support.unlink, ABSTFN) + self.addCleanup(support.unlink, ABSTFN + "1") + + os.symlink(ABSTFN, os.path.relpath(ABSTFN + "1")) + self.assertEqual(ntpath.realpath(ABSTFN), ABSTFN + "1") + + @support.skip_unless_symlink + @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') + def test_realpath_broken_symlinks(self): + ABSTFN = ntpath.abspath(support.TESTFN) + self.addCleanup(support.rmtree, ABSTFN) + + os.mkdir(ABSTFN) + with support.change_cwd(ABSTFN): + os.mkdir("subdir") + os.chdir("subdir") + os.symlink(".", "recursive") + os.symlink("..", "parent") + os.chdir("..") + os.symlink(".", "self") + os.symlink("missing", "broken") + os.symlink(r"broken\bar", "broken1") + os.symlink(r"self\self\broken", "broken2") + os.symlink(r"subdir\parent\subdir\parent\broken", "broken3") + os.symlink(ABSTFN + r"\broken", "broken4") + os.symlink(r"recursive\..\broken", "broken5") + + self.assertEqual(ntpath.realpath("broken"), + ABSTFN + r"\missing") + self.assertEqual(ntpath.realpath(r"broken\foo"), + ABSTFN + r"\missing\foo") + self.assertEqual(ntpath.realpath(r"broken1"), + ABSTFN + r"\missing\bar") + self.assertEqual(ntpath.realpath(r"broken1\baz"), + ABSTFN + r"\missing\bar\baz") + self.assertEqual(ntpath.realpath("broken2"), + ABSTFN + r"\missing") + self.assertEqual(ntpath.realpath("broken3"), + ABSTFN + r"\missing") + self.assertEqual(ntpath.realpath("broken4"), + ABSTFN + r"\missing") + self.assertEqual(ntpath.realpath("broken5"), + ABSTFN + r"\missing") + + self.assertEqual(ntpath.realpath(b"broken"), + os.fsencode(ABSTFN + r"\missing")) + self.assertEqual(ntpath.realpath(rb"broken\foo"), + os.fsencode(ABSTFN + r"\missing\foo")) + self.assertEqual(ntpath.realpath(rb"broken1"), + os.fsencode(ABSTFN + r"\missing\bar")) + self.assertEqual(ntpath.realpath(rb"broken1\baz"), + os.fsencode(ABSTFN + r"\missing\bar\baz")) + self.assertEqual(ntpath.realpath(b"broken2"), + os.fsencode(ABSTFN + r"\missing")) + self.assertEqual(ntpath.realpath(rb"broken3"), + os.fsencode(ABSTFN + r"\missing")) + self.assertEqual(ntpath.realpath(b"broken4"), + os.fsencode(ABSTFN + r"\missing")) + self.assertEqual(ntpath.realpath(b"broken5"), + os.fsencode(ABSTFN + r"\missing")) + + @support.skip_unless_symlink + @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') + def test_realpath_symlink_loops(self): + # Bug #930024, return the path unchanged if we get into an infinite + # symlink loop. + ABSTFN = ntpath.abspath(support.TESTFN) + self.addCleanup(support.unlink, ABSTFN) + self.addCleanup(support.unlink, ABSTFN + "1") + self.addCleanup(support.unlink, ABSTFN + "2") + self.addCleanup(support.unlink, ABSTFN + "y") + self.addCleanup(support.unlink, ABSTFN + "c") + self.addCleanup(support.unlink, ABSTFN + "a") + + os.symlink(ABSTFN, ABSTFN) + self.assertEqual(ntpath.realpath(ABSTFN), ABSTFN) + + os.symlink(ABSTFN + "1", ABSTFN + "2") + os.symlink(ABSTFN + "2", ABSTFN + "1") + self.assertEqual(ntpath.realpath(ABSTFN + "1"), ABSTFN + "1") + self.assertEqual(ntpath.realpath(ABSTFN + "2"), ABSTFN + "2") + + self.assertEqual(ntpath.realpath(ABSTFN + "1\\x"), ABSTFN + "1\\x") + self.assertEqual(ntpath.realpath(ABSTFN + "1\\.."), + ntpath.dirname(ABSTFN)) + self.assertEqual(ntpath.realpath(ABSTFN + "1\\..\\x"), + ntpath.dirname(ABSTFN) + "\\x") + os.symlink(ABSTFN + "x", ABSTFN + "y") + self.assertEqual(ntpath.realpath(ABSTFN + "1\\..\\" + + ntpath.basename(ABSTFN) + "y"), + ABSTFN + "x") + self.assertEqual(ntpath.realpath(ABSTFN + "1\\..\\" + + ntpath.basename(ABSTFN) + "1"), + ABSTFN + "1") + + os.symlink(ntpath.basename(ABSTFN) + "a\\b", ABSTFN + "a") + self.assertEqual(ntpath.realpath(ABSTFN + "a"), ABSTFN + "a\\b") + + os.symlink("..\\" + os.path.basename(os.path.dirname(ABSTFN)) + + "\\" + os.path.basename(ABSTFN) + "c", ABSTFN + "c") + self.assertEqual(ntpath.realpath(ABSTFN + "c"), ABSTFN + "c") + + with support.change_cwd('..'): + # Test using relative path as well. + self.assertEqual(ntpath.realpath(ntpath.basename(ABSTFN)), ABSTFN) + def test_expandvars(self): with support.EnvironmentVarGuard() as env: env.clear() diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index b2cd4cca5f21fe..15fd65b55149e7 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -3358,10 +3358,7 @@ def test_oserror_filename(self): if hasattr(os, "lchmod"): funcs.append((self.filenames, os.lchmod, 0o777)) if hasattr(os, "readlink"): - if sys.platform == "win32": - funcs.append((self.unicode_filenames, os.readlink,)) - else: - funcs.append((self.filenames, os.readlink,)) + funcs.append((self.filenames, os.readlink,)) for filenames, func, *func_args in funcs: From df54868872b1d4f6fc65e203f3de59f3a1d9d480 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 Aug 2019 13:40:33 -0700 Subject: [PATCH 02/12] Fixes link resolution and test file creation --- Lib/ntpath.py | 42 ++++++++++++++----- Lib/test/test_ntpath.py | 15 +++---- .../2019-08-14-13-40-15.bpo-9949.zW45Ks.rst | 1 + 3 files changed, 41 insertions(+), 17 deletions(-) create mode 100644 Misc/NEWS.d/next/Windows/2019-08-14-13-40-15.bpo-9949.zW45Ks.rst diff --git a/Lib/ntpath.py b/Lib/ntpath.py index ef03c7880f558e..5e40deef4094c8 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -525,26 +525,48 @@ def abspath(path): # realpath is a no-op on systems without _getfinalpathname support. realpath = abspath else: + def _readlink_deep(path, seen=None, *, _readlink=os.readlink): + if seen is None: + seen = set() + + while normcase(path) not in seen: + seen.add(normcase(path)) + try: + path = _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 + except ValueError: + # Stop on reparse points that are not symlinks + break + return path + def _getfinalpathname_nonstrict(path): - # Fast-path is to find the target directly - try: - return _getfinalpathname(path) - except OSError: - pass + # Allow file (2) or directory (3) not found, or symlinks that + # cannot be followed (1921) + allowed_winerror = 2, 3, 1921 # Non-strict algorithm is to find as much of the target directory # as we can and join the rest. - path, tail = split(path) + tail = '' + seen = set() while path: try: - return join(_getfinalpathname(path), tail) - except OSError: + path = _readlink_deep(path, seen) + path = _getfinalpathname(path) + return join(path, tail) if tail else path + except OSError as ex: + if ex.winerror not in allowed_winerror: + raise path, name = split(path) if path and not name: tail = path + tail break - tail = join(name, tail) - return tail + tail = join(name, tail) if tail else name + return abspath(tail) def realpath(path): path = os.fspath(path) diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index 97a154792ab922..8b5689c9f0dc6b 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -225,6 +225,7 @@ def test_realpath_pardir(self): @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') def test_realpath_basic(self): ABSTFN = ntpath.abspath(support.TESTFN) + open(ABSTFN, "wb").close() self.addCleanup(support.unlink, ABSTFN) self.addCleanup(support.unlink, ABSTFN + "1") @@ -237,19 +238,20 @@ def test_realpath_basic(self): @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') def test_realpath_relative(self): ABSTFN = ntpath.abspath(support.TESTFN) + open(ABSTFN, "wb").close() self.addCleanup(support.unlink, ABSTFN) self.addCleanup(support.unlink, ABSTFN + "1") os.symlink(ABSTFN, os.path.relpath(ABSTFN + "1")) - self.assertEqual(ntpath.realpath(ABSTFN), ABSTFN + "1") + self.assertEqual(ntpath.realpath(ABSTFN + "1"), ABSTFN) @support.skip_unless_symlink @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') def test_realpath_broken_symlinks(self): ABSTFN = ntpath.abspath(support.TESTFN) + os.mkdir(ABSTFN) self.addCleanup(support.rmtree, ABSTFN) - os.mkdir(ABSTFN) with support.change_cwd(ABSTFN): os.mkdir("subdir") os.chdir("subdir") @@ -330,18 +332,17 @@ def test_realpath_symlink_loops(self): ABSTFN + "x") self.assertEqual(ntpath.realpath(ABSTFN + "1\\..\\" + ntpath.basename(ABSTFN) + "1"), - ABSTFN + "1") + ABSTFN + "2") os.symlink(ntpath.basename(ABSTFN) + "a\\b", ABSTFN + "a") - self.assertEqual(ntpath.realpath(ABSTFN + "a"), ABSTFN + "a\\b") + self.assertEqual(ntpath.realpath(ABSTFN + "a"), ABSTFN + "a") os.symlink("..\\" + os.path.basename(os.path.dirname(ABSTFN)) + "\\" + os.path.basename(ABSTFN) + "c", ABSTFN + "c") self.assertEqual(ntpath.realpath(ABSTFN + "c"), ABSTFN + "c") - with support.change_cwd('..'): - # Test using relative path as well. - self.assertEqual(ntpath.realpath(ntpath.basename(ABSTFN)), ABSTFN) + # Test using relative path as well. + self.assertEqual(ntpath.realpath(ntpath.basename(ABSTFN)), ABSTFN) def test_expandvars(self): with support.EnvironmentVarGuard() as env: diff --git a/Misc/NEWS.d/next/Windows/2019-08-14-13-40-15.bpo-9949.zW45Ks.rst b/Misc/NEWS.d/next/Windows/2019-08-14-13-40-15.bpo-9949.zW45Ks.rst new file mode 100644 index 00000000000000..e42169a927c718 --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2019-08-14-13-40-15.bpo-9949.zW45Ks.rst @@ -0,0 +1 @@ +Enable support for following symlinks in :func:`os.realpath`. From f82129bfe48724ff27cc10714b91af044138b3af Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 Aug 2019 14:20:24 -0700 Subject: [PATCH 03/12] Fix x-plat handling in test_ntpath and additional error code --- Lib/ntpath.py | 6 +++--- Lib/test/test_ntpath.py | 22 ++++++++++++---------- Lib/test/test_shutil.py | 6 +----- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/Lib/ntpath.py b/Lib/ntpath.py index 5e40deef4094c8..315013171dec70 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -545,9 +545,9 @@ def _readlink_deep(path, seen=None, *, _readlink=os.readlink): return path def _getfinalpathname_nonstrict(path): - # Allow file (2) or directory (3) not found, or symlinks that - # cannot be followed (1921) - allowed_winerror = 2, 3, 1921 + # Allow file (2) or directory (3) not found, invalid syntax (123), + # and symlinks that cannot be followed (1921) + allowed_winerror = 2, 3, 123, 1921 # Non-strict algorithm is to find as much of the target directory # as we can and join the rest. diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index 8b5689c9f0dc6b..8959f365d4445b 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -204,22 +204,24 @@ def test_normpath(self): tester("ntpath.normpath('\\\\?\\D:/XY\\Z')", r'\\?\D:/XY\Z') def test_realpath_curdir(self): - tester("ntpath.realpath('.')", os.getcwd()) - tester("ntpath.realpath('./.')", os.getcwd()) - tester("ntpath.realpath('/'.join(['.'] * 100))", os.getcwd()) - tester("ntpath.realpath('.\\.')", os.getcwd()) - tester("ntpath.realpath('\\'.join(['.'] * 100))", os.getcwd()) + expected = ntpath.normpath(os.getcwd()) + tester("ntpath.realpath('.')", expected) + tester("ntpath.realpath('./.')", expected) + tester("ntpath.realpath('/'.join(['.'] * 100))", expected) + tester("ntpath.realpath('.\\.')", expected) + tester("ntpath.realpath('\\'.join(['.'] * 100))", expected) def test_realpath_pardir(self): - tester("ntpath.realpath('..')", os.path.dirname(os.getcwd())) + expected = ntpath.normpath(os.getcwd()) + tester("ntpath.realpath('..')", os.path.dirname(expected)) tester("ntpath.realpath('../..')", - os.path.dirname(os.path.dirname(os.getcwd()))) + os.path.dirname(os.path.dirname(expected))) tester("ntpath.realpath('/'.join(['..'] * 50))", - os.path.splitdrive(os.getcwd())[0] + '\\') + os.path.splitdrive(expected)[0] + '\\') tester("ntpath.realpath('..\\..')", - os.path.dirname(os.path.dirname(os.getcwd()))) + os.path.dirname(os.path.dirname(expected))) tester("ntpath.realpath('\\'.join(['..'] * 50))", - os.path.splitdrive(os.getcwd())[0] + '\\') + os.path.splitdrive(expected)[0] + '\\') @support.skip_unless_symlink @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index e209607f22c145..88dc4d9e195ebe 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1871,11 +1871,7 @@ def test_move_dangling_symlink(self): dst_link = os.path.join(self.dst_dir, 'quux') shutil.move(dst, dst_link) self.assertTrue(os.path.islink(dst_link)) - # On Windows, os.path.realpath does not follow symlinks (issue #9949) - if os.name == 'nt': - self.assertEqual(os.path.realpath(src), os.readlink(dst_link)) - else: - self.assertEqual(os.path.realpath(src), os.path.realpath(dst_link)) + self.assertEqual(os.path.realpath(src), os.path.realpath(dst_link)) @support.skip_unless_symlink @mock_rename From 8ee40462e65298d4453d8976889ced298c3f5174 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 Aug 2019 14:40:19 -0700 Subject: [PATCH 04/12] Fix use of os.path in ntpath tests --- Lib/test/test_ntpath.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index 8959f365d4445b..47a68c828b8220 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -215,13 +215,13 @@ def test_realpath_pardir(self): expected = ntpath.normpath(os.getcwd()) tester("ntpath.realpath('..')", os.path.dirname(expected)) tester("ntpath.realpath('../..')", - os.path.dirname(os.path.dirname(expected))) + os.path.dirname(ntpath.dirname(expected))) tester("ntpath.realpath('/'.join(['..'] * 50))", os.path.splitdrive(expected)[0] + '\\') tester("ntpath.realpath('..\\..')", - os.path.dirname(os.path.dirname(expected))) + os.path.dirname(ntpath.dirname(expected))) tester("ntpath.realpath('\\'.join(['..'] * 50))", - os.path.splitdrive(expected)[0] + '\\') + ntpath.splitdrive(expected)[0] + '\\') @support.skip_unless_symlink @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') From b93c9e8af40d3b0d1f8c665ac4e7965737fa8e2a Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 Aug 2019 14:41:11 -0700 Subject: [PATCH 05/12] Fix additional uses of os.path --- Lib/test/test_ntpath.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index 47a68c828b8220..70116f2d75fe0d 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -213,13 +213,13 @@ def test_realpath_curdir(self): def test_realpath_pardir(self): expected = ntpath.normpath(os.getcwd()) - tester("ntpath.realpath('..')", os.path.dirname(expected)) + tester("ntpath.realpath('..')", ntpath.dirname(expected)) tester("ntpath.realpath('../..')", - os.path.dirname(ntpath.dirname(expected))) + ntpath.dirname(ntpath.dirname(expected))) tester("ntpath.realpath('/'.join(['..'] * 50))", - os.path.splitdrive(expected)[0] + '\\') + ntpath.splitdrive(expected)[0] + '\\') tester("ntpath.realpath('..\\..')", - os.path.dirname(ntpath.dirname(expected))) + ntpath.dirname(ntpath.dirname(expected))) tester("ntpath.realpath('\\'.join(['..'] * 50))", ntpath.splitdrive(expected)[0] + '\\') @@ -244,7 +244,7 @@ def test_realpath_relative(self): self.addCleanup(support.unlink, ABSTFN) self.addCleanup(support.unlink, ABSTFN + "1") - os.symlink(ABSTFN, os.path.relpath(ABSTFN + "1")) + os.symlink(ABSTFN, ntpath.relpath(ABSTFN + "1")) self.assertEqual(ntpath.realpath(ABSTFN + "1"), ABSTFN) @support.skip_unless_symlink @@ -339,8 +339,8 @@ def test_realpath_symlink_loops(self): os.symlink(ntpath.basename(ABSTFN) + "a\\b", ABSTFN + "a") self.assertEqual(ntpath.realpath(ABSTFN + "a"), ABSTFN + "a") - os.symlink("..\\" + os.path.basename(os.path.dirname(ABSTFN)) - + "\\" + os.path.basename(ABSTFN) + "c", ABSTFN + "c") + os.symlink("..\\" + ntpath.basename(ntpath.dirname(ABSTFN)) + + "\\" + ntpath.basename(ABSTFN) + "c", ABSTFN + "c") self.assertEqual(ntpath.realpath(ABSTFN + "c"), ABSTFN + "c") # Test using relative path as well. @@ -440,11 +440,11 @@ def test_abspath(self): def test_relpath(self): tester('ntpath.relpath("a")', 'a') - tester('ntpath.relpath(os.path.abspath("a"))', 'a') + tester('ntpath.relpath(ntpath.abspath("a"))', 'a') tester('ntpath.relpath("a/b")', 'a\\b') tester('ntpath.relpath("../a/b")', '..\\a\\b') with support.temp_cwd(support.TESTFN) as cwd_dir: - currentdir = os.path.basename(cwd_dir) + currentdir = ntpath.basename(cwd_dir) tester('ntpath.relpath("a", "../b")', '..\\'+currentdir+'\\a') tester('ntpath.relpath("a/b", "../c")', '..\\'+currentdir+'\\a\\b') tester('ntpath.relpath("a", "b/c")', '..\\..\\a') @@ -569,7 +569,7 @@ def test_ismount(self): # locations below cannot then refer to mount points # drive, path = ntpath.splitdrive(sys.executable) - with support.change_cwd(os.path.dirname(sys.executable)): + with support.change_cwd(ntpath.dirname(sys.executable)): self.assertFalse(ntpath.ismount(drive.lower())) self.assertFalse(ntpath.ismount(drive.upper())) From 5f9816f398c584bd2051720f715fb82d3361c33c Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 Aug 2019 15:23:37 -0700 Subject: [PATCH 06/12] Validate that the path prefix is unnecessary before removing --- Lib/ntpath.py | 10 ++++++-- Lib/test/test_ntpath.py | 56 +++++++++++++++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/Lib/ntpath.py b/Lib/ntpath.py index 315013171dec70..0657bd729e7aa5 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -589,9 +589,15 @@ def realpath(path): # For UNC paths, the prefix will actually be \\?\UNC\ # Handle that case as well. if path.startswith(unc_prefix): - path = new_unc_prefix + path[len(unc_prefix):] + spath = new_unc_prefix + path[len(unc_prefix):] else: - path = path[len(prefix):] + spath = path[len(prefix):] + # Ensure that the non-prefixed path resolves to the same path + try: + if _getfinalpathname(spath) == _getfinalpathname(path): + path = spath + except OSError as ex: + pass return path diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index 70116f2d75fe0d..99622f3217ff06 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -315,36 +315,72 @@ def test_realpath_symlink_loops(self): self.addCleanup(support.unlink, ABSTFN + "c") self.addCleanup(support.unlink, ABSTFN + "a") + P = "\\\\?\\" + os.symlink(ABSTFN, ABSTFN) - self.assertEqual(ntpath.realpath(ABSTFN), ABSTFN) + self.assertEqual(ntpath.realpath(ABSTFN), P + ABSTFN) os.symlink(ABSTFN + "1", ABSTFN + "2") os.symlink(ABSTFN + "2", ABSTFN + "1") - self.assertEqual(ntpath.realpath(ABSTFN + "1"), ABSTFN + "1") - self.assertEqual(ntpath.realpath(ABSTFN + "2"), ABSTFN + "2") + self.assertEqual(ntpath.realpath(ABSTFN + "1"), P + ABSTFN + "1") + self.assertEqual(ntpath.realpath(ABSTFN + "2"), P + ABSTFN + "2") - self.assertEqual(ntpath.realpath(ABSTFN + "1\\x"), ABSTFN + "1\\x") + self.assertEqual(ntpath.realpath(ABSTFN + "1\\x"), P + ABSTFN + "1\\x") self.assertEqual(ntpath.realpath(ABSTFN + "1\\.."), ntpath.dirname(ABSTFN)) self.assertEqual(ntpath.realpath(ABSTFN + "1\\..\\x"), - ntpath.dirname(ABSTFN) + "\\x") + ntpath.dirname(P + ABSTFN) + "\\x") os.symlink(ABSTFN + "x", ABSTFN + "y") self.assertEqual(ntpath.realpath(ABSTFN + "1\\..\\" + ntpath.basename(ABSTFN) + "y"), - ABSTFN + "x") + P + ABSTFN + "x") self.assertEqual(ntpath.realpath(ABSTFN + "1\\..\\" + ntpath.basename(ABSTFN) + "1"), - ABSTFN + "2") + P + ABSTFN + "2") os.symlink(ntpath.basename(ABSTFN) + "a\\b", ABSTFN + "a") - self.assertEqual(ntpath.realpath(ABSTFN + "a"), ABSTFN + "a") + self.assertEqual(ntpath.realpath(ABSTFN + "a"), P + ABSTFN + "a") os.symlink("..\\" + ntpath.basename(ntpath.dirname(ABSTFN)) + "\\" + ntpath.basename(ABSTFN) + "c", ABSTFN + "c") - self.assertEqual(ntpath.realpath(ABSTFN + "c"), ABSTFN + "c") + self.assertEqual(ntpath.realpath(ABSTFN + "c"), P + ABSTFN + "c") # Test using relative path as well. - self.assertEqual(ntpath.realpath(ntpath.basename(ABSTFN)), ABSTFN) + self.assertEqual(ntpath.realpath(ntpath.basename(ABSTFN)), P + ABSTFN) + + @support.skip_unless_symlink + @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') + def test_realpath_symlink_prefix(self): + ABSTFN = ntpath.abspath(support.TESTFN) + self.addCleanup(support.unlink, ABSTFN + "3") + self.addCleanup(support.unlink, "\\\\?\\" + ABSTFN + "3.") + self.addCleanup(support.unlink, ABSTFN + "3link") + self.addCleanup(support.unlink, ABSTFN + "3.link") + + with open(ABSTFN + "3", "wb") as f: + f.write(b'0') + os.symlink(ABSTFN + "3", ABSTFN + "3link") + + with open("\\\\?\\" + ABSTFN + "3.", "wb") as f: + f.write(b'1') + os.symlink("\\\\?\\" + ABSTFN + "3.", ABSTFN + "3.link") + + self.assertEqual(ntpath.realpath(ABSTFN + "3link"), + ABSTFN + "3") + self.assertEqual(ntpath.realpath(ABSTFN + "3.link"), + "\\\\?\\" + ABSTFN + "3.") + + # Resolved paths should be usable to open target files + with open(ntpath.realpath(ABSTFN + "3link"), "rb") as f: + self.assertEqual(f.read(), b'0') + with open(ntpath.realpath(ABSTFN + "3.link"), "rb") as f: + self.assertEqual(f.read(), b'1') + + # When the prefix is included, it is not stripped + self.assertEqual(ntpath.realpath("\\\\?\\" + ABSTFN + "3link"), + "\\\\?\\" + ABSTFN + "3") + self.assertEqual(ntpath.realpath("\\\\?\\" + ABSTFN + "3.link"), + "\\\\?\\" + ABSTFN + "3.") def test_expandvars(self): with support.EnvironmentVarGuard() as env: From d1d072630001ca7ed8c6c70eefbcd36d115627b7 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 Aug 2019 15:28:25 -0700 Subject: [PATCH 07/12] Don't need second call to _getfinalpathname --- Lib/ntpath.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/ntpath.py b/Lib/ntpath.py index 0657bd729e7aa5..e508cfb8e7e0d7 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -594,7 +594,7 @@ def realpath(path): spath = path[len(prefix):] # Ensure that the non-prefixed path resolves to the same path try: - if _getfinalpathname(spath) == _getfinalpathname(path): + if _getfinalpathname(spath) == path: path = spath except OSError as ex: pass From 9bc4192fb7a64151e4cf7fc45a1ee74966cf437a Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 Aug 2019 16:49:17 -0700 Subject: [PATCH 08/12] Fix test discovery mocks --- Lib/unittest/test/test_discovery.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Lib/unittest/test/test_discovery.py b/Lib/unittest/test/test_discovery.py index 204043b493b5d2..16e081e1fb76ec 100644 --- a/Lib/unittest/test/test_discovery.py +++ b/Lib/unittest/test/test_discovery.py @@ -723,11 +723,13 @@ class Module(object): original_listdir = os.listdir original_isfile = os.path.isfile original_isdir = os.path.isdir + original_realpath = os.path.realpath def cleanup(): os.listdir = original_listdir os.path.isfile = original_isfile os.path.isdir = original_isdir + os.path.realpath = original_realpath del sys.modules['foo'] if full_path in sys.path: sys.path.remove(full_path) @@ -742,6 +744,10 @@ def isdir(_): os.listdir = listdir os.path.isfile = isfile os.path.isdir = isdir + if os.name == 'nt': + # ntpath.realpath may inject path prefixes when failing to + # resolve real files, so we substitute abspath() here instead. + os.path.realpath = os.path.abspath return full_path def test_detect_module_clash(self): From 9dc0608e4e960e9f956a0c84e2c3295ad4b7aaa7 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 15 Aug 2019 16:13:18 -0700 Subject: [PATCH 09/12] Avoid traversing a symlink cycle --- Lib/ntpath.py | 8 ++++++-- Lib/test/test_ntpath.py | 8 ++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/Lib/ntpath.py b/Lib/ntpath.py index e508cfb8e7e0d7..76291f0695e3a2 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -529,10 +529,14 @@ def _readlink_deep(path, seen=None, *, _readlink=os.readlink): if seen is None: seen = set() - while normcase(path) not in seen: + while True: seen.add(normcase(path)) try: - path = _readlink(path) + next_path = _readlink(path) + # Break out _before_ traversing the cycle + if normcase(next_path) in seen: + break + path = next_path except OSError as ex: # Stop on file (2) or directory (3) not found, or # paths that are not reparse points (4390) diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index 99622f3217ff06..c8af194242a743 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -322,10 +322,10 @@ def test_realpath_symlink_loops(self): os.symlink(ABSTFN + "1", ABSTFN + "2") os.symlink(ABSTFN + "2", ABSTFN + "1") - self.assertEqual(ntpath.realpath(ABSTFN + "1"), P + ABSTFN + "1") - self.assertEqual(ntpath.realpath(ABSTFN + "2"), P + ABSTFN + "2") + self.assertEqual(ntpath.realpath(ABSTFN + "1"), P + ABSTFN + "2") + self.assertEqual(ntpath.realpath(ABSTFN + "2"), P + ABSTFN + "1") - self.assertEqual(ntpath.realpath(ABSTFN + "1\\x"), P + ABSTFN + "1\\x") + self.assertEqual(ntpath.realpath(ABSTFN + "1\\x"), P + ABSTFN + "2\\x") self.assertEqual(ntpath.realpath(ABSTFN + "1\\.."), ntpath.dirname(ABSTFN)) self.assertEqual(ntpath.realpath(ABSTFN + "1\\..\\x"), @@ -336,7 +336,7 @@ def test_realpath_symlink_loops(self): P + ABSTFN + "x") self.assertEqual(ntpath.realpath(ABSTFN + "1\\..\\" + ntpath.basename(ABSTFN) + "1"), - P + ABSTFN + "2") + P + ABSTFN + "1") os.symlink(ntpath.basename(ABSTFN) + "a\\b", ABSTFN + "a") self.assertEqual(ntpath.realpath(ABSTFN + "a"), P + ABSTFN + "a") From 28c9bb47ebe71f6e7ba3effab61836d900a49d7c Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 16 Aug 2019 11:31:04 -0700 Subject: [PATCH 10/12] Simplify and document cycle handling in os.path.realpath --- Doc/library/os.path.rst | 4 ++++ Lib/ntpath.py | 21 ++++++++------------- Lib/test/test_ntpath.py | 16 ++++++++++------ 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/Doc/library/os.path.rst b/Doc/library/os.path.rst index 5f11d896915a0a..459f6b972129ff 100644 --- 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 index 76291f0695e3a2..8cc95d8f9f8ab1 100644 --- 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 index c8af194242a743..74dc8c378e2746 100644 --- 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") From ab485a5a1621f3b7ddc64d0fdb3ffcafbdb801cd Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 16 Aug 2019 12:35:43 -0700 Subject: [PATCH 11/12] Update whatsnew --- Doc/whatsnew/3.8.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index e8238251d6ea25..93758e96b57364 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -824,6 +824,9 @@ characters or bytes unrepresentable at the OS level. environment variable and does not use :envvar:`HOME`, which is not normally set for regular user accounts. +:func:`~os.path.realpath` on Windows now resolves reparse points, including +symlinks and directory junctions. + ncurses ------- From 5d560b8a5dc0c2ce661e4f3ee42d59fe4d5cd498 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 21 Aug 2019 13:14:05 -0700 Subject: [PATCH 12/12] Add fast resolution path and minor doc update --- Doc/library/os.path.rst | 3 ++- Lib/ntpath.py | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Doc/library/os.path.rst b/Doc/library/os.path.rst index 459f6b972129ff..640ceecbc78f16 100644 --- a/Doc/library/os.path.rst +++ b/Doc/library/os.path.rst @@ -361,7 +361,8 @@ the :mod:`glob` module.) Accepts a :term:`path-like object`. .. versionchanged:: 3.8 - Symbolic links are now resolved on Windows. + Symbolic links and junctions are now resolved on Windows. + .. function:: relpath(path, start=os.curdir) diff --git a/Lib/ntpath.py b/Lib/ntpath.py index 8cc95d8f9f8ab1..ef4999e1473acb 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -545,6 +545,13 @@ def _readlink_deep(path, seen=None): return path def _getfinalpathname_nonstrict(path): + # Fast path to get the final path name. If this succeeds, there + # is no need to go any further. + try: + return _getfinalpathname(path) + except OSError: + pass + # Allow file (2) or directory (3) not found, invalid syntax (123), # and symlinks that cannot be followed (1921) allowed_winerror = 2, 3, 123, 1921