classification
Title: symlinks incorrectly resolved on POSIX platforms
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.4, Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Arfrever, BreamoreBoy, benjamin.peterson, ezio.melotti, georg.brandl, hynek, larry, o11c, pitrou, python-dev, r.david.murray, serhiy.storchaka, swarecki, tzimmo
Priority: release blocker Keywords: patch

Created on 2009-09-23 05:55 by swarecki, last changed 2013-02-20 20:33 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
issue6975.patch markon, 2009-10-16 21:05 Patch provided for issue 6975 review
issue6975_py3.patch tzimmo, 2012-10-23 11:11 Patch ported to python3 review
posix_realpath.patch serhiy.storchaka, 2012-10-24 19:23 review
posix_realpath_2.patch serhiy.storchaka, 2013-01-10 16:51 review
pathlib_resolve_test.py serhiy.storchaka, 2013-01-14 08:11 Test pathlib for recursive symlinks
Messages (29)
msg93027 - (view) Author: Sylwester Warecki (swarecki) Date: 2009-09-23 05:55
Hi

The behavior of os.realpath function on Linux is different from the one 
presented by the system. 
Although the path (pointing to the linked dir) is correct
and is recognized by os.path.exists once it is resolved
by the realpath() it does not exist - at the end of the 
resolved path you will see the additional parent directory name.
you will see
..... /two/two at the end of the path.

Below is the code that shows the issue.

def linkGen():
  import os
  print os.getcwd()
  test = "./test"
  os.system( "rm -rf " + test  )  
  #os.mkdir( test )
  os.makedirs( test + "/one" )
  os.makedirs( test + "/two" )
  os.symlink( "../two", test + "/two/this_dir" )
  os.symlink( "../two/this_dir/this_dir/this_dir/this_dir/", test + 
"/one/that_dir" )
  print test + "/one/that_dir", "EXISTS?",   
  print os.path.exists( test + "/one/that_dir" )
  print os.path.realpath( test + "/one/that_dir" ), "EXISTS?", 
  print os.path.exists( os.path.realpath( test + "/one/that_dir" ) )
  os.system( "ls -l " + test + "/one/that_dir" )
  # the line above will show that system can see the directory just fine

linkGen()
msg93029 - (view) Author: Sylwester Warecki (swarecki) Date: 2009-09-23 05:56
Hi
I meant of course 
os.path.realpath()
Sylwester
msg94149 - (view) Author: Marco Buccini (markon) Date: 2009-10-16 21:05
I've provided a patch.

I've also added a new test, and it passes.
msg94179 - (view) Author: Marco Buccini (markon) Date: 2009-10-17 15:15
I've just found a similar bug:
http://bugs.python.org/issue1646838

However, does os.path.realpath's behavior have to be as the one
specified by the POSIX standard (
http://www.kernel.org/doc/man-pages/online/pages/man3/realpath.3.html )?
If we wanted to follow the standard, we should break the
retro-compatibility, since we should raise an exception in the case the
path passed as argument doesn't exist.
msg94199 - (view) Author: Sylwester Warecki (swarecki) Date: 2009-10-18 03:00
Marco,

Thanks for looking deeper into it.
I would like to check the solution ASAP.
We have really crazy dir structure which can catch a lot of
the unexpected problems with paths, links, circular links etc.

Should I expect the new version to generate the exception
as you suggested?

Best Regards,
    Sywlester Warecki

Marco Buccini wrote:
> Marco Buccini <marcusbu@gmail.com> added the comment:
>
> I've just found a similar bug:
> http://bugs.python.org/issue1646838
>
> However, does os.path.realpath's behavior have to be as the one
> specified by the POSIX standard (
> http://www.kernel.org/doc/man-pages/online/pages/man3/realpath.3.html )?
> If we wanted to follow the standard, we should break the
> retro-compatibility, since we should raise an exception in the case the
> path passed as argument doesn't exist.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue6975>
> _______________________________________
>
>
msg94209 - (view) Author: Marco Buccini (markon) Date: 2009-10-18 09:07
>Thanks for looking deeper into it.
>I would like to check the solution ASAP.
>We have really crazy dir structure which can catch a lot of
>the unexpected problems with paths, links, circular links etc.

Oh well, so you can see if the patch works correctly on all paths :)
 
>Should I expect the new version to generate the exception
>as you suggested?

 
Unfortunately I'm not God, so I cannot change the "code" of the current
implementation of os.path.realpath (raising an OSError exception)
without breaking any existing code. 
So, consider using the current implementation of os.path.realpath ;)
If you experience other strange behaviors, then open a new bug report.

Bye!
msg111082 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-21 16:10
swarecki or markon, are either of you interested in following up this issue?
msg164111 - (view) Author: Ben Longbons (o11c) Date: 2012-06-26 21:23
After filing a duplicate, issue 15196, I analyzed this:

What happens:
test/one/that_dir
test/one/../two/this_dir/this_dir/this_dir/this_dir
test/two/this_dir/this_dir/this_dir/this_dir
test/two/this_dir/this_dir/this_dir/../two
test/two/this_dir/this_dir/two

What should happen:
test/one/that_dir
test/two/this_dir/this_dir/this_dir/this_dir
test/two/../two/this_dir/this_dir/this_dir
test/two/this_dir/this_dir/this_dir
test/two/../two/this_dir/this_dir
test/two/this_dir/this_dir
test/two/../two/this_dir
test/two/this_dir
test/two/../two
test/two
msg173590 - (view) Author: Kimmo Mustonen (tzimmo) * Date: 2012-10-23 11:11
Tried it out and it seemed to fix the issue.
Ported the patch to python3.
msg173696 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-24 19:23
Previous patches contain redundant code, are not protected from all symlink loops (link -> link/x; dir/link -> ../dir/link) and when a loop is detected the returned result differs from the current one.

The proposed patch solves this issue and some others. Added many new tests.

The test for this issue was simplified:

dir/self -> ../dir
dir/link -> self/self/self
resolve dir/link
msg178613 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-30 20:11
Antoine, how about this mosquito?
msg179467 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-09 16:13
If no one objects I will commit this next week.
msg179469 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2013-01-09 16:16
I'd like to see a review first.  I don't have time to do it myself right now though.
msg179472 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-09 16:27
OK. Perhaps I will prepare similar patches for ntpath and macpath.
msg179479 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2013-01-09 17:21
I will review this first thing tomorrow.
msg179573 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-10 16:51
Patch rewritten using recursion. Non-recursive version is not stack limited (and a little faster), but recursive version is perhaps more understandable. Some comments added.
msg179886 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-01-13 17:51
I don't really understand your algorithm. Why do you need a stack? It should be simply iterative:
- if the symlink is relative, prepend the symlink target to the rest
- if the symlink is absolute, discard the current path and prepend the symlink target to the rest

Here is how pathlib does it:
https://bitbucket.org/pitrou/pathlib/src/67a2524b057f1af5b3cba26370b1353e73cdda16/pathlib.py?at=default#cl-227
msg179920 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-14 08:11
> I don't really understand your algorithm. Why do you need a stack?

Before resolving the symlink we mark the path as unresolved symlink for detecting infinite symlink loops. Before resolving the symlink we mark the path as resolved symlink (and cache the resolved value for speed) for allowing finite symlink recursion.

Here is a test which pathlib fails.
msg179974 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-01-14 20:05
Vis the discussion of x[:0] in the review.  This kind of construct is only unfamiliar because it is new in Python3, and there are not *that* many places that you want to (and can) deal with both bytes and strings using the same code.  But when you can, it is the more-or-less obvious way to write the code.

In this particular case you could also write type(x)().  There are pluses and minuses to both forms, but if any of the rest of the code uses slicing to work around the fact that byte strings index into ints, then I would definitely prefer the slice form.
msg181782 - (view) Author: Roundup Robot (python-dev) Date: 2013-02-10 10:27
New changeset 6ec6dbf787f4 by Serhiy Storchaka in branch '2.7':
Issue #6975: os.path.realpath() now correctly resolves multiple nested symlinks on POSIX platforms.
http://hg.python.org/cpython/rev/6ec6dbf787f4

New changeset c5f4fa02fc86 by Serhiy Storchaka in branch '3.2':
Issue #6975: os.path.realpath() now correctly resolves multiple nested symlinks on POSIX platforms.
http://hg.python.org/cpython/rev/c5f4fa02fc86

New changeset bfe9526606e2 by Serhiy Storchaka in branch '3.3':
Issue #6975: os.path.realpath() now correctly resolves multiple nested symlinks on POSIX platforms.
http://hg.python.org/cpython/rev/bfe9526606e2

New changeset f42cabe6ccb5 by Serhiy Storchaka in branch 'default':
Issue #6975: os.path.realpath() now correctly resolves multiple nested symlinks on POSIX platforms.
http://hg.python.org/cpython/rev/f42cabe6ccb5
msg182296 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * Date: 2013-02-18 04:46
The above revisions have broken handling of arguments with >=2 "..".

Before these revisions:

$ cd /usr/bin
$ python3.2 -c 'import os; print(os.path.realpath(".."))'
/usr
$ python3.2 -c 'import os; print(os.path.realpath("../.."))'
/
$ python3.2 -c 'import os; print(os.path.realpath("../../.."))'
/
$ python3.2 -c 'import os; print(os.path.realpath("../../../.."))'

After these revisions:

$ cd /usr/bin
$ python3.2 -c 'import os; print(os.path.realpath(".."))'
/usr
$ python3.2 -c 'import os; print(os.path.realpath("../.."))'
/usr/bin
$ python3.2 -c 'import os; print(os.path.realpath("../../.."))'
/usr
$ python3.2 -c 'import os; print(os.path.realpath("../../../.."))'
/usr/bin
msg182297 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * Date: 2013-02-18 04:48
The actual output of last command in "Before these revisions:" is: /
msg182309 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-02-18 09:52
Thank you for the report. I'm surprised that the tests are not caught it.
msg182311 - (view) Author: Roundup Robot (python-dev) Date: 2013-02-18 10:24
New changeset 50ed06b3d419 by Serhiy Storchaka in branch '2.7':
Fix posixpath.realpath() for multiple pardirs (fixes issue #6975).
http://hg.python.org/cpython/rev/50ed06b3d419

New changeset cb3fbadb65aa by Serhiy Storchaka in branch '3.2':
Fix posixpath.realpath() for multiple pardirs (fixes issue #6975).
http://hg.python.org/cpython/rev/cb3fbadb65aa

New changeset aad7e68eff0a by Serhiy Storchaka in branch '3.3':
Fix posixpath.realpath() for multiple pardirs (fixes issue #6975).
http://hg.python.org/cpython/rev/aad7e68eff0a

New changeset f99ff3b01fab by Serhiy Storchaka in branch 'default':
Fix posixpath.realpath() for multiple pardirs (fixes issue #6975).
http://hg.python.org/cpython/rev/f99ff3b01fab
msg182315 - (view) Author: Roundup Robot (python-dev) Date: 2013-02-18 11:36
New changeset 3c5517c4fa5d by Serhiy Storchaka in branch '2.7':
Disable posixpath.realpath() tests on Windows (fix for issue #6975).
http://hg.python.org/cpython/rev/3c5517c4fa5d

New changeset 0bbf7cdea551 by Serhiy Storchaka in branch '3.2':
Disable posixpath.realpath() tests on Windows (fix for issue #6975).
http://hg.python.org/cpython/rev/0bbf7cdea551

New changeset 79ea59b394bf by Serhiy Storchaka in branch '3.3':
Disable posixpath.realpath() tests on Windows (fix for issue #6975).
http://hg.python.org/cpython/rev/79ea59b394bf

New changeset aa77f7eb2bf1 by Serhiy Storchaka in branch 'default':
Disable posixpath.realpath() tests on Windows (fix for issue #6975).
http://hg.python.org/cpython/rev/aa77f7eb2bf1
msg182427 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * Date: 2013-02-19 20:51
You could probably test '.\\.' and '..\\..' etc. in these tests on Windows.
msg182537 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-02-20 18:05
I have no access to Windows and can't design Windows tests.
msg182540 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * Date: 2013-02-20 18:36
Use os.path.sep and os.path.sep.encode() instead of hardcoding "/" and b"/".
msg182549 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-02-20 20:33
> Use os.path.sep and os.path.sep.encode() instead of hardcoding "/" and
> b"/".

Some separators will be '\\' (if they are derived from OS functions, i.e. 
getcwd), and some will be '/' (if they are generated by posixpath). I do not 
have the ability to research where there are any. Feel free to fix these tests 
for Windows.
History
Date User Action Args
2013-02-20 20:33:41serhiy.storchakasetmessages: + msg182549
2013-02-20 18:36:20Arfreversetmessages: + msg182540
2013-02-20 18:05:44serhiy.storchakasetmessages: + msg182537
2013-02-19 20:51:21Arfreversetmessages: + msg182427
2013-02-18 11:36:00python-devsetmessages: + msg182315
2013-02-18 11:09:31serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: resolved
2013-02-18 10:24:39python-devsetmessages: + msg182311
2013-02-18 09:52:47serhiy.storchakasetmessages: + msg182309
2013-02-18 04:48:57Arfreversetmessages: + msg182297
2013-02-18 04:46:15Arfreversetstatus: closed -> open
priority: normal -> release blocker


nosy: + Arfrever, benjamin.peterson, larry
messages: + msg182296
resolution: fixed -> (no value)
stage: resolved -> (no value)
2013-02-10 15:21:10serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2013-02-10 10:27:46python-devsetnosy: + python-dev
messages: + msg181782
2013-01-14 21:09:02markonsetnosy: - markon
2013-01-14 20:05:45r.david.murraysetmessages: + msg179974
2013-01-14 08:11:17serhiy.storchakasetfiles: + pathlib_resolve_test.py

messages: + msg179920
2013-01-13 17:51:28pitrousetmessages: + msg179886
2013-01-10 16:51:38serhiy.storchakasetfiles: + posix_realpath_2.patch

messages: + msg179573
2013-01-10 08:16:10hyneksettitle: symlinks incorrectly resolved on Linux -> symlinks incorrectly resolved on POSIX platforms
2013-01-09 17:21:34hyneksetmessages: + msg179479
2013-01-09 16:27:26serhiy.storchakasetmessages: + msg179472
2013-01-09 16:16:43georg.brandlsetnosy: + georg.brandl
messages: + msg179469
2013-01-09 16:13:48serhiy.storchakasetmessages: + msg179467
2012-12-30 20:11:14serhiy.storchakasetmessages: + msg178613
2012-12-29 22:16:52serhiy.storchakasetassignee: serhiy.storchaka
2012-10-24 19:23:22serhiy.storchakasetfiles: + posix_realpath.patch
versions: + Python 3.4
nosy: + serhiy.storchaka

messages: + msg173696

keywords: - easy
2012-10-23 11:11:18tzimmosetfiles: + issue6975_py3.patch
nosy: + tzimmo
messages: + msg173590

2012-09-26 18:51:22ezio.melottisetkeywords: + easy
2012-06-26 21:36:58r.david.murraysetnosy: + pitrou, r.david.murray, hynek

versions: - Python 3.1
2012-06-26 21:36:13r.david.murraylinkissue15196 superseder
2012-06-26 21:23:36o11csetnosy: + o11c

messages: + msg164111
versions: + Python 3.3
2010-07-22 07:47:24brett.cannonsetnosy: - brett.cannon
2010-07-21 16:10:31BreamoreBoysetnosy: + BreamoreBoy

messages: + msg111082
versions: + Python 3.1, Python 2.7, Python 3.2, - Python 2.5
2009-10-18 09:07:21markonsetmessages: + msg94209
2009-10-18 03:00:11swareckisetmessages: + msg94199
2009-10-17 15:15:23markonsetmessages: + msg94179
2009-10-17 14:48:48markonsetnosy: + brett.cannon
2009-10-17 01:09:48ezio.melottisetpriority: normal
nosy: + ezio.melotti

stage: patch review
2009-10-16 21:05:55markonsetfiles: + issue6975.patch

nosy: + markon
messages: + msg94149

keywords: + patch
2009-09-23 05:56:20swareckisetmessages: + msg93029
2009-09-23 05:55:12swareckicreate