[3.12] gh-113188: Fix shutil.copymode() and shutil.copystat() on Windows (GH-113285)

Previously they worked differenly if dst is a symbolic link:
they modified the permission bits of dst itself rather than the file
it points to if follow_symlinks is true or src is not a symbolic link,
and did nothing if follow_symlinks is false and src is a symbolic link.
This commit is contained in:
Serhiy Storchaka 2023-12-23 12:31:52 +02:00 committed by GitHub
parent 4259acd394
commit c7874bb56f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 42 additions and 22 deletions

View file

@ -302,11 +302,15 @@ def copymode(src, dst, *, follow_symlinks=True):
sys.audit("shutil.copymode", src, dst) sys.audit("shutil.copymode", src, dst)
if not follow_symlinks and _islink(src) and os.path.islink(dst): if not follow_symlinks and _islink(src) and os.path.islink(dst):
if hasattr(os, 'lchmod'): if os.name == 'nt':
stat_func, chmod_func = os.lstat, os.chmod
elif hasattr(os, 'lchmod'):
stat_func, chmod_func = os.lstat, os.lchmod stat_func, chmod_func = os.lstat, os.lchmod
else: else:
return return
else: else:
if os.name == 'nt' and os.path.islink(dst):
dst = os.path.realpath(dst, strict=True)
stat_func, chmod_func = _stat, os.chmod stat_func, chmod_func = _stat, os.chmod
st = stat_func(src) st = stat_func(src)
@ -382,8 +386,16 @@ def copystat(src, dst, *, follow_symlinks=True):
# We must copy extended attributes before the file is (potentially) # We must copy extended attributes before the file is (potentially)
# chmod()'ed read-only, otherwise setxattr() will error with -EACCES. # chmod()'ed read-only, otherwise setxattr() will error with -EACCES.
_copyxattr(src, dst, follow_symlinks=follow) _copyxattr(src, dst, follow_symlinks=follow)
_chmod = lookup("chmod")
if os.name == 'nt':
if follow:
if os.path.islink(dst):
dst = os.path.realpath(dst, strict=True)
else:
def _chmod(*args, **kwargs):
os.chmod(*args)
try: try:
lookup("chmod")(dst, mode, follow_symlinks=follow) _chmod(dst, mode, follow_symlinks=follow)
except NotImplementedError: except NotImplementedError:
# if we got a NotImplementedError, it's because # if we got a NotImplementedError, it's because
# * follow_symlinks=False, # * follow_symlinks=False,

View file

@ -1046,23 +1046,23 @@ class TestCopy(BaseTest, unittest.TestCase):
shutil.copymode(src, dst) shutil.copymode(src, dst)
self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode) self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode)
# On Windows, os.chmod does not follow symlinks (issue #15411) # On Windows, os.chmod does not follow symlinks (issue #15411)
if os.name != 'nt': # follow src link
# follow src link os.chmod(dst, stat.S_IRWXO)
os.chmod(dst, stat.S_IRWXO) shutil.copymode(src_link, dst)
shutil.copymode(src_link, dst) self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode)
self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode) # follow dst link
# follow dst link os.chmod(dst, stat.S_IRWXO)
os.chmod(dst, stat.S_IRWXO) shutil.copymode(src, dst_link)
shutil.copymode(src, dst_link) self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode)
self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode) # follow both links
# follow both links os.chmod(dst, stat.S_IRWXO)
os.chmod(dst, stat.S_IRWXO) shutil.copymode(src_link, dst_link)
shutil.copymode(src_link, dst_link) self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode)
self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode)
@unittest.skipUnless(hasattr(os, 'lchmod'), 'requires os.lchmod') @unittest.skipUnless(hasattr(os, 'lchmod') or os.name == 'nt', 'requires os.lchmod')
@os_helper.skip_unless_symlink @os_helper.skip_unless_symlink
def test_copymode_symlink_to_symlink(self): def test_copymode_symlink_to_symlink(self):
_lchmod = os.chmod if os.name == 'nt' else os.lchmod
tmp_dir = self.mkdtemp() tmp_dir = self.mkdtemp()
src = os.path.join(tmp_dir, 'foo') src = os.path.join(tmp_dir, 'foo')
dst = os.path.join(tmp_dir, 'bar') dst = os.path.join(tmp_dir, 'bar')
@ -1074,20 +1074,20 @@ class TestCopy(BaseTest, unittest.TestCase):
os.symlink(dst, dst_link) os.symlink(dst, dst_link)
os.chmod(src, stat.S_IRWXU|stat.S_IRWXG) os.chmod(src, stat.S_IRWXU|stat.S_IRWXG)
os.chmod(dst, stat.S_IRWXU) os.chmod(dst, stat.S_IRWXU)
os.lchmod(src_link, stat.S_IRWXO|stat.S_IRWXG) _lchmod(src_link, stat.S_IRWXO|stat.S_IRWXG)
# link to link # link to link
os.lchmod(dst_link, stat.S_IRWXO) _lchmod(dst_link, stat.S_IRWXO)
old_mode = os.stat(dst).st_mode old_mode = os.stat(dst).st_mode
shutil.copymode(src_link, dst_link, follow_symlinks=False) shutil.copymode(src_link, dst_link, follow_symlinks=False)
self.assertEqual(os.lstat(src_link).st_mode, self.assertEqual(os.lstat(src_link).st_mode,
os.lstat(dst_link).st_mode) os.lstat(dst_link).st_mode)
self.assertEqual(os.stat(dst).st_mode, old_mode) self.assertEqual(os.stat(dst).st_mode, old_mode)
# src link - use chmod # src link - use chmod
os.lchmod(dst_link, stat.S_IRWXO) _lchmod(dst_link, stat.S_IRWXO)
shutil.copymode(src_link, dst, follow_symlinks=False) shutil.copymode(src_link, dst, follow_symlinks=False)
self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode) self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode)
# dst link - use chmod # dst link - use chmod
os.lchmod(dst_link, stat.S_IRWXO) _lchmod(dst_link, stat.S_IRWXO)
shutil.copymode(src, dst_link, follow_symlinks=False) shutil.copymode(src, dst_link, follow_symlinks=False)
self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode) self.assertEqual(os.stat(src).st_mode, os.stat(dst).st_mode)
@ -1124,11 +1124,13 @@ class TestCopy(BaseTest, unittest.TestCase):
os.symlink(dst, dst_link) os.symlink(dst, dst_link)
if hasattr(os, 'lchmod'): if hasattr(os, 'lchmod'):
os.lchmod(src_link, stat.S_IRWXO) os.lchmod(src_link, stat.S_IRWXO)
elif os.name == 'nt':
os.chmod(src_link, stat.S_IRWXO)
if hasattr(os, 'lchflags') and hasattr(stat, 'UF_NODUMP'): if hasattr(os, 'lchflags') and hasattr(stat, 'UF_NODUMP'):
os.lchflags(src_link, stat.UF_NODUMP) os.lchflags(src_link, stat.UF_NODUMP)
src_link_stat = os.lstat(src_link) src_link_stat = os.lstat(src_link)
# follow # follow
if hasattr(os, 'lchmod'): if hasattr(os, 'lchmod') or os.name == 'nt':
shutil.copystat(src_link, dst_link, follow_symlinks=True) shutil.copystat(src_link, dst_link, follow_symlinks=True)
self.assertNotEqual(src_link_stat.st_mode, os.stat(dst).st_mode) self.assertNotEqual(src_link_stat.st_mode, os.stat(dst).st_mode)
# don't follow # don't follow
@ -1139,7 +1141,7 @@ class TestCopy(BaseTest, unittest.TestCase):
# The modification times may be truncated in the new file. # The modification times may be truncated in the new file.
self.assertLessEqual(getattr(src_link_stat, attr), self.assertLessEqual(getattr(src_link_stat, attr),
getattr(dst_link_stat, attr) + 1) getattr(dst_link_stat, attr) + 1)
if hasattr(os, 'lchmod'): if hasattr(os, 'lchmod') or os.name == 'nt':
self.assertEqual(src_link_stat.st_mode, dst_link_stat.st_mode) self.assertEqual(src_link_stat.st_mode, dst_link_stat.st_mode)
if hasattr(os, 'lchflags') and hasattr(src_link_stat, 'st_flags'): if hasattr(os, 'lchflags') and hasattr(src_link_stat, 'st_flags'):
self.assertEqual(src_link_stat.st_flags, dst_link_stat.st_flags) self.assertEqual(src_link_stat.st_flags, dst_link_stat.st_flags)

View file

@ -0,0 +1,6 @@
Fix :func:`shutil.copymode` and :func:`shutil.copystat` on Windows.
Previously they worked differenly if *dst* is a symbolic link:
they modified the permission bits of *dst* itself
rather than the file it points to if *follow_symlinks* is true or *src* is
not a symbolic link, and did not modify the permission bits if
*follow_symlinks* is false and *src* is a symbolic link.