diff --git a/Lib/tempfile.py b/Lib/tempfile.py index bb18d60db0d9198..766cc63a98a1c0d 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -864,15 +864,22 @@ def __init__(self, suffix=None, prefix=None, dir=None, @classmethod def _rmtree(cls, name, ignore_errors=False): + def without_following_symlinks(func, path, *args): + # Pass follow_symlinks=False, unless not supported on this platform. + if func in _os.supports_follow_symlinks: + func(path, *args, follow_symlinks=False) + elif not _os.path.islink(path): + func(path, *args) + + def resetperms(path): + try: + without_following_symlinks(_os.chflags, path, 0) + except AttributeError: + pass + without_following_symlinks(_os.chmod, path, 0o700) + def onerror(func, path, exc_info): if issubclass(exc_info[0], PermissionError): - def resetperms(path): - try: - _os.chflags(path, 0) - except AttributeError: - pass - _os.chmod(path, 0o700) - try: if path != name: resetperms(_os.path.dirname(path)) diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 2319ce3747ff099..c022e2e0f8d1144 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1652,6 +1652,45 @@ def test_cleanup_with_symlink_to_a_directory(self): "were deleted") d2.cleanup() + @os_helper.skip_unless_symlink + @os_helper.skip_unless_working_chmod + @unittest.skipUnless(os.chmod in os.supports_follow_symlinks, 'needs chmod follow_symlinks support') + def test_cleanup_with_error_deleting_symlink(self): + # cleanup() should not follow symlinks when fixing mode bits etc. (#91133) + d1 = self.do_create() + d2 = self.do_create(recurse=0) + + # Symlink d1/my_symlink -> d2, then give d2 a custom mode to see if changes. + os.symlink(d2.name, os.path.join(d1.name, "my_symlink")) + os.chmod(d2.name, 0o567) + expected_mode = os.stat(d2.name).st_mode # can be impacted by umask etc. + + # There are a variety of reasons why the OS may raise a PermissionError, + # but provoking those reliably and cross-platform is not straightforward, + # so raise the error synthetically instead. + real_unlink = os.unlink + error_was_raised = False + def patched_unlink(path, **kwargs): + nonlocal error_was_raised + # unlink may be called with full path or path relative to 'fd' kwarg. + if path.endswith("my_symlink") and not error_was_raised: + error_was_raised = True + raise PermissionError() + real_unlink(path, **kwargs) + + with mock.patch("tempfile._os.unlink", patched_unlink): + # This call to cleanup() should not follow my_symlink when fixing permissions + d1.cleanup() + + self.assertTrue(error_was_raised, "did not see expected 'unlink' call") + self.assertFalse(os.path.exists(d1.name), + "TemporaryDirectory %s exists after cleanup" % d1.name) + self.assertTrue(os.path.exists(d2.name), + "Directory pointed to by a symlink was deleted") + self.assertEqual(os.stat(d2.name).st_mode, expected_mode, + "Mode of the directory pointed to by a symlink changed") + d2.cleanup() + @support.cpython_only def test_del_on_collection(self): # A TemporaryDirectory is deleted when garbage collected