8000 [3.11] gh-91133: tempfile.TemporaryDirectory: fix symlink bug in clea… · python/cpython@5585334 · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 5585334

Browse files
serhiy-storchakakwi-dk
serhiy-storchaka
and
kwi-dk
authored
[3.11] gh-91133: tempfile.TemporaryDirectory: fix symlink bug in cleanup (GH-99930) (GH-112839)
(cherry picked from commit 81c16cd) Co-authored-by: Søren Løvborg <sorenl@unity3d.com>
1 parent 666a484 commit 5585334

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

@@ -863,17 +879,10 @@ def __init__(self, suffix=None, prefix=None, dir=None,
863879
def _rmtree(cls, name, ignore_errors=False):
864880
def onerror(func, path, exc_info):
865881
if issubclass(exc_info[0], PermissionError):
866-
def resetperms(path):
867-
try:
868-
_os.chflags(path, 0)
869-
except AttributeError:
870-
pass
871-
_os.chmod(path, 0o700)
872-
873882
try:
874883
if path != name:
875-
resetperms(_os.path.dirname(path))
876-
resetperms(path)
884+
_resetperms(_os.path.dirname(path))
885+
_resetperms(path)
877886

878887
try:
879888
_os.unlink(path)

‎Lib/test/test_tempfile.py

Lines changed: 105 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,6 +1565,103 @@ def test_cleanup_with_symlink_to_a_directory(self):
15651565
"were deleted")
15661566
d2.cleanup()
15671567

1568+
@os_helper.skip_unless_symlink
1569+
def test_cleanup_with_symlink_modes(self):
1570+
# cleanup() should not follow symlinks when fixing mode bits (#91133)
1571+
with self.do_create(recurse=0) as d2:
1572+
file1 = os.path.join(d2, 'file1')
1573+
open(file1, 'wb').close()
1574+
dir1 = os.path.join(d2, 'dir1')
1575+
os.mkdir(dir1)
1576+
for mode in range(8):
1577+
mode <<= 6
1578+
with self.subTest(mode=format(mode, '03o')):
1579+
def test(target, target_is_directory):
1580+
d1 = self.do_create(recurse=0)
1581+
symlink = os.path.join(d1.name, 'symlink')
1582+
os.symlink(target, symlink,
1583+
target_is_directory=target_is_directory)
1584+
try:
1585+
os.chmod(symlink, mode, follow_symlinks=False)
1586+
except NotImplementedError:
1587+
pass
1588+
try:
1589+
os.chmod(symlink, mode)
1590+
except FileNotFoundError:
1591+
pass
1592+
os.chmod(d1.name, mode)
1593+
d1.cleanup()
1594+
self.assertFalse(os.path.exists(d1.name))
1595+
1596+
with self.subTest('nonexisting file'):
1597+
test('nonexisting', target_is_directory=False)
1598+
with self.subTest('nonexisting dir'):
1599+
test('nonexisting', target_is_directory=True)
1600+
1601+
with self.subTest('existing file'):
1602+
os.chmod(file1, mode)
1603+
old_mode = os.stat(file1).st_mode
1604+
test(file1, target_is_directory=False)
1605+
new_mode = os.stat(file1).st_mode
1606+
self.assertEqual(new_mode, old_mode,
1607+
'%03o != %03o' % (new_mode, old_mode))
1608+
1609+
with self.subTest('existing dir'):
1610+
os.chmod(dir1, mode)
1611+
old_mode = os.stat(dir1).st_mode
1612+
test(dir1, target_is_directory=True)
1613+
new_mode = os.stat(dir1).st_mode
1614+
self.assertEqual(new_mode, old_mode,
1615+
'%03o != %03o' % (new_mode, old_mode))
1616+
1617+
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags')
1618+
@os_helper.skip_unless_symlink
1619+
def test_cleanup_with_symlink_flags(self):
1620+
# cleanup() should not follow symlinks when fixing flags (#91133)
1621+
flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK
1622+
self.check_flags(flags)
1623+
1624+
with self.do_create(recurse=0) as d2:
1625+
file1 = os.path.join(d2, 'file1')
1626+
open(file1, 'wb').close()
1627+
dir1 = os.path.join(d2, 'dir1')
1628+
os.mkdir(dir1)
1629+
def test(target, target_is_directory):
1630+
d1 = self.do_create(recurse=0)
1631+
symlink = os.path.join(d1.name, 'symlink')
1632+
os.symlink(target, symlink,
1633+
target_is_directory=target_is_directory)
1634+
try:
1635+
os.chflags(symlink, flags, follow_symlinks=False)
1636+
except NotImplementedError:
1637+
pass
1638+
try:
1639+
os.chflags(symlink, flags)
1640+
except FileNotFoundError:
1641+
pass
1642+
os.chflags(d1.name, flags)
1643+
d1.cleanup()
1644+
self.assertFalse(os.path.exists(d1.name))
1645+
1646+
with self.subTest('nonexisting file'):
1647+
test('nonexisting', target_is_directory=False)
1648+
with self.subTest('nonexisting dir'):
1649+
test('nonexisting', target_is_directory=True)
1650+
1651+
with self.subTest('existing file'):
1652+
os.chflags(file1, flags)
1653+
old_flags = os.stat(file1).st_flags
1654+
test(file1, target_is_directory=False)
1655+
new_flags = os.stat(file1).st_flags
1656+
self.assertEqual(new_flags, old_flags)
1657+
1658+
with self.subTest('existing dir'):
1659+
os.chflags(dir1, flags)
1660+
old_flags = os.stat(dir1).st_flags
1661+
test(dir1, target_is_directory=True)
1662+
new_flags = os.stat(dir1).st_flags
1663+
self.assertEqual(new_flags, old_flags)
1664+
15681665
@support.cpython_only
15691666
def test_del_on_collection(self):
15701667
# A TemporaryDirectory is deleted when garbage collected
@@ -1737,10 +1834,7 @@ def test_modes(self):
17371834
d.cleanup()
17381835
self.assertFalse(os.path.exists(d.name))
17391836

1740-
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags')
1741-
def test_flags(self):
1742-
flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK
1743-
1837+
def check_flags(self, flags):
17441838
# skip the test if these flags are not supported (ex: FreeBSD 13)
17451839
filename = os_helper.TESTFN
17461840
try:
@@ -1749,13 +1843,18 @@ def test_flags(self):
17491843
os.chflags(filename, flags)
17501844
except OSError as exc:
17511845
# "OSError: [Errno 45] Operation not supported"
1752-
self.skipTest(f"chflags() doesn't support "
1753-
f"UF_IMMUTABLE|UF_NOUNLINK: {exc}")
1846+
self.skipTest(f"chflags() doesn't support flags "
1847+
f"{flags:#b}: {exc}")
17541848
else:
17551849
os.chflags(filename, 0)
17561850
finally:
17571851
os_helper.unlink(filename)
17581852

1853+
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags')
1854+
def test_flags(self):
1855+
flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK
1856+
self.check_flags(flags)
1857+
17591858
d = self.do_create(recurse=3, dirs=2, files=2)
17601859
with d:
17611860
# 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