Skip to content

Commit 8eaeefe

Browse files
[3.10] gh-91133: tempfile.TemporaryDirectory: fix symlink bug in cleanup (GH-99930) (GH-112840)
(cherry picked from commit 81c16cd) Co-authored-by: Søren Løvborg <sorenl@unity3d.com>
1 parent 32e7acd commit 8eaeefe

File tree

3 files changed

+136
-10
lines changed

3 files changed

+136
-10
lines changed

Lib/tempfile.py

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

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

273289
# User visible interfaces.
274290

@@ -827,17 +843,10 @@ def __init__(self, suffix=None, prefix=None, dir=None,
827843
def _rmtree(cls, name, ignore_errors=False):
828844
def onerror(func, path, exc_info):
829845
if issubclass(exc_info[0], PermissionError):
830-
def resetperms(path):
831-
try:
832-
_os.chflags(path, 0)
833-
except AttributeError:
834-
pass
835-
_os.chmod(path, 0o700)
836-
837846
try:
838847
if path != name:
839-
resetperms(_os.path.dirname(path))
840-
resetperms(path)
848+
_resetperms(_os.path.dirname(path))
849+
_resetperms(path)
841850

842851
try:
843852
_os.unlink(path)

Lib/test/test_tempfile.py

+116-1
Original file line numberDiff line numberDiff line change
@@ -1499,6 +1499,103 @@ def test_cleanup_with_symlink_to_a_directory(self):
14991499
"were deleted")
15001500
d2.cleanup()
15011501

1502+
@os_helper.skip_unless_symlink
1503+
def test_cleanup_with_symlink_modes(self):
1504+
# cleanup() should not follow symlinks when fixing mode bits (#91133)
1505+
with self.do_create(recurse=0) as d2:
1506+
file1 = os.path.join(d2, 'file1')
1507+
open(file1, 'wb').close()
1508+
dir1 = os.path.join(d2, 'dir1')
1509+
os.mkdir(dir1)
1510+
for mode in range(8):
1511+
mode <<= 6
1512+
with self.subTest(mode=format(mode, '03o')):
1513+
def test(target, target_is_directory):
1514+
d1 = self.do_create(recurse=0)
1515+
symlink = os.path.join(d1.name, 'symlink')
1516+
os.symlink(target, symlink,
1517+
target_is_directory=target_is_directory)
1518+
try:
1519+
os.chmod(symlink, mode, follow_symlinks=False)
1520+
except NotImplementedError:
1521+
pass
1522+
try:
1523+
os.chmod(symlink, mode)
1524+
except FileNotFoundError:
1525+
pass
1526+
os.chmod(d1.name, mode)
1527+
d1.cleanup()
1528+
self.assertFalse(os.path.exists(d1.name))
1529+
1530+
with self.subTest('nonexisting file'):
1531+
test('nonexisting', target_is_directory=False)
1532+
with self.subTest('nonexisting dir'):
1533+
test('nonexisting', target_is_directory=True)
1534+
1535+
with self.subTest('existing file'):
1536+
os.chmod(file1, mode)
1537+
old_mode = os.stat(file1).st_mode
1538+
test(file1, target_is_directory=False)
1539+
new_mode = os.stat(file1).st_mode
1540+
self.assertEqual(new_mode, old_mode,
1541+
'%03o != %03o' % (new_mode, old_mode))
1542+
1543+
with self.subTest('existing dir'):
1544+
os.chmod(dir1, mode)
1545+
old_mode = os.stat(dir1).st_mode
1546+
test(dir1, target_is_directory=True)
1547+
new_mode = os.stat(dir1).st_mode
1548+
self.assertEqual(new_mode, old_mode,
1549+
'%03o != %03o' % (new_mode, old_mode))
1550+
1551+
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags')
1552+
@os_helper.skip_unless_symlink
1553+
def test_cleanup_with_symlink_flags(self):
1554+
# cleanup() should not follow symlinks when fixing flags (#91133)
1555+
flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK
1556+
self.check_flags(flags)
1557+
1558+
with self.do_create(recurse=0) as d2:
1559+
file1 = os.path.join(d2, 'file1')
1560+
open(file1, 'wb').close()
1561+
dir1 = os.path.join(d2, 'dir1')
1562+
os.mkdir(dir1)
1563+
def test(target, target_is_directory):
1564+
d1 = self.do_create(recurse=0)
1565+
symlink = os.path.join(d1.name, 'symlink')
1566+
os.symlink(target, symlink,
1567+
target_is_directory=target_is_directory)
1568+
try:
1569+
os.chflags(symlink, flags, follow_symlinks=False)
1570+
except NotImplementedError:
1571+
pass
1572+
try:
1573+
os.chflags(symlink, flags)
1574+
except FileNotFoundError:
1575+
pass
1576+
os.chflags(d1.name, flags)
1577+
d1.cleanup()
1578+
self.assertFalse(os.path.exists(d1.name))
1579+
1580+
with self.subTest('nonexisting file'):
1581+
test('nonexisting', target_is_directory=False)
1582+
with self.subTest('nonexisting dir'):
1583+
test('nonexisting', target_is_directory=True)
1584+
1585+
with self.subTest('existing file'):
1586+
os.chflags(file1, flags)
1587+
old_flags = os.stat(file1).st_flags
1588+
test(file1, target_is_directory=False)
1589+
new_flags = os.stat(file1).st_flags
1590+
self.assertEqual(new_flags, old_flags)
1591+
1592+
with self.subTest('existing dir'):
1593+
os.chflags(dir1, flags)
1594+
old_flags = os.stat(dir1).st_flags
1595+
test(dir1, target_is_directory=True)
1596+
new_flags = os.stat(dir1).st_flags
1597+
self.assertEqual(new_flags, old_flags)
1598+
15021599
@support.cpython_only
15031600
def test_del_on_collection(self):
15041601
# A TemporaryDirectory is deleted when garbage collected
@@ -1671,9 +1768,27 @@ def test_modes(self):
16711768
d.cleanup()
16721769
self.assertFalse(os.path.exists(d.name))
16731770

1674-
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.lchflags')
1771+
def check_flags(self, flags):
1772+
# skip the test if these flags are not supported (ex: FreeBSD 13)
1773+
filename = os_helper.TESTFN
1774+
try:
1775+
open(filename, "w").close()
1776+
try:
1777+
os.chflags(filename, flags)
1778+
except OSError as exc:
1779+
# "OSError: [Errno 45] Operation not supported"
1780+
self.skipTest(f"chflags() doesn't support flags "
1781+
f"{flags:#b}: {exc}")
1782+
else:
1783+
os.chflags(filename, 0)
1784+
finally:
1785+
os_helper.unlink(filename)
1786+
1787+
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags')
16751788
def test_flags(self):
16761789
flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK
1790+
self.check_flags(flags)
1791+
16771792
d = self.do_create(recurse=3, dirs=2, files=2)
16781793
with d:
16791794
# Change files and directories flags recursively.
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)