classification
Title: Path.resolve() fails on complex symlinks
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: pitrou Nosy List: Bluebird75, pitrou, python-dev, serhiy.storchaka, vajrasky
Priority: high Keywords: patch

Created on 2013-12-04 20:46 by serhiy.storchaka, last changed 2014-01-10 11:12 by Bluebird75. This issue is now closed.

Files
File name Uploaded Description Edit
pathlib_resolve_test.py serhiy.storchaka, 2013-12-04 20:46
pathlib_resolve.patch serhiy.storchaka, 2013-12-05 13:05 review
pathlib_resolve_2.patch serhiy.storchaka, 2013-12-05 15:37 review
pathlib_resolve_3.patch serhiy.storchaka, 2013-12-07 14:52 review
pathlib_resolve_4.patch pitrou, 2013-12-16 00:50 review
Messages (20)
msg205245 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-12-04 20:46
Here is a script which success with os.path.realpath(), but fails with pathlib.Path.resolve(). The `readlink -f` also success with these examples.
msg205262 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-04 23:17
This is https://bitbucket.org/pitrou/pathlib/issue/9/pathresolve-fails-on-complex-symlinks

As noted there:

"""It's a ENAMETOOLONG error when calling readlink(), because there are many "." components. This is really an edge case caused by the specific test setup, I don't think you'd have that many "." components in real-world use."""
msg205294 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-12-05 13:05
Path.resolve() also fails when last link is absolute.

mkdir testdir
ln -s 0/0 testdir/1
ln -s 1/1 testdir/2
ln -s "$(readlink -f testdir)" testdir/0

Path('testdir/2').resolve() fails:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/serhiy/py/cpython/Lib/pathlib.py", line 1017, in resolve
    s = self._flavour.resolve(self)
  File "/home/serhiy/py/cpython/Lib/pathlib.py", line 273, in resolve
    raise RuntimeError("Symlink loop from %r" % cur)
RuntimeError: Symlink loop from '/home/serhiy/py/cpython/testdir/0'

Here is a patch which implements an algorithm similar to the algorithm used in posixpath.realpath().
msg205303 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-12-05 14:49
Whoa, Serhiy, your patch won't fly on Windows Vista.

For starter, the patch does not use target_is_directory=True option in os.symlink. It needs that option when creating symlink to the directory on Windows Vista.

The maximum depth that pathlib can do on Windows Vista is 4 (Remember, this line: "for depth in 0, 1, 2, 3, 10, 100"?).

More than that:
OSError: [WinError 1921] The name of the file cannot be resolved by the system:
'C:\\Users\\vajrasky\\Code\\cpython\\@test_4220_tmp\\testdir\\link5'

Not sure whether the bug is in pathlib.py or _getfinalpathname (from nt module).
msg205307 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-12-05 15:37
Thank you Vajrasky. Here is a patch with fixed tests.
msg205311 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-12-05 16:39
It works. Just a coding nitpick, instead of hardcoding os.symlink(src, dst, target_is_directory=True) in the test, you can use helper function in the test itself, self.dirlink.
msg205420 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-07 00:15
I would prefer if you made the test simpler. The current setup makes it difficult to understand and reproduce with the command line.
msg205442 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-12-07 08:04
How the test can be simpler? It is already simple.

You ca use pathlib_resolve_test.py but replace `os.symlink('.', 'testdir/0')` by `os.symlink(os.path.abspath('testdir'), 'testdir/0')`. Or use following shell commands:

mkdir testdir
for i in $(seq 100); do ln -s $((i-1))/$((i-1)) testdir/$i; done
ln -s "$(pwd)/testdir" testdir/0

./python -c "import pathlib; print(pathlib.Path('testdir/100').resolve())"
msg205455 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-07 14:15
> How the test can be simpler? It is already simple.

I mean, don't use loops but a simple test setup as in test_resolve_dot.
I am not interested in the pathologic "100 symlinks" case, just the case where the symlink is absolute.
msg205456 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-12-07 14:38
And don't forget to use self.dirlink instead of os.symlink(src, dst, target_is_directory=True).
msg205458 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-12-07 14:52
Here is a patch with unrolled loops.
msg206268 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-16 00:26
> Here is a patch with unrolled loops.

I suppose that was some kind of joke, but what I meant was that we don't need to test with 100 levels of symlinks. 2 or 3 are enough...
msg206269 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-16 00:50
Here is a patch with refactored tests. Vajrasky, do you want to test under Windows?
msg206310 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-12-16 13:43
> I suppose that was some kind of joke, but what I meant was that we don't need to test with 100 levels of symlinks. 2 or 3 are enough...

Yes, sorry for this joke. Your tests LGTM, but why you repeat similar code 3-4 times instead using loops?
msg206313 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-12-16 13:59
The patch passed on Windows Vista.
msg206345 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-16 18:49
> why you repeat similar code 3-4 times instead using loops?

For no real reason :) I'll try with a loop.
msg206346 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-16 18:53
Ah, I remember. Using subtests would make it more annoying to backport to 2.7.
msg206348 - (view) Author: Roundup Robot (python-dev) Date: 2013-12-16 18:57
New changeset 12a52186b4fd by Antoine Pitrou in branch 'default':
Issue #19887: Improve the Path.resolve() algorithm to support certain symlink chains.
http://hg.python.org/cpython/rev/12a52186b4fd
msg206349 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-16 18:58
Thanks a lot for the patch!
msg207849 - (view) Author: Philippe Fremy (Bluebird75) Date: 2014-01-10 11:12
Hi,

This precise set of tests fails on Windows 7 on a NTFS partition (on revision  c0b0e7aef360+ tip ), see below.

The problem  is probably minor (drive letter case). I won't be able to develop a fix myself, but I'll be happy to test one.

cheers,

Philippe

======================================================================
FAIL: test_complex_symlinks_absolute (test.test_pathlib.PathTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\Users\Morgane\Documents\000\Dev\CPython\cpython\lib\test\test_pathlib.py", line 1724, in test_complex_symlinks_absolute
    self._check_complex_symlinks(BASE)
  File "c:\Users\Morgane\Documents\000\Dev\CPython\cpython\lib\test\test_pathlib.py", line 1692, in _check_complex_symlinks
    self.assertEqual(str(p), BASE)
AssertionError: 'C:\\Users\\Morgane\\Documents\\000\\Dev\\[53 chars]_tmp' != 'c:\\Users\\Morgane\\Documents\\000\\Dev\\[53 chars]_tmp'
- C:\Users\Morgane\Documents\000\Dev\CPython\cpython\build\test_python_6060\@test_6060_tmp
? ^
+ c:\Users\Morgane\Documents\000\Dev\CPython\cpython\build\test_python_6060\@test_6060_tmp
? ^


======================================================================
FAIL: test_complex_symlinks_relative (test.test_pathlib.PathTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\Users\Morgane\Documents\000\Dev\CPython\cpython\lib\test\test_pathlib.py", line 1728, in test_complex_symlinks_relative
    self._check_complex_symlinks('.')
  File "c:\Users\Morgane\Documents\000\Dev\CPython\cpython\lib\test\test_pathlib.py", line 1692, in _check_complex_symlinks
    self.assertEqual(str(p), BASE)
AssertionError: 'C:\\Users\\Morgane\\Documents\\000\\Dev\\[53 chars]_tmp' != 'c:\\Users\\Morgane\\Documents\\000\\Dev\\[53 chars]_tmp'
- C:\Users\Morgane\Documents\000\Dev\CPython\cpython\build\test_python_6060\@test_6060_tmp
? ^
+ c:\Users\Morgane\Documents\000\Dev\CPython\cpython\build\test_python_6060\@test_6060_tmp
? ^


======================================================================
FAIL: test_complex_symlinks_relative_dot_dot (test.test_pathlib.PathTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\Users\Morgane\Documents\000\Dev\CPython\cpython\lib\test\test_pathlib.py", line 1732, in test_complex_symlinks_relative_dot_dot
    self._check_complex_symlinks(os.path.join('dirA', '..'))
  File "c:\Users\Morgane\Documents\000\Dev\CPython\cpython\lib\test\test_pathlib.py", line 1692, in _check_complex_symlinks
    self.assertEqual(str(p), BASE)
AssertionError: 'C:\\Users\\Morgane\\Documents\\000\\Dev\\[53 chars]_tmp' != 'c:\\Users\\Morgane\\Documents\\000\\Dev\\[53 chars]_tmp'
- C:\Users\Morgane\Documents\000\Dev\CPython\cpython\build\test_python_6060\@test_6060_tmp
? ^
+ c:\Users\Morgane\Documents\000\Dev\CPython\cpython\build\test_python_6060\@test_6060_tmp
? ^


======================================================================
FAIL: test_complex_symlinks_absolute (test.test_pathlib.WindowsPathTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\Users\Morgane\Documents\000\Dev\CPython\cpython\lib\test\test_pathlib.py", line 1724, in test_complex_symlinks_absolute
    self._check_complex_symlinks(BASE)
  File "c:\Users\Morgane\Documents\000\Dev\CPython\cpython\lib\test\test_pathlib.py", line 1692, in _check_complex_symlinks
    self.assertEqual(str(p), BASE)
AssertionError: 'C:\\Users\\Morgane\\Documents\\000\\Dev\\[53 chars]_tmp' != 'c:\\Users\\Morgane\\Documents\\000\\Dev\\[53 chars]_tmp'
- C:\Users\Morgane\Documents\000\Dev\CPython\cpython\build\test_python_6060\@test_6060_tmp
? ^
+ c:\Users\Morgane\Documents\000\Dev\CPython\cpython\build\test_python_6060\@test_6060_tmp
? ^


======================================================================
FAIL: test_complex_symlinks_relative (test.test_pathlib.WindowsPathTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\Users\Morgane\Documents\000\Dev\CPython\cpython\lib\test\test_pathlib.py", line 1728, in test_complex_symlinks_relative
    self._check_complex_symlinks('.')
  File "c:\Users\Morgane\Documents\000\Dev\CPython\cpython\lib\test\test_pathlib.py", line 1692, in _check_complex_symlinks
    self.assertEqual(str(p), BASE)
AssertionError: 'C:\\Users\\Morgane\\Documents\\000\\Dev\\[53 chars]_tmp' != 'c:\\Users\\Morgane\\Documents\\000\\Dev\\[53 chars]_tmp'
- C:\Users\Morgane\Documents\000\Dev\CPython\cpython\build\test_python_6060\@test_6060_tmp
? ^
+ c:\Users\Morgane\Documents\000\Dev\CPython\cpython\build\test_python_6060\@test_6060_tmp
? ^


======================================================================
FAIL: test_complex_symlinks_relative_dot_dot (test.test_pathlib.WindowsPathTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\Users\Morgane\Documents\000\Dev\CPython\cpython\lib\test\test_pathlib.py", line 1732, in test_complex_symlinks_relative_dot_dot
    self._check_complex_symlinks(os.path.join('dirA', '..'))
  File "c:\Users\Morgane\Documents\000\Dev\CPython\cpython\lib\test\test_pathlib.py", line 1692, in _check_complex_symlinks
    self.assertEqual(str(p), BASE)
AssertionError: 'C:\\Users\\Morgane\\Documents\\000\\Dev\\[53 chars]_tmp' != 'c:\\Users\\Morgane\\Documents\\000\\Dev\\[53 chars]_tmp'
- C:\Users\Morgane\Documents\000\Dev\CPython\cpython\build\test_python_6060\@test_6060_tmp
? ^
+ c:\Users\Morgane\Documents\000\Dev\CPython\cpython\build\test_python_6060\@test_6060_tmp
? ^


----------------------------------------------------------------------
Ran 335 tests in 1.455s

FAILED (failures=6, skipped=93)
test test_pathlib failed
1 test failed:
    test_pathlib
History
Date User Action Args
2014-01-10 11:12:48Bluebird75setnosy: + Bluebird75
messages: + msg207849
2013-12-16 18:58:29pitrousetstatus: open -> closed
resolution: fixed
messages: + msg206349

stage: patch review -> resolved
2013-12-16 18:57:50python-devsetnosy: + python-dev
messages: + msg206348
2013-12-16 18:53:21pitrousetmessages: + msg206346
2013-12-16 18:49:01pitrousetmessages: + msg206345
2013-12-16 13:59:24vajraskysetmessages: + msg206313
2013-12-16 13:43:46serhiy.storchakasetmessages: + msg206310
2013-12-16 00:50:15pitrousetfiles: + pathlib_resolve_4.patch

messages: + msg206269
2013-12-16 00:26:29pitrousetmessages: + msg206268
2013-12-11 22:30:47serhiy.storchakasetassignee: pitrou
2013-12-11 09:03:43serhiy.storchakalinkissue19717 dependencies
2013-12-07 14:52:02serhiy.storchakasetfiles: + pathlib_resolve_3.patch

messages: + msg205458
2013-12-07 14:38:22vajraskysetmessages: + msg205456
2013-12-07 14:15:11pitrousetmessages: + msg205455
2013-12-07 08:10:25serhiy.storchakasettitle: Path.resolve() ENAMETOOLONG on pathologic symlinks -> Path.resolve() fails on complex symlinks
2013-12-07 08:04:15serhiy.storchakasetmessages: + msg205442
2013-12-07 00:15:15pitrousetmessages: + msg205420
2013-12-05 16:39:02vajraskysetmessages: + msg205311
2013-12-05 15:37:56serhiy.storchakasetfiles: + pathlib_resolve_2.patch

messages: + msg205307
2013-12-05 14:49:26vajraskysetnosy: + vajrasky
messages: + msg205303
2013-12-05 13:05:41serhiy.storchakasetfiles: + pathlib_resolve.patch
priority: low -> high
messages: + msg205294

keywords: + patch
stage: patch review
2013-12-04 23:17:37pitrousetpriority: normal -> low

messages: + msg205262
title: Path.resolve() fails on deep symlinks -> Path.resolve() ENAMETOOLONG on pathologic symlinks
2013-12-04 20:46:45serhiy.storchakacreate