From e2b4c6d7996c1c406687bd9a8f000ac414b6ed3a Mon Sep 17 00:00:00 2001 From: barneygale Date: Mon, 12 Aug 2024 01:10:38 +0100 Subject: [PATCH] GH-73991: Disallow copying directory into itself via `pathlib.Path.copy()` --- Lib/pathlib/_abc.py | 38 +++++++++++++++++++---- Lib/test/test_pathlib/test_pathlib_abc.py | 25 +++++++++++++++ 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index 500846d19cf811..e835351ca2a10e 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -11,6 +11,7 @@ resemble pathlib's PurePath and Path respectively. """ +import errno import functools import operator import posixpath @@ -564,14 +565,33 @@ def samefile(self, other_path): return (st.st_ino == other_st.st_ino and st.st_dev == other_st.st_dev) - def _samefile_safe(self, other_path): + def _ensure_different_file(self, other_path): """ - Like samefile(), but returns False rather than raising OSError. + Raise OSError(EINVAL) if both paths refer to the same file. """ try: - return self.samefile(other_path) + if not self.samefile(other_path): + return except (OSError, ValueError): - return False + return + err = OSError(errno.EINVAL, "Source and target are the same file") + err.filename = str(self) + err.filename2 = str(other_path) + raise err + + def _ensure_distinct_path(self, other_path): + """ + Raise OSError(EINVAL) if the other path is within this path. + """ + if self == other_path: + err = OSError(errno.EINVAL, "Source and target are the same path") + elif self in other_path.parents: + err = OSError(errno.EINVAL, "Source path is a parent of target path") + else: + return + err.filename = str(self) + err.filename2 = str(other_path) + raise err def open(self, mode='r', buffering=-1, encoding=None, errors=None, newline=None): @@ -826,8 +846,7 @@ def _copy_file(self, target): """ Copy the contents of this file to the given target. """ - if self._samefile_safe(target): - raise OSError(f"{self!r} and {target!r} are the same file") + self._ensure_different_file(target) with self.open('rb') as source_f: try: with target.open('wb') as target_f: @@ -847,6 +866,13 @@ def copy(self, target, *, follow_symlinks=True, dirs_exist_ok=False, """ if not isinstance(target, PathBase): target = self.with_segments(target) + try: + self._ensure_distinct_path(target) + except OSError as err: + if on_error is None: + raise + on_error(err) + return stack = [(self, target)] while stack: src, dst = stack.pop() diff --git a/Lib/test/test_pathlib/test_pathlib_abc.py b/Lib/test/test_pathlib/test_pathlib_abc.py index 629a1d4bdeb4de..ea752dc7134e97 100644 --- a/Lib/test/test_pathlib/test_pathlib_abc.py +++ b/Lib/test/test_pathlib/test_pathlib_abc.py @@ -1823,6 +1823,12 @@ def test_copy_file_empty(self): self.assertTrue(target.exists()) self.assertEqual(target.read_bytes(), b'') + def test_copy_file_to_itself(self): + base = self.cls(self.base) + source = base / 'empty' + source.write_bytes(b'') + self.assertRaises(OSError, source.copy, source) + def test_copy_dir_simple(self): base = self.cls(self.base) source = base / 'dirC' @@ -1909,6 +1915,25 @@ def test_copy_dir_to_existing_directory_dirs_exist_ok(self): self.assertTrue(target.joinpath('fileC').read_text(), "this is file C\n") + def test_copy_dir_to_itself(self): + base = self.cls(self.base) + source = base / 'dirC' + self.assertRaises(OSError, source.copy, source) + + def test_copy_dir_to_itself_on_error(self): + base = self.cls(self.base) + source = base / 'dirC' + errors = [] + source.copy(source, on_error=errors.append) + self.assertEqual(len(errors), 1) + self.assertIsInstance(errors[0], OSError) + + def test_copy_dir_into_itself(self): + base = self.cls(self.base) + source = base / 'dirC' + target = base / 'dirC' / 'dirD' / 'copyC' + self.assertRaises(OSError, source.copy, target) + def test_copy_missing_on_error(self): base = self.cls(self.base) source = base / 'foo'