Issue #16408: Fix file descriptors not being closed in error conditions in the zipfile module.

Patch by Serhiy Storchaka.

Lib/zipfile.py

@@ -719,8 +719,9 @@ class ZipFile:
             self.fp = file
             self.filename = getattr(file, 'name', None)
 
+        try:
             if key == 'r':
-            self._GetContents()
+                self._RealGetContents()
             elif key == 'w':
                 # set the modified flag so central directory gets written
                 # even if no files are added to the archive
@@ -739,10 +740,13 @@ class ZipFile:
                     # even if no files are added to the archive
                     self._didModify = True
             else:
-            if not self._filePassed:
-                self.fp.close()
-                self.fp = None
                 raise RuntimeError('Mode must be "r", "w" or "a"')
+        except:
+            fp = self.fp
+            self.fp = None
+            if not self._filePassed:
+                fp.close()
+            raise
 
     def __enter__(self):
         return self
@@ -750,17 +754,6 @@ class ZipFile:
     def __exit__(self, type, value, traceback):
         self.close()
 
-    def _GetContents(self):
-        """Read the directory, making sure we close the file if the format
-        is bad."""
-        try:
-            self._RealGetContents()
-        except BadZipFile:
-            if not self._filePassed:
-                self.fp.close()
-                self.fp = None
-            raise
-
     def _RealGetContents(self):
         """Read in the table of contents for the ZIP file."""
         fp = self.fp
@@ -862,7 +855,7 @@ class ZipFile:
             try:
                 # Read by chunks, to avoid an OverflowError or a
                 # MemoryError with very large embedded files.
-                f = self.open(zinfo.filename, "r")
+                with self.open(zinfo.filename, "r") as f:
                     while f.read(chunk_size):     # Check CRC-32
                         pass
             except BadZipFile:
@@ -926,18 +919,14 @@ class ZipFile:
         else:
             zef_file = io.open(self.filename, 'rb')
 
+        try:
             # Make sure we have an info object
             if isinstance(name, ZipInfo):
                 # 'name' is already an info object
                 zinfo = name
             else:
                 # Get info object for name
-            try:
                 zinfo = self.getinfo(name)
-            except KeyError:
-                if not self._filePassed:
-                    zef_file.close()
-                raise
             zef_file.seek(zinfo.header_offset, 0)
 
             # Skip the file header:
@@ -957,8 +946,6 @@ class ZipFile:
                 fname_str = fname.decode("cp437")
 
             if fname_str != zinfo.orig_filename:
-            if not self._filePassed:
-                zef_file.close()
                 raise BadZipFile(
                     'File name in directory %r and header %r differ.'
                     % (zinfo.orig_filename, fname))
@@ -970,10 +957,8 @@ class ZipFile:
                 if not pwd:
                     pwd = self.pwd
                 if not pwd:
-                if not self._filePassed:
-                    zef_file.close()
-                raise RuntimeError("File %s is encrypted, "
-                                   "password required for extraction" % name)
+                    raise RuntimeError("File %s is encrypted, password "
+                                       "required for extraction" % name)
 
                 zd = _ZipDecrypter(pwd)
                 # The first 12 bytes in the cypher stream is an encryption header
@@ -990,12 +975,14 @@ class ZipFile:
                     # compare against the CRC otherwise
                     check_byte = (zinfo.CRC >> 24) & 0xff
                 if h[11] != check_byte:
-                if not self._filePassed:
-                    zef_file.close()
                     raise RuntimeError("Bad password for file", name)
 
             return ZipExtFile(zef_file, mode, zinfo, zd,
                               close_fileobj=not self._filePassed)
+        except:
+            if not self._filePassed:
+                zef_file.close()
+            raise
 
     def extract(self, member, path=None, pwd=None):
         """Extract a member from the archive to the current working directory,
@@ -1052,11 +1039,9 @@ class ZipFile:
                 os.mkdir(targetpath)
             return targetpath
 
-        source = self.open(member, pwd=pwd)
-        target = open(targetpath, "wb")
+        with self.open(member, pwd=pwd) as source, \
+             open(targetpath, "wb") as target:
             shutil.copyfileobj(source, target)
-        source.close()
-        target.close()
 
         return targetpath
 
@@ -1220,6 +1205,7 @@ class ZipFile:
         if self.fp is None:
             return
 
+        try:
             if self.mode in ("w", "a") and self._didModify: # write ending records
                 count = 0
                 pos1 = self.fp.tell()
@@ -1311,10 +1297,11 @@ class ZipFile:
                 self.fp.write(endrec)
                 self.fp.write(self._comment)
                 self.fp.flush()
-
-        if not self._filePassed:
-            self.fp.close()
+        finally:
+            fp = self.fp
             self.fp = None
+            if not self._filePassed:
+                fp.close()
 
 
 class PyZipFile(ZipFile):
@@ -1481,15 +1468,14 @@ def main(args = None):
         if len(args) != 2:
             print(USAGE)
             sys.exit(1)
-        zf = ZipFile(args[1], 'r')
+        with ZipFile(args[1], 'r') as zf:
             zf.printdir()
-        zf.close()
 
     elif args[0] == '-t':
         if len(args) != 2:
             print(USAGE)
             sys.exit(1)
-        zf = ZipFile(args[1], 'r')
+        with ZipFile(args[1], 'r') as zf:
             badfile = zf.testzip()
         if badfile:
             print("The following enclosed file is corrupted: {!r}".format(badfile))
@@ -1500,7 +1486,7 @@ def main(args = None):
             print(USAGE)
             sys.exit(1)
 
-        zf = ZipFile(args[1], 'r')
+        with ZipFile(args[1], 'r') as zf:
             out = args[2]
             for path in zf.namelist():
                 if path.startswith('./'):
@@ -1513,7 +1499,6 @@ def main(args = None):
                     os.makedirs(tgtdir)
                 with open(tgt, 'wb') as fp:
                     fp.write(zf.read(path))
-        zf.close()
 
     elif args[0] == '-c':
         if len(args) < 3:
@@ -1529,11 +1514,9 @@ def main(args = None):
                             os.path.join(path, nm), os.path.join(zippath, nm))
             # else: ignore
 
-        zf = ZipFile(args[1], 'w', allowZip64=True)
+        with ZipFile(args[1], 'w', allowZip64=True) as zf:
             for src in args[2:]:
                 addToZip(zf, src, os.path.basename(src))
 
-        zf.close()
-
 if __name__ == "__main__":
     main()

Misc/NEWS

@@ -164,6 +164,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #16408: Fix file descriptors not being closed in error conditions
+  in the zipfile module.  Patch by Serhiy Storchaka.
+
 - Issue #16140: The subprocess module no longer double closes its child
   subprocess.PIPE parent file descriptors on child error prior to exec().