diff --git a/Lib/contextlib.py b/Lib/contextlib.py index 5b646fabca0225..2ac1a9808e37cf 100644 --- a/Lib/contextlib.py +++ b/Lib/contextlib.py @@ -805,10 +805,38 @@ class chdir(AbstractContextManager): def __init__(self, path): self.path = path self._old_cwd = [] + if self._supports_fd(): + self._getcwd = self._getcwd_fd + self._close = os.close + + @staticmethod + def _supports_fd(): + return os.chdir in os.supports_fd + + @staticmethod + def _getcwd(): + return os.getcwd() + + @staticmethod + def _close(dir): + pass + + @staticmethod + def _getcwd_fd(): + return os.open('.', os.O_RDONLY) def __enter__(self): - self._old_cwd.append(os.getcwd()) - os.chdir(self.path) + dir = self._getcwd() + try: + os.chdir(self.path) + except: + self._close(dir) + raise + self._old_cwd.append(dir) def __exit__(self, *excinfo): - os.chdir(self._old_cwd.pop()) + dir = self._old_cwd.pop() + try: + os.chdir(dir) + finally: + self._close(dir) diff --git a/Lib/test/test_contextlib.py b/Lib/test/test_contextlib.py index cf6519598037e9..34e009dbcccb56 100644 --- a/Lib/test/test_contextlib.py +++ b/Lib/test/test_contextlib.py @@ -1361,6 +1361,61 @@ def test_exception(self): self.assertEqual(str(re), "boom") self.assertEqual(os.getcwd(), old_cwd) + def test_failed_chdir(self): + old_cwd = os.getcwd() + target = self.make_relative_path('non_existent_directory') + self.assertNotEqual(old_cwd, target) + ctx = chdir(target) + self.assertRaises(OSError, ctx.__enter__) + self.assertFalse(ctx._old_cwd) + self.assertEqual(os.getcwd(), old_cwd) + + @unittest.skipUnless(chdir._supports_fd(), "chdir requires fd support") + def test_chdir_to_fd(self): + old_cwd = os.getcwd() + target = self.make_relative_path('data') + fd = os.open(target, os.O_RDONLY) + try: + with chdir(fd): + self.assertEqual(os.getcwd(), target) + finally: + os.close(fd) + self.assertEqual(os.getcwd(), old_cwd) + + @unittest.skipUnless(chdir._supports_fd(), + "chdir requires fd support for long path") + @unittest.skipUnless(os_helper.can_symlink(), "need symlink support") + def test_original_path_too_long(self): + with tempfile.TemporaryDirectory() as dir: + with chdir(dir): + count = 0 + big_name = 'a' * os.pathconf('.', 'PC_NAME_MAX') + path_max = os.pathconf('.', 'PC_PATH_MAX') + while len(dir) < path_max + 1: + dir = os.path.join(dir, big_name) + os.mkdir(big_name) + os.symlink(big_name, 'a', target_is_directory=True) + os.chdir('a') + count += 1 + + self.assertGreater(count, 0) + os.mkdir('a') + with chdir('a'): + self.assertGreater(len(os.getcwd()), path_max) + + @unittest.skipUnless(chdir._supports_fd(), + "chdir requires fd support for deleted dir") + def test_original_path_deleted(self): + original = os.getcwd() + dir = tempfile.TemporaryDirectory() + self.addCleanup(dir.cleanup) + try: + os.chdir(dir.name) + with chdir(original): + dir.cleanup() + finally: + os.chdir(original) + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Library/2025-04-26-12-10-32.gh-issue-89708.haEUh3.rst b/Misc/NEWS.d/next/Library/2025-04-26-12-10-32.gh-issue-89708.haEUh3.rst new file mode 100644 index 00000000000000..2189490d694852 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-04-26-12-10-32.gh-issue-89708.haEUh3.rst @@ -0,0 +1,3 @@ +Use file descriptors in :func:`contextlib.chdir` when possible to prevent +issues when the original working directory is concurrently deleted or when its +path length is longer than the maximum allowed by the host platform.