8000 gh-91133: tempfile.TemporaryDirectory: fix symlink bug in cleanup (GH… · python/cpython@81c16cd · GitHub
[go: up one dir, main page]
Skip to content

Commit 81c16cd

Browse files
kwi-dkserhiy-storchaka
kwi-dk
and
serhiy-storchaka
authored
gh-91133: tempfile.TemporaryDirectory: fix symlink bug in cleanup (GH-99930)
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
1 parent 21221c3 commit 81c16cd

File tree

3 files changed

+125
-15
lines changed

3 files changed

+125
-15
lines changed

‎Lib/tempfile.py

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,22 @@ def _mkstemp_inner(dir, pre, suf, flags, output_type):
270270
raise FileExistsError(_errno.EEXIST,
271271
"No usable temporary file name found")
272272

273+
def _dont_follow_symlinks(func, path, *args):
274+
# Pass follow_symlinks=False, unless not supported on this platform.
275+
if func in _os.supports_follow_symlinks:
276+
func(path, *args, follow_symlinks=False)
277+
elif _os.name == 'nt' or not _os.path.islink(path):
278+
func(path, *args)
279+
280+
def _resetperms(path):
281+
try:
282+
chflags = _os.chflags
283+
except AttributeError:
284+
pass
285+
else:
286+
_dont_follow_symlinks(chflags, path, 0)
287+
_dont_follow_symlinks(_os.chmod, path, 0o700)
288+
273289

274290
# User visible interfaces.
275291

@@ -876,17 +892,10 @@ def __init__(self, suffix=None, prefix=None, dir=None,
876892
def _rmtree(cls, name, ignore_errors=False):
877893
def onexc(func, path, exc):
878894
if isinstance(exc, PermissionError):
879-
def resetperms(path):
880-
try:
881-
_os.chflags(path, 0)
882-
except AttributeError:
883-
pass
884-
_os.chmod(path, 0o700)
885-
886895
try:
887896
if path != name:
888-
resetperms(_os.path.dirname(path))
889-
resetperms(path)
897+
_resetperms(_os.path.dirname(path))
898+
_resetperms(path)
890899

891900
try:
892901
_os.unlink(path)

‎Lib/test/test_tempfile.py

Lines changed: 105 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1673,6 +1673,103 @@ def test_cleanup_with_symlink_to_a_directory(self):
16731673
"were deleted")
16741674
d2.cleanup()
16751675

1676+
@os_helper.skip_unless_symlink
1677+
def test_cleanup_with_symlink_modes(self):
1678+
# cleanup() should not follow symlinks when fixing mode bits (#91133)
1679+
with self.do_create(recurse=0) as d2:
1680+
file1 = os.path.join(d2, 'file1')
1681+
open(file1, 'wb').close()
1682+
dir1 = os.path.join(d2, 'dir1')
1683+
os.mkdir(dir1)
1684+
for mode in range(8):
1685+
mode <<= 6
1686+
with self.subTest(mode=format(mode, '03o')):
1687+
def test(target, target_is_directory):
1688+
d1 = self.do_create(recurse=0)
1689+
symlink = os.path.join(d1.name, 'symlink')
1690+
os.symlink(target, symlink,
1691+
target_is_directory=target_is_directory)
1692+
try:
1693+
os.chmod(symlink, mode, follow_symlinks=False)
1694+
except NotImplementedError:
1695+
pass
1696+
try:
1697+
os.chmod(symlink, mode)
1698+
except FileNotFoundError:
1699+
pass
1700+
os.chmod(d1.name, mode)
1701+
d1.cleanup()
1702+
self.assertFalse(os.path.exists(d1.name))
1703+
1704+
with self.subTest('nonexisting file'):
1705+
test('nonexisting', target_is_directory=False)
1706+
with self.subTest('nonexisting dir'):
1707+
test('nonexisting', target_is_directory=True)
1708+
1709+
with self.subTest('existing file'):
1710+
os.chmod(file1, mode)
1711+
old_mode = os.stat(file1).st_mode
1712+
test(file1, target_is_directory=False)
1713+
new_mode = os.stat(file1).st_mode
1714+
self.assertEqual(new_mode, old_mode,
1715+
'%03o != %03o' % (new_mode, old_mode))
1716+
1717+
with self.subTest('existing dir'):
1718+
os.chmod(dir1, mode)
1719+
old_mode = os.stat(dir1).st_mode
1720+
test(dir1, target_is_directory=True)
1721+
new_mode = os.stat(dir1).st_mode
1722+
self.assertEqual(new_mode, old_mode,
1723+
'%03o != %03o' % (new_mode, old_mode))
1724+
1725+
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags')
1726+
@os_helper.skip_unless_symlink
1727+
def test_cleanup_with_symlink_flags(self):
1728+
# cleanup() should not follow symlinks when fixing flags (#91133)
1729+
flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK
1730+
self.check_flags(flags)
1731+
1732+
with self.do_create(recurse=0) as d2:
1733+
file1 = os.path.join(d2, 'file1')
1734+
open(file1, 'wb').close()
1735+
dir1 = os.path.join(d2, 'dir1')
1736+
os.mkdir(dir1)
1737+
def test(target, target_is_directory):
1738+
d1 = self.do_create(recurse=0)
1739+
symlink = os.path.join(d1.name, 'symlink')
1740+
os.symlink(target, symlink,
1741+
target_is_directory=target_is_directory)
1742+
try:
1743+
os.chflags(symlink, flags, follow_symlinks=False)
1744+
except NotImplementedError:
1745+
pass
1746+
try:
1747+
os.chflags(symlink, flags)
1748+
except FileNotFoundError:
1749+
pass
1750+
os.chflags(d1.name, flags)
1751+
d1.cleanup()
1752+
self.assertFalse(os.path.exists(d1.name))
1753+
1754+
with self.subTest('nonexisting file'):
1755+
test('nonexisting', target_is_directory=False)
1756+
with self.subTest('nonexisting dir'):
1757+
test('nonexisting', target_is_directory=True)
1758+
1759+
with self.subTest('existing file'):
1760+
os.chflags(file1, flags)
1761+
old_flags = os.stat(file1).st_flags
1762+
test(file1, target_is_directory=False)
1763+
new_flags = os.stat(file1).st_flags
1764+
self.assertEqual(new_flags, old_flags)
1765+
1766+
with self.subTest('existing dir'):
1767+
os.chflags(dir1, flags)
1768+
old_flags = os.stat(dir1).st_flags
1769+
test(dir1, target_is_directory=True)
1770+
new_flags = os.stat(dir1).st_flags
1771+
self.assertEqual(new_flags, old_flags)
1772+
16761773
@support.cpython_only
16771774
def test_del_on_collection(self):
16781775
# A TemporaryDirectory is deleted when garbage collected
@@ -1845,10 +1942,7 @@ def test_modes(self):
18451942
d.cleanup()
18461943
self.assertFalse(os.path.exists(d.name))
18471944

1848-
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags')
1849-
def test_flags(self):
1850-
flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK
1851-
1945+
def check_flags(self, flags):
18521946
# skip the test if these flags are not supported (ex: FreeBSD 13)
18531947
filename = os_helper.TESTFN
18541948
try:
@@ -1857,13 +1951,18 @@ def test_flags(self):
18571951
os.chflags(filename, flags)
18581952
except OSError as exc:
18591953
# "OSError: [Errno 45] Operation not supported"
1860-
self.skipTest(f"chflags() doesn't support "
1861-
f"UF_IMMUTABLE|UF_NOUNLINK: {exc}")
1954+
self.skipTest(f"chflags() doesn't support flags "
1955+
f"{flags:#b}: {exc}")
18621956
else:
18631957
os.chflags(filename, 0)
18641958
finally:
18651959
os_helper.unlink(filename)
18661960

1961+
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags')
1962+
def test_flags(self):
1963+
flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK
1964+
self.check_flags(flags)
1965+
18671966
d = self.do_create(recurse=3, dirs=2, files=2)
18681967
with d:
18691968
# Change files and directories flags recursively.

‎Misc/NEWS.d/next/Library/2022-12-01-16-57-44.gh-issue-91133.LKMVCV.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a bug in :class:`tempfile.TemporaryDirectory` cleanup, which now no longer
2+
dereferences symlinks when working around file system permission errors.

0 commit comments

Comments
 (0)
0