From 79efbb719383386051c72f2ee932eeca8e033e6b Mon Sep 17 00:00:00 2001 From: Olexa Bilaniuk Date: Thu, 9 May 2019 22:22:06 -0500 Subject: [PATCH] bpo-24538: Fix bug in shutil involving the copying of xattrs to read-only files. (PR-13212) Extended attributes can only be set on user-writeable files, but shutil previously first chmod()ed the destination file to the source's permissions and then tried to copy xattrs. This will cause failures if attempting to copy read-only files with xattrs, as occurs with Git clones on Lustre FS. --- Lib/shutil.py | 4 +++- Lib/test/test_shutil.py | 8 ++++++++ Misc/ACKS | 1 + .../next/Library/2019-05-09-08-35-18.bpo-24538.WK8Y-k.rst | 3 +++ 4 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2019-05-09-08-35-18.bpo-24538.WK8Y-k.rst diff --git a/Lib/shutil.py b/Lib/shutil.py index 6cfe3738f6e..b2e8f5fd759 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -359,6 +359,9 @@ def lookup(name): mode = stat.S_IMODE(st.st_mode) lookup("utime")(dst, ns=(st.st_atime_ns, st.st_mtime_ns), follow_symlinks=follow) + # We must copy extended attributes before the file is (potentially) + # chmod()'ed read-only, otherwise setxattr() will error with -EACCES. + _copyxattr(src, dst, follow_symlinks=follow) try: lookup("chmod")(dst, mode, follow_symlinks=follow) except NotImplementedError: @@ -382,7 +385,6 @@ def lookup(name): break else: raise - _copyxattr(src, dst, follow_symlinks=follow) def copy(src, dst, *, follow_symlinks=True): """Copy data and mode bits ("cp src dst"). Return the file's destination. diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index e709a5661bf..eeebb97ff69 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -531,12 +531,20 @@ def _raise_on_src(fname, *, follow_symlinks=True): # test that shutil.copystat copies xattrs src = os.path.join(tmp_dir, 'the_original') + srcro = os.path.join(tmp_dir, 'the_original_ro') write_file(src, src) + write_file(srcro, srcro) os.setxattr(src, 'user.the_value', b'fiddly') + os.setxattr(srcro, 'user.the_value', b'fiddly') + os.chmod(srcro, 0o444) dst = os.path.join(tmp_dir, 'the_copy') + dstro = os.path.join(tmp_dir, 'the_copy_ro') write_file(dst, dst) + write_file(dstro, dstro) shutil.copystat(src, dst) + shutil.copystat(srcro, dstro) self.assertEqual(os.getxattr(dst, 'user.the_value'), b'fiddly') + self.assertEqual(os.getxattr(dstro, 'user.the_value'), b'fiddly') @support.skip_unless_symlink @support.skip_unless_xattr diff --git a/Misc/ACKS b/Misc/ACKS index 300e7889407..dfb96375360 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -149,6 +149,7 @@ Stephen Bevan Ron Bickers Natalia B. Bidart Adrian von Bidder +Olexa Bilaniuk David Binger Dominic Binks Philippe Biondi diff --git a/Misc/NEWS.d/next/Library/2019-05-09-08-35-18.bpo-24538.WK8Y-k.rst b/Misc/NEWS.d/next/Library/2019-05-09-08-35-18.bpo-24538.WK8Y-k.rst new file mode 100644 index 00000000000..e799f931bce --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-05-09-08-35-18.bpo-24538.WK8Y-k.rst @@ -0,0 +1,3 @@ +In `shutil.copystat()`, first copy extended file attributes and then file +permissions, since extended attributes can only be set on the destination +while it is still writeable.