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

Patch by Serhiy Storchaka.

Lib/zipfile.py

@@ -718,8 +718,9 @@ class ZipFile(object):
             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
@@ -738,10 +739,13 @@ class ZipFile(object):
                     # even if no files are added to the archive
                     self._didModify = True
             else:
-            if not self._filePassed:
-                self.fp.close()
+                raise RuntimeError('Mode must be "r", "w" or "a"')
+        except:
+            fp = self.fp
             self.fp = None
-            raise RuntimeError, 'Mode must be "r", "w" or "a"'
+            if not self._filePassed:
+                fp.close()
+            raise
 
     def __enter__(self):
         return self
@@ -749,17 +753,6 @@ class ZipFile(object):
     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
@@ -853,7 +846,7 @@ class ZipFile(object):
             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:
@@ -909,6 +902,7 @@ class ZipFile(object):
             zef_file = open(self.filename, 'rb')
             should_close = True
 
+        try:
             # Make sure we have an info object
             if isinstance(name, ZipInfo):
                 # 'name' is already an info object
@@ -963,6 +957,10 @@ class ZipFile(object):
 
             return ZipExtFile(zef_file, mode, zinfo, zd,
                     close_fileobj=should_close)
+        except:
+            if should_close:
+                zef_file.close()
+            raise
 
     def extract(self, member, path=None, pwd=None):
         """Extract a member from the archive to the current working directory,
@@ -1019,11 +1017,9 @@ class ZipFile(object):
                 os.mkdir(targetpath)
             return targetpath
 
-        source = self.open(member, pwd=pwd)
-        target = file(targetpath, "wb")
+        with self.open(member, pwd=pwd) as source, \
+             file(targetpath, "wb") as target:
             shutil.copyfileobj(source, target)
-        source.close()
-        target.close()
 
         return targetpath
 
@@ -1184,6 +1180,7 @@ class ZipFile(object):
         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()
@@ -1276,10 +1273,11 @@ class ZipFile(object):
                 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):
@@ -1401,15 +1399,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))
@@ -1420,7 +1417,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('./'):
@@ -1433,7 +1430,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:
@@ -1449,11 +1445,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

@@ -148,6 +148,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 #16327: The subprocess module no longer leaks file descriptors
   used for stdin/stdout/stderr pipes to the child when fork() fails.