gh-59999: Add option to preserve permissions in ZipFile.extract

Doc/library/zipfile.rst

@@ -137,6 +137,29 @@ The module defines the following items:
       methods, and may either refuse to process the ZIP file altogether,
       or fail to extract individual files.
 
+.. data:: PreserveMode.NONE
+
+    Constant for use in :meth:`ZipFile.extractall` and :meth:`ZipFile.extract` methods. Do not
+    preserve permissions of zipped files.
+
+    .. versionadded:: next
+
+.. data:: PreserveMode.SAFE
+
+    Constant for use in :meth:`ZipFile.extractall` and :meth:`ZipFile.extract` methods.
+    Preserve safe subset of permissions of the zipped files only: permissions
+    for reading, writing, execution for user, group and others.
+
+    .. versionadded:: next
+
+.. data:: PreserveMode.ALL
+
+    Constant for use in :meth:`ZipFile.extractall` and :meth:`ZipFile.extract` methods.
+    Preserve all the permissions of the zipped files, including unsafe ones:
+    UID bit (:data:`stat.S_ISUID`), group UID bit (:data:`stat.S_ISGID`),
+    sticky bit (:data:`stat.S_ISVTX`).
+
+    .. versionadded:: next
 
 .. seealso::
 
@@ -347,13 +370,15 @@ ZipFile Objects
       object was changed from ``'r'`` to ``'rb'``.
 
 
-.. method:: ZipFile.extract(member, path=None, pwd=None)
+.. method:: ZipFile.extract(member, path=None, pwd=None, \
+                            preserve_permissions=PreserveMode.NONE)
 
    Extract a member from the archive to the current working directory; *member*
    must be its full name or a :class:`ZipInfo` object.  Its file information is
-   extracted as accurately as possible.  *path* specifies a different directory
-   to extract to.  *member* can be a filename or a :class:`ZipInfo` object.
-   *pwd* is the password used for encrypted files as a :class:`bytes` object.
+   extracted as accurately as possible.  *member* can be a filename or a
+   :class:`ZipInfo` object.
+
+   *path*, *pwd*, and *preserve_permissions* have the same meaning as for :meth:`extract`.
 
    Returns the normalized path created (a directory or new file).
 
@@ -374,13 +399,17 @@ ZipFile Objects
    .. versionchanged:: 3.6.2
       The *path* parameter accepts a :term:`path-like object`.
 
+   .. versionchanged:: next
+      The *preserve_permissions* parameter was added.
 
-.. method:: ZipFile.extractall(path=None, members=None, pwd=None)
 
-   Extract all members from the archive to the current working directory.  *path*
-   specifies a different directory to extract to.  *members* is optional and must
-   be a subset of the list returned by :meth:`namelist`.  *pwd* is the password
-   used for encrypted files as a :class:`bytes` object.
+.. method:: ZipFile.extractall(path=None, members=None, pwd=None, \
+                               preserve_permissions=PreserveMode.NONE)
+
+   Extract all members from the archive to the current working directory.
+   *members* is optional and must be a subset of the list returned by :meth:`namelist`.
+
+   *path*, *pwd*, and *preserve_permissions* have the same meaning as for :meth:`extract`.
 
    .. warning::
 
@@ -397,6 +426,9 @@ ZipFile Objects
    .. versionchanged:: 3.6.2
       The *path* parameter accepts a :term:`path-like object`.
 
+   .. versionchanged:: next
+      The *preserve_permissions* parameter was added.
+
 
 .. method:: ZipFile.printdir()
 

Lib/test/test_zipfile/test_core.py

@@ -1,6 +1,7 @@
 import _pyio
 import array
 import contextlib
+import functools
 import importlib.util
 import io
 import itertools
@@ -49,6 +50,12 @@ def get_files(test):
         yield f
         test.assertFalse(f.closed)
 
+def isroot():
+    if sys.platform == "win32" or sys.platform == "wasi":
+        return False
+
+    return os.getuid() == 0
+
 class AbstractTestsWithSourceFile:
     @classmethod
     def setUpClass(cls):
@@ -1534,7 +1541,6 @@ def test_write_pathlike(self):
 
 
 class ExtractTests(unittest.TestCase):
-
     def make_test_file(self):
         with zipfile.ZipFile(TESTFN2, "w", zipfile.ZIP_STORED) as zipfp:
             for fpath, fdata in SMALL_TEST_DATA:
@@ -2582,6 +2588,84 @@ def tearDown(self):
         unlink(TESTFN)
         unlink(TESTFN2)
 
+@unittest.skipIf(sys.platform == "win32" or sys.platform == "wasi", "Requires file permissions")
+class TestsPermissionExtraction(unittest.TestCase):
+    def setUp(self):
+        os.mkdir(TESTFNDIR)
+        self.files = []
+
+        # Arbitrarily set the umask to 0 in order to retrieve the current umask
+        # Then restore the original value for the umask
+        self.umask = os.umask(0)
+        os.umask(self.umask)
+
+        with zipfile.ZipFile(TESTFN2, 'w', zipfile.ZIP_STORED) as zf:
+            for base_mode in range(0o1000):
+                for special_mask in range(0o10):
+                    mode = stat.S_IFREG | base_mode | special_mask << 9
+                    filename = os.path.join(TESTFNDIR, str(mode))
+                    zinfo = zipfile.ZipInfo(filename)
+                    zinfo.external_attr = (mode << 16)
+                    zf.writestr(zinfo, filename)
+                    self.files.append((filename, mode))
+
+    def tearDown(self):
+        os.unlink(TESTFN2)
+        rmtree(TESTFNDIR)
+
+    def test_extractall_preserve_none(self):
+        with zipfile.ZipFile(TESTFN2, 'r') as zf:
+            zf.extractall()
+            for filename, mode in self.files:
+                expected_mode = 0o666 & ~self.umask
+                self.assertTrue(os.path.exists(filename))
+                self.assertEqual(os.stat(filename).st_mode,
+                                 stat.S_IFREG | expected_mode)
+
+    def test_extract_preserve_none(self):
+        with zipfile.ZipFile(TESTFN2, 'r') as zf:
+            for filename, mode in self.files:
+                zf.extract(filename)
+                expected_mode = 0o666 & ~self.umask
+                self.assertTrue(os.path.exists(filename))
+                self.assertEqual(os.stat(filename).st_mode,
+                                 stat.S_IFREG | expected_mode)
+
+    def test_extractall_preserve_safe(self):
+        with zipfile.ZipFile(TESTFN2, 'r') as zf:
+            zf.extractall(preserve_permissions=zipfile.PreserveMode.SAFE)
+            for filename, mode in self.files:
+                self.assertTrue(os.path.exists(filename))
+                self.assertEqual(os.stat(filename).st_mode,
+                                 stat.S_IFREG | (mode & 0o777))
+
+    def test_extract_preserve_safe(self):
+        with zipfile.ZipFile(TESTFN2, 'r') as zf:
+            for filename, mode in self.files:
+                zf.extract(filename,
+                              preserve_permissions=zipfile.PreserveMode.SAFE)
+                self.assertTrue(os.path.exists(filename))
+                self.assertEqual(os.stat(filename).st_mode,
+                                 stat.S_IFREG | (mode & 0o777))
+
+    @unittest.skipUnless(isroot(), "requires root")
+    def test_extractall_preserve_all(self):
+        with zipfile.ZipFile(TESTFN2, 'r') as zf:
+            zf.extractall(preserve_permissions=zipfile.PreserveMode.ALL)
+            for filename, mode in self.files:
+                self.assertTrue(os.path.exists(filename))
+                self.assertEqual(os.stat(filename).st_mode,
+                                 stat.S_IFREG | mode)
+
+    @unittest.skipUnless(isroot(), "requires root")
+    def test_extract_preserve_all(self):
+        with zipfile.ZipFile(TESTFN2, 'r') as zf:
+            for filename, mode in self.files:
+                zf.extract(filename,
+                              preserve_permissions=zipfile.PreserveMode.ALL)
+                self.assertTrue(os.path.exists(filename))
+                self.assertEqual(os.stat(filename).st_mode,
+                                 stat.S_IFREG | mode)
 
 class AbstractBadCrcTests:
     def test_testzip_with_bad_crc(self):

Lib/zipfile/__init__.py

@@ -4,9 +4,12 @@
 XXX references to utf-8 need further investigation.
 """
 import binascii
+import contextlib
+import enum
 import importlib.util
 import io
 import os
+import pathlib
 import shutil
 import stat
 import struct
@@ -33,6 +36,7 @@
 
 __all__ = ["BadZipFile", "BadZipfile", "error",
            "ZIP_STORED", "ZIP_DEFLATED", "ZIP_BZIP2", "ZIP_LZMA",
+           "PreserveMode",
            "is_zipfile", "ZipInfo", "ZipFile", "PyZipFile", "LargeZipFile",
            "Path"]
 
@@ -373,6 +377,12 @@ def _sanitize_filename(filename):
     return filename
 
 
+class PreserveMode(enum.Enum):
+    """Options for preserving file permissions upon extraction."""
+    NONE = enum.auto()
+    SAFE = enum.auto()
+    ALL = enum.auto()
+
 class ZipInfo:
     """Class with attributes describing each file in the ZIP archive."""
 
@@ -1792,26 +1802,30 @@ def _open_to_write(self, zinfo, force_zip64=False):
         self._writing = True
         return _ZipWriteFile(self, zinfo, zip64)
 
-    def extract(self, member, path=None, pwd=None):
+    def extract(self, member, path=None, pwd=None,
+                preserve_permissions=PreserveMode.NONE):
         """Extract a member from the archive to the current working directory,
            using its full name. Its file information is extracted as accurately
            as possible. 'member' may be a filename or a ZipInfo object. You can
            specify a different directory using 'path'. You can specify the
-           password to decrypt the file using 'pwd'.
+           password to decrypt the file using 'pwd'. `preserve_permissions'
+           controls whether permissions of zipped files are preserved.
         """
         if path is None:
             path = os.getcwd()
         else:
             path = os.fspath(path)
 
-        return self._extract_member(member, path, pwd)
+        return self._extract_member(member, path, pwd, preserve_permissions)
 
-    def extractall(self, path=None, members=None, pwd=None):
+    def extractall(self, path=None, members=None, pwd=None,
+                   preserve_permissions=PreserveMode.NONE):
         """Extract all members from the archive to the current working
            directory. 'path' specifies a different directory to extract to.
            'members' is optional and must be a subset of the list returned
            by namelist(). You can specify the password to decrypt all files
-           using 'pwd'.
+           using 'pwd'. `preserve_permissions' controls whether permissions
+           of zipped files are preserved.
         """
         if members is None:
             members = self.namelist()
@@ -1822,7 +1836,7 @@ def extractall(self, path=None, members=None, pwd=None):
             path = os.fspath(path)
 
         for zipinfo in members:
-            self._extract_member(zipinfo, path, pwd)
+            self._extract_member(zipinfo, path, pwd, preserve_permissions)
 
     @classmethod
     def _sanitize_windows_name(cls, arcname, pathsep):
@@ -1839,7 +1853,27 @@ def _sanitize_windows_name(cls, arcname, pathsep):
         arcname = pathsep.join(x for x in arcname if x)
         return arcname
 
-    def _extract_member(self, member, targetpath, pwd):
+    def _apply_permissions(self, member, path, mode):
+        """
+        Apply ZipFile permissions to a file on the filesystem with
+        specified PreserveMode
+        """
+        if mode == PreserveMode.NONE:
+            return
+
+        # Ignore permissions if the archive was created on Windows
+        if member.create_system == 0:
+            return
+
+        mask = {
+            PreserveMode.SAFE: 0o777,
+            PreserveMode.ALL: 0xFFFF,
+        }
+        new_mode = (member.external_attr >> 16) & mask[mode]
+        os.chmod(path, new_mode)
+
+    def _extract_member(self, member, targetpath, pwd,
+                        preserve_permissions=PreserveMode.NONE):
         """Extract the ZipInfo object 'member' to a physical
            file on the path targetpath.
         """
@@ -1886,6 +1920,7 @@ def _extract_member(self, member, targetpath, pwd):
              open(targetpath, "wb") as target:
             shutil.copyfileobj(source, target)
 
+        self._apply_permissions(member, targetpath, preserve_permissions)
         return targetpath
 
     def _writecheck(self, zinfo):

Misc/ACKS

@@ -207,6 +207,7 @@ David Bonner
 Angelin Booz
 Médéric Boquien
 Matias Bordese
+Alexey Boriskin
 Jonas Borgström
 Jurjen Bos
 Peter Bosch

Misc/NEWS.d/next/Library/2022-04-13-01-20-21.gh-issue-59999.lfhqku.rst

@@ -0,0 +1 @@
+Add options to preserve file permissions in :meth:`zipfile.ZipFile.extract` and :meth:`zipfile.ZipFile.extractall`