Skip to content

Commit d54e22a

Browse files
[3.9] gh-91133: tempfile.TemporaryDirectory: fix symlink bug in cleanup (GH-99930) (GH-112842)
(cherry picked from commit 81c16cd) Co-authored-by: Søren Løvborg <[email protected]>
1 parent a6f73f6 commit d54e22a

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
@@ -268,6 +268,22 @@ def _mkstemp_inner(dir, pre, suf, flags, output_type):
268268
raise FileExistsError(_errno.EEXIST,
269269
"No usable temporary file name found")
270270

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

272288
# User visible interfaces.
273289

@@ -789,17 +805,10 @@ def __init__(self, suffix=None, prefix=None, dir=None):
789805
def _rmtree(cls, name):
790806
def onerror(func, path, exc_info):
791807
if issubclass(exc_info[0], PermissionError):
792-
def resetperms(path):
793-
try:
794-
_os.chflags(path, 0)
795-
except AttributeError:
796-
pass
797-
_os.chmod(path, 0o700)
798-
799808
try:
800809
if path != name:
801-
resetperms(_os.path.dirname(path))
802-
resetperms(path)
810+
_resetperms(_os.path.dirname(path))
811+
_resetperms(path)
803812

804813
try:
805814
_os.unlink(path)

Lib/test/test_tempfile.py

+116-1
Original file line numberDiff line numberDiff line change
@@ -1394,6 +1394,103 @@ def test_cleanup_with_symlink_to_a_directory(self):
13941394
"were deleted")
13951395
d2.cleanup()
13961396

1397+
@support.skip_unless_symlink
1398+
def test_cleanup_with_symlink_modes(self):
1399+
# cleanup() should not follow symlinks when fixing mode bits (#91133)
1400+
with self.do_create(recurse=0) as d2:
1401+
file1 = os.path.join(d2, 'file1')
1402+
open(file1, 'wb').close()
1403+
dir1 = os.path.join(d2, 'dir1')
1404+
os.mkdir(dir1)
1405+
for mode in range(8):
1406+
mode <<= 6
1407+
with self.subTest(mode=format(mode, '03o')):
1408+
def test(target, target_is_directory):
1409+
d1 = self.do_create(recurse=0)
1410+
symlink = os.path.join(d1.name, 'symlink')
1411+
os.symlink(target, symlink,
1412+
target_is_directory=target_is_directory)
1413+
try:
1414+
os.chmod(symlink, mode, follow_symlinks=False)
1415+
except NotImplementedError:
1416+
pass
1417+
try:
1418+
os.chmod(symlink, mode)
1419+
except FileNotFoundError:
1420+
pass
1421+
os.chmod(d1.name, mode)
1422+
d1.cleanup()
1423+
self.assertFalse(os.path.exists(d1.name))
1424+
1425+
with self.subTest('nonexisting file'):
1426+
test('nonexisting', target_is_directory=False)
1427+
with self.subTest('nonexisting dir'):
1428+
test('nonexisting', target_is_directory=True)
1429+
1430+
with self.subTest('existing file'):
1431+
os.chmod(file1, mode)
1432+
old_mode = os.stat(file1).st_mode
1433+
test(file1, target_is_directory=False)
1434+
new_mode = os.stat(file1).st_mode
1435+
self.assertEqual(new_mode, old_mode,
1436+
'%03o != %03o' % (new_mode, old_mode))
1437+
1438+
with self.subTest('existing dir'):
1439+
os.chmod(dir1, mode)
1440+
old_mode = os.stat(dir1).st_mode
1441+
test(dir1, target_is_directory=True)
1442+
new_mode = os.stat(dir1).st_mode
1443+
self.assertEqual(new_mode, old_mode,
1444+
'%03o != %03o' % (new_mode, old_mode))
1445+
1446+
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags')
1447+
@support.skip_unless_symlink
1448+
def test_cleanup_with_symlink_flags(self):
1449+
# cleanup() should not follow symlinks when fixing flags (#91133)
1450+
flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK
1451+
self.check_flags(flags)
1452+
1453+
with self.do_create(recurse=0) as d2:
1454+
file1 = os.path.join(d2, 'file1')
1455+
open(file1, 'wb').close()
1456+
dir1 = os.path.join(d2, 'dir1')
1457+
os.mkdir(dir1)
1458+
def test(target, target_is_directory):
1459+
d1 = self.do_create(recurse=0)
1460+
symlink = os.path.join(d1.name, 'symlink')
1461+
os.symlink(target, symlink,
1462+
target_is_directory=target_is_directory)
1463+
try:
1464+
os.chflags(symlink, flags, follow_symlinks=False)
1465+
except NotImplementedError:
1466+
pass
1467+
try:
1468+
os.chflags(symlink, flags)
1469+
except FileNotFoundError:
1470+
pass
1471+
os.chflags(d1.name, flags)
1472+
d1.cleanup()
1473+
self.assertFalse(os.path.exists(d1.name))
1474+
1475+
with self.subTest('nonexisting file'):
1476+
test('nonexisting', target_is_directory=False)
1477+
with self.subTest('nonexisting dir'):
1478+
test('nonexisting', target_is_directory=True)
1479+
1480+
with self.subTest('existing file'):
1481+
os.chflags(file1, flags)
1482+
old_flags = os.stat(file1).st_flags
1483+
test(file1, target_is_directory=False)
1484+
new_flags = os.stat(file1).st_flags
1485+
self.assertEqual(new_flags, old_flags)
1486+
1487+
with self.subTest('existing dir'):
1488+
os.chflags(dir1, flags)
1489+
old_flags = os.stat(dir1).st_flags
1490+
test(dir1, target_is_directory=True)
1491+
new_flags = os.stat(dir1).st_flags
1492+
self.assertEqual(new_flags, old_flags)
1493+
13971494
@support.cpython_only
13981495
def test_del_on_collection(self):
13991496
# A TemporaryDirectory is deleted when garbage collected
@@ -1506,9 +1603,27 @@ def test_modes(self):
15061603
d.cleanup()
15071604
self.assertFalse(os.path.exists(d.name))
15081605

1509-
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.lchflags')
1606+
def check_flags(self, flags):
1607+
# skip the test if these flags are not supported (ex: FreeBSD 13)
1608+
filename = support.TESTFN
1609+
try:
1610+
open(filename, "w").close()
1611+
try:
1612+
os.chflags(filename, flags)
1613+
except OSError as exc:
1614+
# "OSError: [Errno 45] Operation not supported"
1615+
self.skipTest(f"chflags() doesn't support flags "
1616+
f"{flags:#b}: {exc}")
1617+
else:
1618+
os.chflags(filename, 0)
1619+
finally:
1620+
support.unlink(filename)
1621+
1622+
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags')
15101623
def test_flags(self):
15111624
flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK
1625+
self.check_flags(flags)
1626+
15121627
d = self.do_create(recurse=3, dirs=2, files=2)
15131628
with d:
15141629
# 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)