From 38b491c8ad1792451af4d2dba4899e583f1cbb47 Mon Sep 17 00:00:00 2001 From: barneygale Date: Sun, 19 May 2024 15:27:26 +0100 Subject: [PATCH 1/7] pathlib ABCs: remove duplicate `realpath()` implementation. Add private `posixpath._realpath()` function, which is a generic version of `realpath()` that can be parameterised with string tokens (`sep`, `curdir`, `pardir`) and query functions (`getcwd`, `lstat`, `readlink`). Also add support for limiting the number of symlink traversals. In the private `pathlib._abc.PathBase` class, call `posixpath._realpath()` and remove our re-implementation of the same algorithm. This speeds up `PathBase.resolve()` because we instantiate fewer `PathBase` objects, and because `_realpath()` caches symlink targets. --- Lib/pathlib/_abc.py | 88 +++++++++++++++------------------------------ Lib/posixpath.py | 16 ++++++--- 2 files changed, 41 insertions(+), 63 deletions(-) diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index 568a17df26fc336..c2ba6a671ba43df 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -12,8 +12,8 @@ """ import functools +import posixpath from glob import _Globber, _no_recurse_symlinks -from errno import ENOTDIR, ELOOP from stat import S_ISDIR, S_ISLNK, S_ISREG, S_ISSOCK, S_ISBLK, S_ISCHR, S_ISFIFO @@ -670,65 +670,35 @@ def resolve(self, strict=False): """ if self._resolving: return self - path_root, parts = self._stack - path = self.with_segments(path_root) - try: - path = path.absolute() - except UnsupportedOperation: - path_tail = [] - else: - path_root, path_tail = path._stack - path_tail.reverse() - - # If the user has *not* overridden the `readlink()` method, then symlinks are unsupported - # and (in non-strict mode) we can improve performance by not calling `stat()`. - querying = strict or getattr(self.readlink, '_supported', True) - link_count = 0 - while parts: - part = parts.pop() - if not part or part == '.': - continue - if part == '..': - if not path_tail: - if path_root: - # Delete '..' segment immediately following root - continue - elif path_tail[-1] != '..': - # Delete '..' segment and its predecessor - path_tail.pop() - continue - path_tail.append(part) - if querying and part != '..': - path = self.with_segments(path_root + self.parser.sep.join(path_tail)) + + def getcwd(): + return str(self.with_segments().absolute()) + + if strict or getattr(self.readlink, '_supported', True): + def lstat(path_str): + path = self.with_segments(path_str) path._resolving = True - try: - st = path.stat(follow_symlinks=False) - if S_ISLNK(st.st_mode): - # Like Linux and macOS, raise OSError(errno.ELOOP) if too many symlinks are - # encountered during resolution. - link_count += 1 - if link_count >= self._max_symlinks: - raise OSError(ELOOP, "Too many symbolic links in path", self._raw_path) - target_root, target_parts = path.readlink()._stack - # If the symlink target is absolute (like '/etc/hosts'), set the current - # path to its uppermost parent (like '/'). - if target_root: - path_root = target_root - path_tail.clear() - else: - path_tail.pop() - # Add the symlink target's reversed tail parts (like ['hosts', 'etc']) to - # the stack of unresolved path parts. - parts.extend(target_parts) - continue - elif parts and not S_ISDIR(st.st_mode): - raise NotADirectoryError(ENOTDIR, "Not a directory", self._raw_path) - except OSError: - if strict: - raise - else: - querying = False - return self.with_segments(path_root + self.parser.sep.join(path_tail)) + return path.lstat() + + def readlink(path_str): + path = self.with_segments(path_str) + path._resolving = True + return str(path.readlink()) + + else: + # If the user has *not* overridden the `readlink()` method, then + # symlinks are unsupported and (in non-strict mode) we can improve + # performance by not calling `path.lstat()`. + def lstat(path_str): + raise OSError("Symlinks are unsupported.") + + def readlink(path_str): + raise OSError("Symlinks are unsupported.") + + return self.with_segments(posixpath._realpath( + str(self), strict, self.parser.sep, + getcwd=getcwd, lstat=lstat, readlink=readlink, + maxlinks=self._max_symlinks)) def symlink_to(self, target, target_is_directory=False): """ diff --git a/Lib/posixpath.py b/Lib/posixpath.py index c04c628de55ee26..30a487a335404af 100644 --- a/Lib/posixpath.py +++ b/Lib/posixpath.py @@ -22,6 +22,7 @@ altsep = None devnull = '/dev/null' +import errno import os import sys import stat @@ -432,7 +433,10 @@ def realpath(filename, *, strict=False): curdir = '.' pardir = '..' getcwd = os.getcwd + return _realpath(filename, strict, sep, curdir, pardir, getcwd) +def _realpath(filename, strict, sep=sep, curdir=curdir, pardir=pardir, + getcwd=os.getcwd, lstat=os.lstat, readlink=os.readlink, maxlinks=-1): # The stack of unresolved path parts. When popped, a special value of None # indicates that a symlink target has been resolved, and that the original # symlink path can be retrieved by popping again. The [::-1] slice is a @@ -448,6 +452,7 @@ def realpath(filename, *, strict=False): # used both to detect symlink loops and to speed up repeated traversals of # the same links. seen = {} + link_count = 0 while rest: name = rest.pop() @@ -467,10 +472,14 @@ def realpath(filename, *, strict=False): else: newpath = path + sep + name try: - st = os.lstat(newpath) + st = lstat(newpath) if not stat.S_ISLNK(st.st_mode): path = newpath continue + if strict and maxlinks != -1: + link_count += 1 + if link_count > maxlinks: + raise OSError(errno.ELOOP, "Too many symbolic links in path", newpath) if newpath in seen: # Already seen this path path = seen[newpath] @@ -479,11 +488,10 @@ def realpath(filename, *, strict=False): continue # The symlink is not resolved, so we must have a symlink loop. if strict: - # Raise OSError(errno.ELOOP) - os.stat(newpath) + raise OSError(errno.ELOOP, "Symlink loop", newpath) path = newpath continue - target = os.readlink(newpath) + target = readlink(newpath) except OSError: if strict: raise From 17bce8a0b21c7529679207d3f6656741525a5350 Mon Sep 17 00:00:00 2001 From: barneygale Date: Sun, 19 May 2024 16:05:22 +0100 Subject: [PATCH 2/7] Slightly improve code --- Lib/pathlib/_abc.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index c2ba6a671ba43df..8ad3e4a4ec4cf1e 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -689,11 +689,11 @@ def readlink(path_str): # If the user has *not* overridden the `readlink()` method, then # symlinks are unsupported and (in non-strict mode) we can improve # performance by not calling `path.lstat()`. - def lstat(path_str): - raise OSError("Symlinks are unsupported.") + def skip(path_str): + # This exception will be internally consumed by `_realpath()`. + raise OSError("Operation skipped.") - def readlink(path_str): - raise OSError("Symlinks are unsupported.") + lstat = readlink = skip return self.with_segments(posixpath._realpath( str(self), strict, self.parser.sep, From 4ac30d92fc53cc5022ae6ab46fb969dd523e8462 Mon Sep 17 00:00:00 2001 From: barneygale Date: Sun, 19 May 2024 18:42:01 +0100 Subject: [PATCH 3/7] Disable `seen` mapping when maxlinks is given. --- Lib/posixpath.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/Lib/posixpath.py b/Lib/posixpath.py index 30a487a335404af..e17f028e3861e20 100644 --- a/Lib/posixpath.py +++ b/Lib/posixpath.py @@ -476,11 +476,14 @@ def _realpath(filename, strict, sep=sep, curdir=curdir, pardir=pardir, if not stat.S_ISLNK(st.st_mode): path = newpath continue - if strict and maxlinks != -1: + if maxlinks != -1: link_count += 1 if link_count > maxlinks: - raise OSError(errno.ELOOP, "Too many symbolic links in path", newpath) - if newpath in seen: + if strict: + raise OSError(errno.ELOOP, "Too many symbolic links in path", newpath) + path = newpath + continue + elif newpath in seen: # Already seen this path path = seen[newpath] if path is not None: @@ -498,15 +501,17 @@ def _realpath(filename, strict, sep=sep, curdir=curdir, pardir=pardir, path = newpath continue # Resolve the symbolic link - seen[newpath] = None # not resolved symlink if target.startswith(sep): # Symlink target is absolute; reset resolved path. path = sep - # Push the symlink path onto the stack, and signal its specialness by - # also pushing None. When these entries are popped, we'll record the - # fully-resolved symlink target in the 'seen' mapping. - rest.append(newpath) - rest.append(None) + if maxlinks == -1: + # Mark this symlink as seen but not fully resolved. + seen[newpath] = None + # Push the symlink path onto the stack, and signal its specialness by + # also pushing None. When these entries are popped, we'll record the + # fully-resolved symlink target in the 'seen' mapping. + rest.append(newpath) + rest.append(None) # Push the unresolved symlink target parts onto the stack. rest.extend(target.split(sep)[::-1]) From 716b3440e4e0dd3911cfbbdce0dcc84ae33430e4 Mon Sep 17 00:00:00 2001 From: barneygale Date: Sun, 19 May 2024 19:02:51 +0100 Subject: [PATCH 4/7] Small tidy-ups --- Lib/posixpath.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Lib/posixpath.py b/Lib/posixpath.py index e17f028e3861e20..25ad5f19e3c6739 100644 --- a/Lib/posixpath.py +++ b/Lib/posixpath.py @@ -452,6 +452,9 @@ def _realpath(filename, strict, sep=sep, curdir=curdir, pardir=pardir, # used both to detect symlink loops and to speed up repeated traversals of # the same links. seen = {} + + # Number of symlinks traversed. When the number of traversals is limited + # by *maxlinks*, this is used instead of *seen* to detect symlink loops. link_count = 0 while rest: @@ -476,7 +479,7 @@ def _realpath(filename, strict, sep=sep, curdir=curdir, pardir=pardir, if not stat.S_ISLNK(st.st_mode): path = newpath continue - if maxlinks != -1: + elif maxlinks != -1: link_count += 1 if link_count > maxlinks: if strict: @@ -507,9 +510,9 @@ def _realpath(filename, strict, sep=sep, curdir=curdir, pardir=pardir, if maxlinks == -1: # Mark this symlink as seen but not fully resolved. seen[newpath] = None - # Push the symlink path onto the stack, and signal its specialness by - # also pushing None. When these entries are popped, we'll record the - # fully-resolved symlink target in the 'seen' mapping. + # Push the symlink path onto the stack, and signal its specialness + # by also pushing None. When these entries are popped, we'll + # record the fully-resolved symlink target in the 'seen' mapping. rest.append(newpath) rest.append(None) # Push the unresolved symlink target parts onto the stack. From db79c9473e03da38336decd47b1f459cb1448927 Mon Sep 17 00:00:00 2001 From: barneygale Date: Sun, 19 May 2024 19:11:17 +0100 Subject: [PATCH 5/7] Use `is` comparison for speed, add *strict* default. --- Lib/posixpath.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/posixpath.py b/Lib/posixpath.py index 25ad5f19e3c6739..cef098512726429 100644 --- a/Lib/posixpath.py +++ b/Lib/posixpath.py @@ -435,8 +435,8 @@ def realpath(filename, *, strict=False): getcwd = os.getcwd return _realpath(filename, strict, sep, curdir, pardir, getcwd) -def _realpath(filename, strict, sep=sep, curdir=curdir, pardir=pardir, - getcwd=os.getcwd, lstat=os.lstat, readlink=os.readlink, maxlinks=-1): +def _realpath(filename, strict=False, sep=sep, curdir=curdir, pardir=pardir, + getcwd=os.getcwd, lstat=os.lstat, readlink=os.readlink, maxlinks=None): # The stack of unresolved path parts. When popped, a special value of None # indicates that a symlink target has been resolved, and that the original # symlink path can be retrieved by popping again. The [::-1] slice is a @@ -479,7 +479,7 @@ def _realpath(filename, strict, sep=sep, curdir=curdir, pardir=pardir, if not stat.S_ISLNK(st.st_mode): path = newpath continue - elif maxlinks != -1: + elif maxlinks is not None: link_count += 1 if link_count > maxlinks: if strict: @@ -507,7 +507,7 @@ def _realpath(filename, strict, sep=sep, curdir=curdir, pardir=pardir, if target.startswith(sep): # Symlink target is absolute; reset resolved path. path = sep - if maxlinks == -1: + if maxlinks is None: # Mark this symlink as seen but not fully resolved. seen[newpath] = None # Push the symlink path onto the stack, and signal its specialness From 943fad4d69a6431eb374169e6575a44c056ddee9 Mon Sep 17 00:00:00 2001 From: barneygale Date: Sun, 19 May 2024 19:29:49 +0100 Subject: [PATCH 6/7] Address review feedback --- Lib/pathlib/_abc.py | 1 - Lib/posixpath.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index 8ad3e4a4ec4cf1e..7a5af1e5ac0bc0c 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -684,7 +684,6 @@ def readlink(path_str): path = self.with_segments(path_str) path._resolving = True return str(path.readlink()) - else: # If the user has *not* overridden the `readlink()` method, then # symlinks are unsupported and (in non-strict mode) we can improve diff --git a/Lib/posixpath.py b/Lib/posixpath.py index cef098512726429..f73ff9d97c6db16 100644 --- a/Lib/posixpath.py +++ b/Lib/posixpath.py @@ -483,7 +483,7 @@ def _realpath(filename, strict=False, sep=sep, curdir=curdir, pardir=pardir, link_count += 1 if link_count > maxlinks: if strict: - raise OSError(errno.ELOOP, "Too many symbolic links in path", newpath) + raise OSError(errno.ELOOP, "Too many levels of symbolic links", newpath) path = newpath continue elif newpath in seen: @@ -494,7 +494,7 @@ def _realpath(filename, strict=False, sep=sep, curdir=curdir, pardir=pardir, continue # The symlink is not resolved, so we must have a symlink loop. if strict: - raise OSError(errno.ELOOP, "Symlink loop", newpath) + raise OSError(errno.ELOOP, "Too many levels of symbolic links", newpath) path = newpath continue target = readlink(newpath) From 293ca50640ce36305237cc7cc7257701455b1471 Mon Sep 17 00:00:00 2001 From: Barney Gale Date: Mon, 3 Jun 2024 23:10:16 +0100 Subject: [PATCH 7/7] Apply suggestions from code review Co-authored-by: Nice Zombies --- Lib/posixpath.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/posixpath.py b/Lib/posixpath.py index f73ff9d97c6db16..c418d2bf37e534e 100644 --- a/Lib/posixpath.py +++ b/Lib/posixpath.py @@ -483,7 +483,8 @@ def _realpath(filename, strict=False, sep=sep, curdir=curdir, pardir=pardir, link_count += 1 if link_count > maxlinks: if strict: - raise OSError(errno.ELOOP, "Too many levels of symbolic links", newpath) + raise OSError(errno.ELOOP, os.strerror(errno.ELOOP), + newpath) path = newpath continue elif newpath in seen: @@ -494,7 +495,8 @@ def _realpath(filename, strict=False, sep=sep, curdir=curdir, pardir=pardir, continue # The symlink is not resolved, so we must have a symlink loop. if strict: - raise OSError(errno.ELOOP, "Too many levels of symbolic links", newpath) + raise OSError(errno.ELOOP, os.strerror(errno.ELOOP), + newpath) path = newpath continue target = readlink(newpath)