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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-109187: Improve symlink loop handling in pathlib.Path.resolve() #109192

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 9 additions & 5 deletions Doc/library/pathlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1381,15 +1381,19 @@ call fails (for example because the path doesn't exist).
>>> p.resolve()
PosixPath('/home/antoine/pathlib/setup.py')

If the path doesn't exist and *strict* is ``True``, :exc:`FileNotFoundError`
is raised. If *strict* is ``False``, the path is resolved as far as possible
and any remainder is appended without checking whether it exists. If an
infinite loop is encountered along the resolution path, :exc:`RuntimeError`
is raised.
If a path doesn't exist or a symlink loop is encountered, and *strict* is
``True``, :exc:`OSError` is raised. If *strict* is ``False``, the path is
resolved as far as possible and any remainder is appended without checking
whether it exists.

.. versionchanged:: 3.6
The *strict* parameter was added (pre-3.6 behavior is strict).

.. versionchanged:: 3.13
Symlink loops are treated like other errors: :exc:`OSError` is raised in
strict mode, and no exception is raised in non-strict mode. In previous
versions, :exc:`RuntimeError` is raised no matter the value of *strict*.

.. method:: Path.rglob(pattern, *, case_sensitive=None, follow_symlinks=None)

Glob the given relative *pattern* recursively. This is like calling
Expand Down
21 changes: 1 addition & 20 deletions Lib/pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -1230,26 +1230,7 @@ def resolve(self, strict=False):
normalizing it.
"""

def check_eloop(e):
winerror = getattr(e, 'winerror', 0)
if e.errno == ELOOP or winerror == _WINERROR_CANT_RESOLVE_FILENAME:
raise RuntimeError("Symlink loop from %r" % e.filename)

try:
s = os.path.realpath(self, strict=strict)
except OSError as e:
check_eloop(e)
raise
p = self.with_segments(s)

# In non-strict mode, realpath() doesn't raise on symlink loops.
# Ensure we get an exception by calling stat()
if not strict:
try:
p.stat()
except OSError as e:
check_eloop(e)
return p
return self.with_segments(os.path.realpath(self, strict=strict))

def owner(self):
"""
Expand Down
13 changes: 8 additions & 5 deletions Lib/test/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -3178,10 +3178,11 @@ def test_absolute(self):
self.assertEqual(str(P('//a').absolute()), '//a')
self.assertEqual(str(P('//a/b').absolute()), '//a/b')

def _check_symlink_loop(self, *args, strict=True):
def _check_symlink_loop(self, *args):
path = self.cls(*args)
with self.assertRaises(RuntimeError):
print(path.resolve(strict))
with self.assertRaises(OSError) as cm:
path.resolve(strict=True)
self.assertEqual(cm.exception.errno, errno.ELOOP)

@unittest.skipIf(
is_emscripten or is_wasi,
Expand Down Expand Up @@ -3240,7 +3241,8 @@ def test_resolve_loop(self):
os.symlink('linkZ/../linkZ', join('linkZ'))
self._check_symlink_loop(BASE, 'linkZ')
# Non-strict
self._check_symlink_loop(BASE, 'linkZ', 'foo', strict=False)
p = self.cls(BASE, 'linkZ', 'foo')
self.assertEqual(p.resolve(strict=False), p)
# Loops with absolute symlinks.
os.symlink(join('linkU/inside'), join('linkU'))
self._check_symlink_loop(BASE, 'linkU')
Expand All @@ -3249,7 +3251,8 @@ def test_resolve_loop(self):
os.symlink(join('linkW/../linkW'), join('linkW'))
self._check_symlink_loop(BASE, 'linkW')
# Non-strict
self._check_symlink_loop(BASE, 'linkW', 'foo', strict=False)
q = self.cls(BASE, 'linkW', 'foo')
self.assertEqual(q.resolve(strict=False), q)

def test_glob(self):
P = self.cls
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
:meth:`pathlib.Path.resolve` now treats symlink loops like other errors: in
strict mode, :exc:`OSError` is raised, and in non-strict mode, no exception
is raised.