From d8f50e5e191027ad3358bafe6828a1263d3c0cda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20L=C3=B8vborg?= Date: Thu, 1 Dec 2022 14:30:26 +0100 Subject: [PATCH 01/12] tempfile tests: try not to leave NOUNLINK files behind During development, it becomes tiresome to have to manually clean up these files in case of unrelated TemporaryDirectory breakage. --- Lib/test/test_tempfile.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 7c2c8de7a2e6fc..2319ce3747ff09 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1826,17 +1826,28 @@ def test_modes(self): @unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.lchflags') def test_flags(self): - flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK - d = self.do_create(recurse=3, dirs=2, files=2) - with d: - # Change files and directories flags recursively. - for root, dirs, files in os.walk(d.name, topdown=False): + def chflags_recursively(name, flags): + for root, dirs, files in os.walk(name, topdown=False): for name in files: os.chflags(os.path.join(root, name), flags) os.chflags(root, flags) - d.cleanup() - self.assertFalse(os.path.exists(d.name)) + d = self.do_create(recurse=3, dirs=2, files=2) + try: + with d: + chflags_recursively(d.name, stat.UF_IMMUTABLE | stat.UF_NOUNLINK) + d.cleanup() + + self.assertFalse(os.path.exists(d.name)) + + finally: + # Even if test fails, make a best-effort attempt at not leaving + # behind NOUNLINK files, because they require manual removal, + # which becomes tiresome during development. + try: + chflags_recursively(d.name, 0) + except Exception: + pass # we tried if __name__ == "__main__": unittest.main() From d268947f7a846a7b91df054a4185b215dc794038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20L=C3=B8vborg?= Date: Thu, 1 Dec 2022 14:10:41 +0100 Subject: [PATCH 02/12] gh-91133: tempfile.TemporaryDirectory: fix symlink bug in cleanup --- Lib/tempfile.py | 21 ++++++---- Lib/test/test_tempfile.py | 39 +++++++++++++++++++ ...2-12-01-16-57-44.gh-issue-91133.LKMVCV.rst | 2 + 3 files changed, 55 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst diff --git a/Lib/tempfile.py b/Lib/tempfile.py index bb18d60db0d919..766cc63a98a1c0 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 2319ce3747ff09..c022e2e0f8d114 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 diff --git a/Misc/NEWS.d/next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst b/Misc/NEWS.d/next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst new file mode 100644 index 00000000000000..afa02423ed3d41 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst @@ -0,0 +1,2 @@ +Fix a bug `:class:`tempfile.TemporaryDirectory` cleanup, which now no longer +dereferences symlinks when working around file system permission errors. From 60de13ae894b26bdb4926e1e1c520e78778e901a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20L=C3=B8vborg?= Date: Thu, 1 Dec 2022 17:11:03 +0100 Subject: [PATCH 03/12] NEWS: fix blurb typo --- .../next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst b/Misc/NEWS.d/next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst index afa02423ed3d41..131aa2b75e25da 100644 --- a/Misc/NEWS.d/next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst +++ b/Misc/NEWS.d/next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst @@ -1,2 +1,2 @@ -Fix a bug `:class:`tempfile.TemporaryDirectory` cleanup, which now no longer +Fix a bug :class:`tempfile.TemporaryDirectory` cleanup, which now no longer dereferences symlinks when working around file system permission errors. From 983a9ac95d4cf9cc60e50f867263680a9c230b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20L=C3=B8vborg?= Date: Thu, 15 Dec 2022 12:02:09 +0100 Subject: [PATCH 04/12] Revert "tempfile tests: try not to leave NOUNLINK files behind" This reverts commit d8f50e5e191027ad3358bafe6828a1263d3c0cda. --- Lib/test/test_tempfile.py | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index c022e2e0f8d114..e6c89beb48a452 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1865,28 +1865,17 @@ def test_modes(self): @unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.lchflags') def test_flags(self): - def chflags_recursively(name, flags): - for root, dirs, files in os.walk(name, topdown=False): + flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK + d = self.do_create(recurse=3, dirs=2, files=2) + with d: + # Change files and directories flags recursively. + for root, dirs, files in os.walk(d.name, topdown=False): for name in files: os.chflags(os.path.join(root, name), flags) os.chflags(root, flags) + d.cleanup() + self.assertFalse(os.path.exists(d.name)) - d = self.do_create(recurse=3, dirs=2, files=2) - try: - with d: - chflags_recursively(d.name, stat.UF_IMMUTABLE | stat.UF_NOUNLINK) - d.cleanup() - - self.assertFalse(os.path.exists(d.name)) - - finally: - # Even if test fails, make a best-effort attempt at not leaving - # behind NOUNLINK files, because they require manual removal, - # which becomes tiresome during development. - try: - chflags_recursively(d.name, 0) - except Exception: - pass # we tried if __name__ == "__main__": unittest.main() From 5b83afea44f86b5eda9c67dbed32d28c92a2aa54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20L=C3=B8vborg?= Date: Thu, 15 Dec 2022 12:04:14 +0100 Subject: [PATCH 05/12] Apply suggestions from code review Co-authored-by: Carl Meyer --- Lib/test/test_tempfile.py | 2 +- .../next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index e6c89beb48a452..58cdcb1a2a06da 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1660,7 +1660,7 @@ def test_cleanup_with_error_deleting_symlink(self): 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. + # Symlink d1/my_symlink -> d2, then give d2 a custom mode to see if it 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. diff --git a/Misc/NEWS.d/next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst b/Misc/NEWS.d/next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst index 131aa2b75e25da..7991048fc48e03 100644 --- a/Misc/NEWS.d/next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst +++ b/Misc/NEWS.d/next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst @@ -1,2 +1,2 @@ -Fix a bug :class:`tempfile.TemporaryDirectory` cleanup, which now no longer +Fix a bug in :class:`tempfile.TemporaryDirectory` cleanup, which now no longer dereferences symlinks when working around file system permission errors. From db40615c2fe3a44b31d6f043b9805c0bdb5b4715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20L=C3=B8vborg?= Date: Thu, 15 Dec 2022 18:59:06 +0100 Subject: [PATCH 06/12] Run test even when chmod doesn't support follow_symlinks --- Lib/test/test_tempfile.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 58cdcb1a2a06da..16a52e2a67d815 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1654,7 +1654,6 @@ def test_cleanup_with_symlink_to_a_directory(self): @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() From cb9aea4e61da138893c763a612c5de3e935e9bc0 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 6 Dec 2023 16:03:08 +0200 Subject: [PATCH 07/12] Rewrite tests. --- Lib/test/test_tempfile.py | 79 ++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 34 deletions(-) diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index a8321c85b9b922..a51f09ea43e050 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1674,42 +1674,53 @@ def test_cleanup_with_symlink_to_a_directory(self): d2.cleanup() @os_helper.skip_unless_symlink - @os_helper.skip_unless_working_chmod 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 it 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() + with self.do_create(recurse=0) as d2: + file1 = os.path.join(d2, 'file1') + open(file1, 'wb').close() + dir1 = os.path.join(d2, 'dir1') + os.mkdir(dir1) + for mode in range(8): + mode <<= 6 + with self.subTest(mode=format(mode, '03o')): + def test(target, target_is_directory): + d1 = self.do_create(recurse=0) + symlink = os.path.join(d1.name, 'symlink') + os.symlink(target, symlink, + target_is_directory=target_is_directory) + try: + os.chmod(symlink, mode, follow_symlinks=False) + except NotImplementedError: + pass + try: + os.chmod(symlink, mode) + except FileNotFoundError: + pass + os.chmod(d1.name, mode) + d1.cleanup() + self.assertFalse(os.path.exists(d1.name)) + + with self.subTest('nonexisting file'): + test('nonexisting', target_is_directory=False) + with self.subTest('nonexisting dir'): + test('nonexisting', target_is_directory=True) + + with self.subTest('existing file'): + os.chmod(file1, mode) + old_mode = os.stat(file1).st_mode + test(file1, target_is_directory=False) + new_mode = os.stat(file1).st_mode + self.assertEqual(new_mode, old_mode, + '%03o != %03o' % (new_mode, old_mode)) + + with self.subTest('existing dir'): + os.chmod(dir1, mode) + old_mode = os.stat(dir1).st_mode + test(dir1, target_is_directory=True) + new_mode = os.stat(dir1).st_mode + self.assertEqual(new_mode, old_mode, + '%03o != %03o' % (new_mode, old_mode)) @support.cpython_only def test_del_on_collection(self): From f106e25c269f823cf0e5abd826df6c9c2578a47c Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 6 Dec 2023 21:32:30 +0200 Subject: [PATCH 08/12] Fix on Windows. --- Lib/tempfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/tempfile.py b/Lib/tempfile.py index c7621936e68681..930ea725b9b210 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -878,7 +878,7 @@ 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): + elif _os.name == 'nt' or not _os.path.islink(path): func(path, *args) def resetperms(path): From 0be03d13307ebe9483efcaeae09496e00a13874e Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 6 Dec 2023 22:08:51 +0200 Subject: [PATCH 09/12] Add a test for file flags. --- Lib/test/test_tempfile.py | 60 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index a51f09ea43e050..2c5e597666402a 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1674,8 +1674,8 @@ def test_cleanup_with_symlink_to_a_directory(self): d2.cleanup() @os_helper.skip_unless_symlink - def test_cleanup_with_error_deleting_symlink(self): - # cleanup() should not follow symlinks when fixing mode bits etc. (#91133) + def test_cleanup_with_symlink_modes(self): + # cleanup() should not follow symlinks when fixing mode bits (#91133) with self.do_create(recurse=0) as d2: file1 = os.path.join(d2, 'file1') open(file1, 'wb').close() @@ -1722,6 +1722,54 @@ def test(target, target_is_directory): self.assertEqual(new_mode, old_mode, '%03o != %03o' % (new_mode, old_mode)) + @unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags') + @os_helper.skip_unless_symlink + def test_cleanup_with_symlink_flags(self): + # cleanup() should not follow symlinks when fixing flags (#91133) + flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK + self.check_flags() + + with self.do_create(recurse=0) as d2: + file1 = os.path.join(d2, 'file1') + open(file1, 'wb').close() + dir1 = os.path.join(d2, 'dir1') + os.mkdir(dir1) + def test(target, target_is_directory): + d1 = self.do_create(recurse=0) + symlink = os.path.join(d1.name, 'symlink') + os.symlink(target, symlink, + target_is_directory=target_is_directory) + try: + os.chflags(symlink, flags, follow_symlinks=False) + except NotImplementedError: + pass + try: + os.chflags(symlink, flags) + except FileNotFoundError: + pass + os.chflags(d1.name, flags) + d1.cleanup() + self.assertFalse(os.path.exists(d1.name)) + + with self.subTest('nonexisting file'): + test('nonexisting', target_is_directory=False) + with self.subTest('nonexisting dir'): + test('nonexisting', target_is_directory=True) + + with self.subTest('existing file'): + os.chflags(file1, flags) + old_flags = os.stat(file1).st_flags + test(file1, target_is_directory=False) + new_flags = os.stat(file1).st_flags + self.assertEqual(new_flags, old_flags) + + with self.subTest('existing dir'): + os.chflags(dir1, flags) + old_flags = os.stat(dir1).st_flags + test(dir1, target_is_directory=True) + new_flags = os.stat(dir1).st_flags + self.assertEqual(new_flags, old_flags) + @support.cpython_only def test_del_on_collection(self): # A TemporaryDirectory is deleted when garbage collected @@ -1894,8 +1942,7 @@ def test_modes(self): d.cleanup() self.assertFalse(os.path.exists(d.name)) - @unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags') - def test_flags(self): + def check_flags(self): flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK # skip the test if these flags are not supported (ex: FreeBSD 13) @@ -1913,6 +1960,11 @@ def test_flags(self): finally: os_helper.unlink(filename) + @unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags') + def test_flags(self): + flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK + self.check_flags() + d = self.do_create(recurse=3, dirs=2, files=2) with d: # Change files and directories flags recursively. From 0a05de044d57bd671327530d8788000b826403f5 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 7 Dec 2023 11:24:08 +0200 Subject: [PATCH 10/12] Address review comments. --- Lib/tempfile.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/Lib/tempfile.py b/Lib/tempfile.py index 930ea725b9b210..1a61c7787ee5c8 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -270,6 +270,20 @@ def _mkstemp_inner(dir, pre, suf, flags, output_type): raise FileExistsError(_errno.EEXIST, "No usable temporary file name found") +def _dont_follow_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 _os.name == 'nt' or not _os.path.islink(path): + func(path, *args) + +def _resetperms(path): + try: + _dont_follow_symlinks(_os.chflags, path, 0) + except AttributeError: + pass + _dont_follow_symlinks(_os.chmod, path, 0o700) + # User visible interfaces. @@ -874,26 +888,12 @@ 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 _os.name == 'nt' or 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 onexc(func, path, exc): if isinstance(exc, PermissionError): try: if path != name: - resetperms(_os.path.dirname(path)) - resetperms(path) + _resetperms(_os.path.dirname(path)) + _resetperms(path) try: _os.unlink(path) From 58924b654ba42a8ce8f8a8ea0fb3ff4765ddad0a Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 7 Dec 2023 15:29:17 +0200 Subject: [PATCH 11/12] Update Lib/tempfile.py Co-authored-by: Victor Stinner --- Lib/tempfile.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/tempfile.py b/Lib/tempfile.py index 1a61c7787ee5c8..9a5e7d01c2379b 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -279,9 +279,11 @@ def _dont_follow_symlinks(func, path, *args): def _resetperms(path): try: - _dont_follow_symlinks(_os.chflags, path, 0) + chflags = _os.chflags except AttributeError: pass + else: + _dont_follow_symlinks(chflags, path, 0) _dont_follow_symlinks(_os.chmod, path, 0o700) From b88478f8161d4edef41914bd27f1986e4713f8dc Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 7 Dec 2023 16:52:35 +0200 Subject: [PATCH 12/12] Address review comments. --- Lib/test/test_tempfile.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 2c5e597666402a..2729bec7a21c71 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1727,7 +1727,7 @@ def test(target, target_is_directory): def test_cleanup_with_symlink_flags(self): # cleanup() should not follow symlinks when fixing flags (#91133) flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK - self.check_flags() + self.check_flags(flags) with self.do_create(recurse=0) as d2: file1 = os.path.join(d2, 'file1') @@ -1942,9 +1942,7 @@ def test_modes(self): d.cleanup() self.assertFalse(os.path.exists(d.name)) - def check_flags(self): - flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK - + def check_flags(self, flags): # skip the test if these flags are not supported (ex: FreeBSD 13) filename = os_helper.TESTFN try: @@ -1953,8 +1951,8 @@ def check_flags(self): os.chflags(filename, flags) except OSError as exc: # "OSError: [Errno 45] Operation not supported" - self.skipTest(f"chflags() doesn't support " - f"UF_IMMUTABLE|UF_NOUNLINK: {exc}") + self.skipTest(f"chflags() doesn't support flags " + f"{flags:#b}: {exc}") else: os.chflags(filename, 0) finally: @@ -1963,7 +1961,7 @@ def check_flags(self): @unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags') def test_flags(self): flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK - self.check_flags() + self.check_flags(flags) d = self.do_create(recurse=3, dirs=2, files=2) with d: