classification
Title: ntpath.realpath() does not fully resolve relative paths
Type: behavior Stage: resolved
Components: Windows Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: steve.dower Nosy List: eryksun, miss-islington, paul.moore, steve.dower, tim.golden, vstinner, zach.ware
Priority: normal Keywords: patch

Created on 2019-10-11 22:27 by steve.dower, last changed 2019-11-18 17:32 by steve.dower. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 16723 merged steve.dower, 2019-10-11 23:01
PR 16783 merged miss-islington, 2019-10-14 15:43
PR 16967 merged steve.dower, 2019-10-28 17:33
PR 17174 merged steve.dower, 2019-11-15 17:54
PR 17184 merged steve.dower, 2019-11-15 23:39
PR 17186 merged miss-islington, 2019-11-16 00:04
Messages (18)
msg354509 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-10-11 22:27
Because the abspath() call is deferred until the end of the resolution process, if the current working directory is not already a real path, it will not be fully resolved.

By passing it through abspath() at the start of the function, we ensure that the correct path is resolved.

This will help resolve some inconsistencies in the test suite when tests are launched with a short filename (FILENA~1) in the working directory (though some other fixes are probably needed to help here).
msg354511 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-10-11 22:42
Updating change_cwd() in Lib/test/support/__init__.py to call chdir(realpath(path)) also fixes the test issues I was facing, but the realpath() implementation needs fixing.
msg354512 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-10-11 23:03
PR 16723 is just for the test fix.

The realpath() fix should get tests that chdir (without change_cwd()) into a symlink and then realpath() on a relative path. Today, that should fail to resolve the sections in the CWD, and with the fix it should resolve the entire path.
msg354638 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-10-14 15:42
New changeset d83fc2702951f56a7339aa95d62414ed6e0fb40d by Steve Dower in branch 'master':
bpo-38453: Resolve test directories before chdir to them (GH-16723)
https://github.com/python/cpython/commit/d83fc2702951f56a7339aa95d62414ed6e0fb40d
msg354640 - (view) Author: miss-islington (miss-islington) Date: 2019-10-14 16:01
New changeset aa909b6b1242b4969b20bb0250ac386f9b4120d7 by Miss Islington (bot) in branch '3.8':
bpo-38453: Resolve test directories before chdir to them (GH-16723)
https://github.com/python/cpython/commit/aa909b6b1242b4969b20bb0250ac386f9b4120d7
msg355574 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-10-28 17:35
PR 16967 should fix the relative path resolution property, by joining with cwd if the original path is unprefixed and not absolute, and then joining with any symlink directories if their link is not absolute.
msg355575 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-10-28 17:44
I also had to special-case "realpath('nul')" to maintain the behaviour that we deliberately maintained in another bug, and I think isabs() should always return true for "\\?\" prefixed paths (given that join() will promote a "\\?\" prefixed path to the root).
msg355680 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-10-29 21:33
The discussion on the PR has raised one scenario where our algorithm conflicts with what we can reasonably emulate from the IO manager (which is itself not entirely self-consistent):

If you call ntpath.realpath(r"C:\spam\eggs") where "spam" is a symlink/junction to "D:\foo\baz" and "D:\foo\baz\eggs" is a relative symlink to "..\bar" and "D:\foo\bar" does not exist, we will incorrectly return "C:\bar".

Note that if "D:\foo\bar" *does* exist, we will return the correct result. And if "C:\spam" is *not* a symlink/junction, we will return the correct result.

Essentially, we fail in the case where ntpath.exists(X) fails and realpath(dirname(X)) != dirname(X) and readlink(X).startswith("..\\") is True.

Considering it's taken months of patient explanation from Eryk Sun for me to understand this, and nobody has ever complained about it, I'm prepared to call it a known limitation.
msg356548 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-11-13 19:31
My latest push to PR 16967 is a significant change to what ntpath.realpath will return for broken symlinks, but I think it's the right change.

Basically, if the OS fully resolves the path, great! We'll return that (and handle \\?\ stripping)

If the OS doesn't, we'll assume it's a broken link and try and read the link target from the path. Most of the time, we'll get "not a link", but if it succeeds then we'll return here. For absolute links, we'll return it. For relative symlinks, we'll join with the parent directory. For relative anything-else, we'll return the path to the link itself.

If we can't read a link (most likely), then we'll work up the path doing the same thing. Once we successfully get further than the original path, we'll join the unprocessed tail on and return without attempting to resolve anything more (assuming that the OS would have done it earlier if it was possible).

So if you look at the changed test_ntpath.py you'll see the different results (the test_shutil.py changes are mainly to avoid long/short name comparison failures when realpath() fixes them, by putting the test files in the actual test directory instead of global TEMP).
msg356701 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-11-15 17:49
New changeset abde52cd8e31830bfc06c5803221faae6172104a by Steve Dower in branch 'master':
bpo-38453: Ensure ntpath.realpath correctly resolves relative paths (GH-16967)
https://github.com/python/cpython/commit/abde52cd8e31830bfc06c5803221faae6172104a
msg356702 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-11-15 17:49
I think the PR as it stands is a net improvement over where we currently are, so I'll merge it. We can make more tweaks as necessary.
msg356722 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-15 22:30
AMD64 Windows8.1 Non-Debug 3.x is grumpy:
https://buildbot.python.org/all/#/builders/12/builds/3459

======================================================================
FAIL: test_realpath_cwd (test.test_ntpath.TestNtpath)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\buildarea\3.x.ware-win81-release.nondebug\build\lib\test\test_ntpath.py", line 424, in test_realpath_cwd
    self.assertPathEqual(test_file_long, ntpath.realpath(test_file_short))
  File "D:\buildarea\3.x.ware-win81-release.nondebug\build\lib\test\test_ntpath.py", line 62, in assertPathEqual
    self.assertEqual(path1, path2)
AssertionError: 'D:\\[88 chars]worker_4476\\@test_4476_tmp\\MyVeryLongDirectoryName\\file.txt' != 'D:\\[88 chars]worker_4476\\@test_4476_tmp\\MYVERY~1\\file.txt'
- D:\buildarea\3.x.ware-win81-release.nondebug\build\build\test_python_4088\test_python_worker_4476\@test_4476_tmp\MyVeryLongDirectoryName\file.txt
?                                                                                                                   ^ ^^^^^^^^^^^^^^^^^^^^
+ D:\buildarea\3.x.ware-win81-release.nondebug\build\build\test_python_4088\test_python_worker_4476\@test_4476_tmp\MYVERY~1\file.txt
?                                                                                                                   ^ ^^^^^
msg356723 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-15 22:32
Grumpy AMD64 Windows10 3.x:
https://buildbot.python.org/all/#/builders/3/builds/3788

======================================================================
FAIL: test_realpath_cwd (test.test_ntpath.TestNtpath)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\buildarea\3.x.bolen-windows10\build\lib\test\test_ntpath.py", line 424, in test_realpath_cwd
    self.assertPathEqual(test_file_long, ntpath.realpath(test_file_short))
  File "D:\buildarea\3.x.bolen-windows10\build\lib\test\test_ntpath.py", line 62, in assertPathEqual
    self.assertEqual(path1, path2)
AssertionError: 'D:\\[76 chars]worker_6956\\@test_6956_tmp\\MyVeryLongDirectoryName\\file.txt' != 'D:\\[76 chars]worker_6956\\@test_6956_tmp\\MYVERY~1\\file.txt'
- D:\buildarea\3.x.bolen-windows10\build\build\test_python_6236\test_python_worker_6956\@test_6956_tmp\MyVeryLongDirectoryName\file.txt
?                                                                                                       ^ ^^^^^^^^^^^^^^^^^^^^
+ D:\buildarea\3.x.bolen-windows10\build\build\test_python_6236\test_python_worker_6956\@test_6956_tmp\MYVERY~1\file.txt
?                                                                                                       ^ ^^^^^

pythoninfo:

windows.RtlAreLongPathsEnabled: False
msg356726 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-11-15 23:25
New changeset 66c0f01f98fd6db0a4b1e789b9db9c7247a24ba9 by Steve Dower in branch '3.8':
bpo-38453: Ensure ntpath.realpath correctly resolves relative paths (GH-16967)
https://github.com/python/cpython/commit/66c0f01f98fd6db0a4b1e789b9db9c7247a24ba9
msg356727 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-11-15 23:26
Thanks, Victor! That looks different from the one I was originally fixing (even though the long/short path mismatch is there), so I'll take a look.
msg356733 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-11-16 00:04
New changeset 7c6130c8c36c941255365e5414c956fc919b8629 by Steve Dower in branch 'master':
bpo-38453: Ensure correct short path is obtained for test (GH-17184)
https://github.com/python/cpython/commit/7c6130c8c36c941255365e5414c956fc919b8629
msg356739 - (view) Author: miss-islington (miss-islington) Date: 2019-11-16 00:21
New changeset 6f602fbd3568ed7bebadfa3c7d3de40a271216f9 by Miss Islington (bot) in branch '3.8':
bpo-38453: Ensure correct short path is obtained for test (GH-17184)
https://github.com/python/cpython/commit/6f602fbd3568ed7bebadfa3c7d3de40a271216f9
msg356888 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-11-18 17:32
Closing this as I believe it's done, but happy to reopen if we find another edge case (or start a new issue).
History
Date User Action Args
2019-11-18 17:32:26steve.dowersetstatus: open -> closed
messages: + msg356888

assignee: steve.dower
resolution: fixed
stage: patch review -> resolved
2019-11-16 00:21:31miss-islingtonsetmessages: + msg356739
2019-11-16 00:04:29miss-islingtonsetpull_requests: + pull_request16693
2019-11-16 00:04:03steve.dowersetmessages: + msg356733
2019-11-15 23:39:52steve.dowersetpull_requests: + pull_request16691
2019-11-15 23:26:32steve.dowersetmessages: + msg356727
2019-11-15 23:25:16steve.dowersetmessages: + msg356726
2019-11-15 22:32:17vstinnersetmessages: + msg356723
2019-11-15 22:30:50vstinnersetnosy: + vstinner
messages: + msg356722
2019-11-15 17:54:41steve.dowersetpull_requests: + pull_request16682
2019-11-15 17:49:52steve.dowersetmessages: + msg356702
2019-11-15 17:49:27steve.dowersetmessages: + msg356701
2019-11-13 19:32:00steve.dowersetmessages: + msg356548
2019-10-29 21:33:26steve.dowersetmessages: + msg355680
2019-10-28 17:44:02steve.dowersetmessages: + msg355575
2019-10-28 17:35:38steve.dowersetmessages: + msg355574
2019-10-28 17:33:52steve.dowersetpull_requests: + pull_request16494
2019-10-14 16:38:59steve.dowersettitle: ntpath.realpath() should make absolute path earlier -> ntpath.realpath() does not fully resolve relative paths
2019-10-14 16:01:32miss-islingtonsetnosy: + miss-islington
messages: + msg354640
2019-10-14 15:43:02miss-islingtonsetpull_requests: + pull_request16343
2019-10-14 15:42:25steve.dowersetmessages: + msg354638
2019-10-11 23:03:21steve.dowersetmessages: + msg354512
2019-10-11 23:01:47steve.dowersetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request16301
2019-10-11 22:42:30steve.dowersetmessages: + msg354511
2019-10-11 22:27:20steve.dowercreate