Skip to content

Commit 02a9259

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

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

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

267283
# User visible interfaces.
268284

@@ -786,17 +802,10 @@ def __init__(self, suffix=None, prefix=None, dir=None):
786802
def _rmtree(cls, name):
787803
def onerror(func, path, exc_info):
788804
if issubclass(exc_info[0], PermissionError):
789-
def resetperms(path):
790-
try:
791-
_os.chflags(path, 0)
792-
except AttributeError:
793-
pass
794-
_os.chmod(path, 0o700)
795-
796805
try:
797806
if path != name:
798-
resetperms(_os.path.dirname(path))
799-
resetperms(path)
807+
_resetperms(_os.path.dirname(path))
808+
_resetperms(path)
800809

801810
try:
802811
_os.unlink(path)

Lib/test/test_tempfile.py

+116-1
Original file line numberDiff line numberDiff line change
@@ -1377,6 +1377,103 @@ def test_cleanup_with_symlink_to_a_directory(self):
13771377
"were deleted")
13781378
d2.cleanup()
13791379

1380+
@support.skip_unless_symlink
1381+
def test_cleanup_with_symlink_modes(self):
1382+
# cleanup() should not follow symlinks when fixing mode bits (#91133)
1383+
with self.do_create(recurse=0) as d2:
1384+
file1 = os.path.join(d2, 'file1')
1385+
open(file1, 'wb').close()
1386+
dir1 = os.path.join(d2, 'dir1')
1387+
os.mkdir(dir1)
1388+
for mode in range(8):
1389+
mode <<= 6
1390+
with self.subTest(mode=format(mode, '03o')):
1391+
def test(target, target_is_directory):
1392+
d1 = self.do_create(recurse=0)
1393+
symlink = os.path.join(d1.name, 'symlink')
1394+
os.symlink(target, symlink,
1395+
target_is_directory=target_is_directory)
1396+
try:
1397+
os.chmod(symlink, mode, follow_symlinks=False)
1398+
except NotImplementedError:
1399+
pass
1400+
try:
1401+
os.chmod(symlink, mode)
1402+
except FileNotFoundError:
1403+
pass
1404+
os.chmod(d1.name, mode)
1405+
d1.cleanup()
1406+
self.assertFalse(os.path.exists(d1.name))
1407+
1408+
with self.subTest('nonexisting file'):
1409+
test('nonexisting', target_is_directory=False)
1410+
with self.subTest('nonexisting dir'):
1411+
test('nonexisting', target_is_directory=True)
1412+
1413+
with self.subTest('existing file'):
1414+
os.chmod(file1, mode)
1415+
old_mode = os.stat(file1).st_mode
1416+
test(file1, target_is_directory=False)
1417+
new_mode = os.stat(file1).st_mode
1418+
self.assertEqual(new_mode, old_mode,
1419+
'%03o != %03o' % (new_mode, old_mode))
1420+
1421+
with self.subTest('existing dir'):
1422+
os.chmod(dir1, mode)
1423+
old_mode = os.stat(dir1).st_mode
1424+
test(dir1, target_is_directory=True)
1425+
new_mode = os.stat(dir1).st_mode
1426+
self.assertEqual(new_mode, old_mode,
1427+
'%03o != %03o' % (new_mode, old_mode))
1428+
1429+
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags')
1430+
@support.skip_unless_symlink
1431+
def test_cleanup_with_symlink_flags(self):
1432+
# cleanup() should not follow symlinks when fixing flags (#91133)
1433+
flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK
1434+
self.check_flags(flags)
1435+
1436+
with self.do_create(recurse=0) as d2:
1437+
file1 = os.path.join(d2, 'file1')
1438+
open(file1, 'wb').close()
1439+
dir1 = os.path.join(d2, 'dir1')
1440+
os.mkdir(dir1)
1441+
def test(target, target_is_directory):
1442+
d1 = self.do_create(recurse=0)
1443+
symlink = os.path.join(d1.name, 'symlink')
1444+
os.symlink(target, symlink,
1445+
target_is_directory=target_is_directory)
1446+
try:
1447+
os.chflags(symlink, flags, follow_symlinks=False)
1448+
except NotImplementedError:
1449+
pass
1450+
try:
1451+
os.chflags(symlink, flags)
1452+
except FileNotFoundError:
1453+
pass
1454+
os.chflags(d1.name, flags)
1455+
d1.cleanup()
1456+
self.assertFalse(os.path.exists(d1.name))
1457+
1458+
with self.subTest('nonexisting file'):
1459+
test('nonexisting', target_is_directory=False)
1460+
with self.subTest('nonexisting dir'):
1461+
test('nonexisting', target_is_directory=True)
1462+
1463+
with self.subTest('existing file'):
1464+
os.chflags(file1, flags)
1465+
old_flags = os.stat(file1).st_flags
1466+
test(file1, target_is_directory=False)
1467+
new_flags = os.stat(file1).st_flags
1468+
self.assertEqual(new_flags, old_flags)
1469+
1470+
with self.subTest('existing dir'):
1471+
os.chflags(dir1, flags)
1472+
old_flags = os.stat(dir1).st_flags
1473+
test(dir1, target_is_directory=True)
1474+
new_flags = os.stat(dir1).st_flags
1475+
self.assertEqual(new_flags, old_flags)
1476+
13801477
@support.cpython_only
13811478
def test_del_on_collection(self):
13821479
# A TemporaryDirectory is deleted when garbage collected
@@ -1489,9 +1586,27 @@ def test_modes(self):
14891586
d.cleanup()
14901587
self.assertFalse(os.path.exists(d.name))
14911588

1492-
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.lchflags')
1589+
def check_flags(self, flags):
1590+
# skip the test if these flags are not supported (ex: FreeBSD 13)
1591+
filename = support.TESTFN
1592+
try:
1593+
open(filename, "w").close()
1594+
try:
1595+
os.chflags(filename, flags)
1596+
except OSError as exc:
1597+
# "OSError: [Errno 45] Operation not supported"
1598+
self.skipTest(f"chflags() doesn't support flags "
1599+
f"{flags:#b}: {exc}")
1600+
else:
1601+
os.chflags(filename, 0)
1602+
finally:
1603+
support.unlink(filename)
1604+
1605+
@unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.chflags')
14931606
def test_flags(self):
14941607
flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK
1608+
self.check_flags(flags)
1609+
14951610
d = self.do_create(recurse=3, dirs=2, files=2)
14961611
with d:
14971612
# 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)