Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os.path.realpath('loop/../link') does not resolve link #117546

Closed
barneygale opened this issue Apr 4, 2024 · 6 comments
Closed

os.path.realpath('loop/../link') does not resolve link #117546

barneygale opened this issue Apr 4, 2024 · 6 comments
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes stdlib Python modules in the Lib dir

Comments

@barneygale
Copy link
Contributor

barneygale commented Apr 4, 2024

If os.path.realpath() encounters a symlink loop on posix, it prematurely stops querying paths and resolving symlink targets. This differs from coreutils realpath:

$ cd /tmp
$ ln -s loop loop
$ ln -s target link
$ realpath -m loop/../link
/tmp/target
$ python -c 'import os.path; print(os.path.realpath("loop/../link"))'
/tmp/link

It also differs from how lstat() errors are handled:

$ realpath -m missing/../link
/tmp/target
$ python -c 'import os.path; print(os.path.realpath("missing/../link"))'
/tmp/target

Linked PRs

@eryksun eryksun added stdlib Python modules in the Lib dir 3.12 bugs and security fixes 3.13 bugs and security fixes labels Apr 4, 2024
@eryksun
Copy link
Contributor

eryksun commented Apr 4, 2024

As Barney noted, this is only an issue on POSIX. The non-strict implementation on Windows ignores a symlink loop to resolve as much as possible. Also, on Windows, for the given examples, the ".." component gets resolved first, which is like the -L (i.e. --logical) option of coreutils realpath. That makes os.path.realpath() on Windows partially non-strict even with strict=True. Nowadays the system implements this logical path handling in both the user-mode runtime library and the kernel. (In older versions of Windows, the kernel implemented POSIX-like physical path parsing when resolving the target of a relative symlink. For example, for "loop\..\target", it used to resolve "loop" before it resolved "..".)

barneygale added a commit to barneygale/cpython that referenced this issue Apr 5, 2024
…/link')`

Continue resolving symlink targets after encountering a symlink loop, which
matches coreutils `realpath` behaviour.
@barneygale
Copy link
Contributor Author

Thanks Eryk!

I suppose users could do realpath(normpath(path)) to get realpath --logical behaviour on POSIX.

@encukou
Copy link
Member

encukou commented Apr 8, 2024

This behaviour goes back to Python 2. IMO, we should be very careful when changing it. The status quo seems fine.
OTOH, it would be nice if realpath was idempotent.

I don't feel qualified to review this.

@barneygale
Copy link
Contributor Author

Appreciate the feedback.

Serhiy has separately approved the change, but given your reservations, I won't merge yet. Hopefully another core dev comes along to share their views and perhaps cast the deciding vote (if not I'll summon one in due course).

This bug is pretty rare. To hit it you need to supply a path that:

  1. Includes a symlink loop, and
  2. Has subsequent .. segment(s) that walk up past the loop (e.g. loop/.., or loop/blah/../../)
  3. Has further segments that point to a different, non-looping symlink

The unlikelihood of hitting all three might explain why I couldn't find any existing bug reports, despite the bug going way back :)

@encukou
Copy link
Member

encukou commented Apr 10, 2024

Serhiy is the os.path expert, he should get the deciding vote. I am careful because I'm not familiar with os.path maintenance :)

I agree that this is an edge case you hopefully won't see in production.

barneygale added a commit that referenced this issue Apr 10, 2024
…)` (#117568)

Continue resolving symlink targets after encountering a symlink loop, which
matches coreutils `realpath` behaviour.
@barneygale
Copy link
Contributor Author

Thanks! Fix merged into main. I'm not planning to backport because the fix would need to be different (due to #114848) and because the bug is so rare. If anyone would like it backported, please leave a comment and I'll re-open.

diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
…/link')` (python#117568)

Continue resolving symlink targets after encountering a symlink loop, which
matches coreutils `realpath` behaviour.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes stdlib Python modules in the Lib dir
Projects
None yet
Development

No branches or pull requests

3 participants