Skip to content

Commit 86c318c

Browse files
barneygalebarneygale
committed
realpath(): only raise OSError for symlink loops in strict mode
1 parent a82bc18 commit 86c318c

File tree

8 files changed

+151
-49
lines changed

8 files changed

+151
-49
lines changed

‎Doc/library/os.path.rst

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -350,9 +350,10 @@ the :mod:`glob` module.)
350350
links encountered in the path (if they are supported by the operating
351351
system).
352352

353-
If the path doesn't exist and *strict* is ``True``, :exc:`FileNotFoundError`
354-
is raised. If *strict* is ``False``, the path is resolved as far as possible
355-
and any remainder is appended without checking whether it exists.
353+
If a path doesn't exist or a symlink loop is encountered, and *strict* is
354+
``True``, :exc:`OSError` is raised. If *strict* is ``False``, the path is
355+
resolved as far as possible and any remainder is appended without checking
356+
whether it exists.
356357

357358
.. note::
358359
This function emulates the operating system's procedure for making a path
@@ -371,10 +372,6 @@ the :mod:`glob` module.)
371372
.. versionchanged:: 3.10
372373
The *strict* parameter was added.
373374

374-
.. versionchanged:: 3.10
375-
Raises :exc:`OSError` when a symbolic link cycle occurs. Previously
376-
returned one member of the cycle.
377-
378375

379376
.. function:: relpath(path, start=os.curdir)
380377

‎Lib/ntpath.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,8 @@ def _getfinalpathname_nonstrict(path):
603603
# 87: ERROR_INVALID_PARAMETER
604604
# 123: ERROR_INVALID_NAME
605605
# 1920: ERROR_CANT_ACCESS_FILE
606-
allowed_winerror = 1, 2, 3, 5, 21, 32, 50, 67, 87, 123, 1920
606+
# 1921: ERROR_CANT_RESOLVE_FILENAME (implies unfollowable symlink)
607+
allowed_winerror = 1, 2, 3, 5, 21, 32, 50, 67, 87, 123, 1920, 1921
607608

608609
# Non-strict algorithm is to find as much of the target directory
609610
# as we can and join the rest.
@@ -659,10 +660,7 @@ def realpath(path, *, strict=False):
659660
path = _getfinalpathname(path)
660661
initial_winerror = 0
661662
except OSError as ex:
662-
# ERROR_CANT_RESOLVE_FILENAME (1921) is from exceeding the
663-
# max allowed number of reparse attempts (currently 63), which
664-
# is either due to a loop or a chain of links that's too long.
665-
if strict or ex.winerror == 1921:
663+
if strict:
666664
raise
667665
initial_winerror = ex.winerror
668666
path = _getfinalpathname_nonstrict(path)

‎Lib/pathlib.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,14 +1055,22 @@ def resolve(self, strict=False):
10551055
normalizing it (for example turning slashes into backslashes under
10561056
Windows).
10571057
"""
1058+
10581059
try:
1059-
p = self._accessor.realpath(self, strict=strict)
1060+
s = self._accessor.realpath(self, strict=strict)
1061+
p = self._from_parts((s,))
1062+
1063+
# In non-strict mode, realpath() doesn't raise on symlink loops.
1064+
# Ensure we get an exception by calling stat()
1065+
if not strict:
1066+
p.stat()
10601067
except OSError as ex:
1061-
winerr = getattr(ex, 'winerror', 0)
1062-
if ex.errno == ELOOP or winerr == _WINERROR_CANT_RESOLVE_FILENAME:
1068+
winerror = getattr(ex, 'winerror', 0)
1069+
if ex.errno == ELOOP or winerror == _WINERROR_CANT_RESOLVE_FILENAME:
10631070
raise RuntimeError("Symlink loop from %r", ex.filename)
1064-
raise
1065-
return self._from_parts((p,))
1071+
if strict:
1072+
raise
1073+
return p
10661074

10671075
def stat(self, *, follow_symlinks=True):
10681076
"""

‎Lib/posixpath.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ def realpath(filename, *, strict=False):
391391
"""Return the canonical path of the specified filename, eliminating any
392392
symbolic links encountered in the path."""
393393
filename = os.fspath(filename)
394-
path = _joinrealpath(filename[:0], filename, strict, {})
394+
path, ok = _joinrealpath(filename[:0], filename, strict, {})
395395
return abspath(path)
396396

397397
# Join two paths, normalizing and eliminating any symbolic links
@@ -444,13 +444,19 @@ def _joinrealpath(path, rest, strict, seen):
444444
# use cached value
445445
continue
446446
# The symlink is not resolved, so we must have a symlink loop.
447-
# Raise OSError(errno.ELOOP)
448-
os.stat(newpath)
447+
if strict:
448+
# Raise OSError(errno.ELOOP)
449+
os.stat(newpath)
450+
else:
451+
# Return already resolved part + rest of the path unchanged.
452+
return join(newpath, rest), False
449453
seen[newpath] = None # not resolved symlink
450-
path = _joinrealpath(path, os.readlink(newpath), strict, seen)
454+
path, ok = _joinrealpath(path, os.readlink(newpath), strict, seen)
455+
if not ok:
456+
return join(path, rest), False
451457
seen[newpath] = path # resolved symlink
452458

453-
return path
459+
return path, True
454460

455461

456462
supports_unicode_filenames = (sys.platform == 'darwin')

‎Lib/test/test_ntpath.py

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,9 @@ def test_realpath_broken_symlinks(self):
351351
@os_helper.skip_unless_symlink
352352
@unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname')
353353
def test_realpath_symlink_loops(self):
354-
# Symlink loops raise OSError
354+
# Symlink loops in non-strict mode are non-deterministic as to which
355+
# path is returned, but it will always be the fully resolved path of
356+
# one member of the cycle
355357
ABSTFN = ntpath.abspath(os_helper.TESTFN)
356358
self.addCleanup(os_helper.unlink, ABSTFN)
357359
self.addCleanup(os_helper.unlink, ABSTFN + "1")
@@ -361,16 +363,16 @@ def test_realpath_symlink_loops(self):
361363
self.addCleanup(os_helper.unlink, ABSTFN + "a")
362364

363365
os.symlink(ABSTFN, ABSTFN)
364-
self.assertRaises(OSError, ntpath.realpath, ABSTFN)
366+
self.assertPathEqual(ntpath.realpath(ABSTFN), ABSTFN)
365367

366368
os.symlink(ABSTFN + "1", ABSTFN + "2")
367369
os.symlink(ABSTFN + "2", ABSTFN + "1")
368-
self.assertRaises(OSError, ntpath.realpath, ABSTFN + "1")
369-
self.assertRaises(OSError, ntpath.realpath, ABSTFN + "2")
370-
self.assertRaises(OSError, ntpath.realpath, ABSTFN + "1\\x")
370+
expected = (ABSTFN + "1", ABSTFN + "2")
371+
self.assertPathIn(ntpath.realpath(ABSTFN + "1"), expected)
372+
self.assertPathIn(ntpath.realpath(ABSTFN + "2"), expected)
371373

372-
# Windows eliminates '..' components before resolving links, so the
373-
# following 3 realpath() calls are not expected to raise.
374+
self.assertPathIn(ntpath.realpath(ABSTFN + "1\\x"),
375+
(ntpath.join(r, "x") for r in expected))
374376
self.assertPathEqual(ntpath.realpath(ABSTFN + "1\\.."),
375377
ntpath.dirname(ABSTFN))
376378
self.assertPathEqual(ntpath.realpath(ABSTFN + "1\\..\\x"),
@@ -379,18 +381,66 @@ def test_realpath_symlink_loops(self):
379381
self.assertPathEqual(ntpath.realpath(ABSTFN + "1\\..\\"
380382
+ ntpath.basename(ABSTFN) + "y"),
381383
ABSTFN + "x")
384+
self.assertPathIn(ntpath.realpath(ABSTFN + "1\\..\\"
385+
+ ntpath.basename(ABSTFN) + "1"),
386+
expected)
387+
388+
os.symlink(ntpath.basename(ABSTFN) + "a\\b", ABSTFN + "a")
389+
self.assertPathEqual(ntpath.realpath(ABSTFN + "a"), ABSTFN + "a")
390+
391+
os.symlink("..\\" + ntpath.basename(ntpath.dirname(ABSTFN))
392+
+ "\\" + ntpath.basename(ABSTFN) + "c", ABSTFN + "c")
393+
self.assertPathEqual(ntpath.realpath(ABSTFN + "c"), ABSTFN + "c")
394+
395+
# Test using relative path as well.
396+
self.assertPathEqual(ntpath.realpath(ntpath.basename(ABSTFN)), ABSTFN)
397+
398+
@os_helper.skip_unless_symlink
399+
@unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname')
400+
def test_realpath_symlink_loops_strict(self):
401+
# Symlink loops raise OSError in strict mode
402+
ABSTFN = ntpath.abspath(os_helper.TESTFN)
403+
self.addCleanup(os_helper.unlink, ABSTFN)
404+
self.addCleanup(os_helper.unlink, ABSTFN + "1")
405+
self.addCleanup(os_helper.unlink, ABSTFN + "2")
406+
self.addCleanup(os_helper.unlink, ABSTFN + "y")
407+
self.addCleanup(os_helper.unlink, ABSTFN + "c")
408+
self.addCleanup(os_helper.unlink, ABSTFN + "a")
409+
410+
os.symlink(ABSTFN, ABSTFN)
411+
self.assertRaises(OSError, ntpath.realpath, ABSTFN, strict=True)
412+
413+
os.symlink(ABSTFN + "1", ABSTFN + "2")
414+
os.symlink(ABSTFN + "2", ABSTFN + "1")
415+
self.assertRaises(OSError, ntpath.realpath, ABSTFN + "1", strict=True)
416+
self.assertRaises(OSError, ntpath.realpath, ABSTFN + "2", strict=True)
417+
self.assertRaises(OSError, ntpath.realpath, ABSTFN + "1\\x", strict=True)
418+
419+
# Windows eliminates '..' components before resolving links, so the
420+
# following 3 realpath() calls are not expected to raise.
421+
self.assertPathEqual(ntpath.realpath(ABSTFN + "1\\..", strict=True),
422+
ntpath.dirname(ABSTFN))
423+
self.assertPathEqual(ntpath.realpath(ABSTFN + "1\\..\\x", strict=True),
424+
ntpath.dirname(ABSTFN) + "\\x")
425+
os.symlink(ABSTFN + "x", ABSTFN + "y")
426+
self.assertPathEqual(ntpath.realpath(ABSTFN + "1\\..\\"
427+
+ ntpath.basename(ABSTFN) + "y",
428+
strict=True),
429+
ABSTFN + "x")
382430
self.assertRaises(OSError, ntpath.realpath,
383-
ABSTFN + "1\\..\\" + ntpath.basename(ABSTFN) + "1")
431+
ABSTFN + "1\\..\\" + ntpath.basename(ABSTFN) + "1",
432+
strict=True)
384433

385434
os.symlink(ntpath.basename(ABSTFN) + "a\\b", ABSTFN + "a")
386-
self.assertRaises(OSError, ntpath.realpath, ABSTFN + "a")
435+
self.assertRaises(OSError, ntpath.realpath, ABSTFN + "a", strict=True)
387436

388437
os.symlink("..\\" + ntpath.basename(ntpath.dirname(ABSTFN))
389438
+ "\\" + ntpath.basename(ABSTFN) + "c", ABSTFN + "c")
390-
self.assertRaises(OSError, ntpath.realpath, ABSTFN + "c")
439+
self.assertRaises(OSError, ntpath.realpath, ABSTFN + "c", strict=True)
391440

392441
# Test using relative path as well.
393-
self.assertRaises(OSError, ntpath.realpath, ntpath.basename(ABSTFN))
442+
self.assertRaises(OSError, ntpath.realpath, ntpath.basename(ABSTFN),
443+
strict=True)
394444

395445
@os_helper.skip_unless_symlink
396446
@unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname')

‎Lib/test/test_posixpath.py

Lines changed: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -382,33 +382,78 @@ def test_realpath_relative(self):
382382
"Missing symlink implementation")
383383
@skip_if_ABSTFN_contains_backslash
384384
def test_realpath_symlink_loops(self):
385-
# Bug #43757, raise OSError if we get into an infinite symlink loop.
385+
# Bug #930024, return the path unchanged if we get into an infinite
386+
# symlink loop in non-strict mode (default).
386387
try:
387388
os.symlink(ABSTFN, ABSTFN)
388-
self.assertRaises(OSError, realpath, ABSTFN)
389+
self.assertEqual(realpath(ABSTFN), ABSTFN)
389390

390391
os.symlink(ABSTFN+"1", ABSTFN+"2")
391392
os.symlink(ABSTFN+"2", ABSTFN+"1")
392-
self.assertRaises(OSError, realpath, ABSTFN+"1")
393-
self.assertRaises(OSError, realpath, ABSTFN+"2")
393+
self.assertEqual(realpath(ABSTFN+"1"), ABSTFN+"1")
394+
self.assertEqual(realpath(ABSTFN+"2"), ABSTFN+"2")
394395

395-
self.assertRaises(OSError, realpath, ABSTFN+"1/x")
396-
self.assertRaises(OSError, realpath, ABSTFN+"1/..")
397-
self.assertRaises(OSError, realpath, ABSTFN+"1/../x")
396+
self.assertEqual(realpath(ABSTFN+"1/x"), ABSTFN+"1/x")
397+
self.assertEqual(realpath(ABSTFN+"1/.."), dirname(ABSTFN))
398+
self.assertEqual(realpath(ABSTFN+"1/../x"), dirname(ABSTFN) + "/x")
398399
os.symlink(ABSTFN+"x", ABSTFN+"y")
399-
self.assertRaises(OSError, realpath, ABSTFN+"1/../" + basename(ABSTFN) + "y")
400-
self.assertRaises(OSError, realpath, ABSTFN+"1/../" + basename(ABSTFN) + "1")
400+
self.assertEqual(realpath(ABSTFN+"1/../" + basename(ABSTFN) + "y"),
401+
ABSTFN + "y")
402+
self.assertEqual(realpath(ABSTFN+"1/../" + basename(ABSTFN) + "1"),
403+
ABSTFN + "1")
401404

402405
os.symlink(basename(ABSTFN) + "a/b", ABSTFN+"a")
403-
self.assertRaises(OSError, realpath, ABSTFN+"a")
406+
self.assertEqual(realpath(ABSTFN+"a"), ABSTFN+"a/b")
404407

405408
os.symlink("../" + basename(dirname(ABSTFN)) + "/" +
406409
basename(ABSTFN) + "c", ABSTFN+"c")
407-
self.assertRaises(OSError, realpath, ABSTFN+"c")
410+
self.assertEqual(realpath(ABSTFN+"c"), ABSTFN+"c")
408411

409412
# Test using relative path as well.
410413
with os_helper.change_cwd(dirname(ABSTFN)):
411-
self.assertRaises(OSError, realpath, basename(ABSTFN))
414+
self.assertEqual(realpath(basename(ABSTFN)), ABSTFN)
415+
finally:
416+
os_helper.unlink(ABSTFN)
417+
os_helper.unlink(ABSTFN+"1")
418+
os_helper.unlink(ABSTFN+"2")
419+
os_helper.unlink(ABSTFN+"y")
420+
os_helper.unlink(ABSTFN+"c")
421+
os_helper.unlink(ABSTFN+"a")
422+
423+
@unittest.skipUnless(hasattr(os, "symlink"),
424+
"Missing symlink implementation")
425+
@skip_if_ABSTFN_contains_backslash
426+
def test_realpath_symlink_loops_strict(self):
427+
# Bug #43757, raise OSError if we get into an infinite symlink loop in
428+
# strict mode.
429+
try:
430+
os.symlink(ABSTFN, ABSTFN)
431+
self.assertRaises(OSError, realpath, ABSTFN, strict=True)
432+
433+
os.symlink(ABSTFN+"1", ABSTFN+"2")
434+
os.symlink(ABSTFN+"2", ABSTFN+"1")
435+
self.assertRaises(OSError, realpath, ABSTFN+"1", strict=True)
436+
self.assertRaises(OSError, realpath, ABSTFN+"2", strict=True)
437+
438+
self.assertRaises(OSError, realpath, ABSTFN+"1/x", strict=True)
439+
self.assertRaises(OSError, realpath, ABSTFN+"1/..", strict=True)
440+
self.assertRaises(OSError, realpath, ABSTFN+"1/../x", strict=True)
441+
os.symlink(ABSTFN+"x", ABSTFN+"y")
442+
self.assertRaises(OSError, realpath,
443+
ABSTFN+"1/../" + basename(ABSTFN) + "y", strict=True)
444+
self.assertRaises(OSError, realpath,
445+
ABSTFN+"1/../" + basename(ABSTFN) + "1", strict=True)
446+
447+
os.symlink(basename(ABSTFN) + "a/b", ABSTFN+"a")
448+
self.assertRaises(OSError, realpath, ABSTFN+"a", strict=True)
449+
450+
os.symlink("../" + basename(dirname(ABSTFN)) + "/" +
451+
basename(ABSTFN) + "c", ABSTFN+"c")
452+
self.assertRaises(OSError, realpath, ABSTFN+"c", strict=True)
453+
454+
# Test using relative path as well.
455+
with os_helper.change_cwd(dirname(ABSTFN)):
456+
self.assertRaises(OSError, realpath, basename(ABSTFN), strict=True)
412457
finally:
413458
os_helper.unlink(ABSTFN)
414459
os_helper.unlink(ABSTFN+"1")

‎Misc/NEWS.d/next/Library/2021-04-08-22-11-27.bpo-25264.b33fa0.rst

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
:func:`os.path.realpath` now accepts a *strict* keyword-only argument.
2-
When set to ``True``, :exc:`FileNotFoundException` is raised if a path
3-
doesn't exist.
2+
When set to ``True``, :exc:`OSError` is raised if a path doesn't exist
3+
or a symlink loop is encountered.

‎Misc/NEWS.d/next/Library/2021-04-08-22-11-27.bpo-25264.hEqA5d.rst

Lines changed: 0 additions & 2 deletions
This file was deleted.

0 commit comments

Comments
 (0)