bpo-40564: Avoid copying state from extant ZipFile. (pythonGH-22371)

jaraco · web-flow · commit ebbe8033b1c6 · 2020-10-03T10:58:39.000-04:00
bpo-40564: Avoid copying state from extant ZipFile.
diff --git a/Lib/test/test_zipfile.py b/Lib/test/test_zipfile.py
@@ -2889,6 +2889,33 @@ def test_open(self):
                 data = strm.read()
             assert data == "content of a"
 
+    def test_open_write(self):
+        """
+        If the zipfile is open for write, it should be possible to
+        write bytes or text to it.
+        """
+        zf = zipfile.Path(zipfile.ZipFile(io.BytesIO(), mode='w'))
+        with zf.joinpath('file.bin').open('wb') as strm:
+            strm.write(b'binary contents')
+        with zf.joinpath('file.txt').open('w') as strm:
+            strm.write('text file')
+
+    def test_open_extant_directory(self):
+        """
+        Attempting to open a directory raises IsADirectoryError.
+        """
+        zf = zipfile.Path(add_dirs(build_alpharep_fixture()))
+        with self.assertRaises(IsADirectoryError):
+            zf.joinpath('b').open()
+
+    def test_open_missing_directory(self):
+        """
+        Attempting to open a missing directory raises FileNotFoundError.
+        """
+        zf = zipfile.Path(add_dirs(build_alpharep_fixture()))
+        with self.assertRaises(FileNotFoundError):
+            zf.joinpath('z').open()
+
     def test_read(self):
         for alpharep in self.zipfile_alpharep():
             root = zipfile.Path(alpharep)
@@ -2986,6 +3013,12 @@ def test_implied_dirs_performance(self):
         data = ['/'.join(string.ascii_lowercase + str(n)) for n in range(10000)]
         zipfile.CompleteDirs._implied_dirs(data)
 
+    def test_read_does_not_close(self):
+        for alpharep in self.zipfile_ondisk():
+            with zipfile.ZipFile(alpharep) as file:
+                for rep in range(2):
+                    zipfile.Path(file, 'a.txt').read_text()
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/Lib/zipfile.py b/Lib/zipfile.py
@@ -2197,13 +2197,12 @@ def make(cls, source):
         if not isinstance(source, ZipFile):
             return cls(source)
 
-        # Only allow for FastPath when supplied zipfile is read-only
+        # Only allow for FastLookup when supplied zipfile is read-only
         if 'r' not in source.mode:
             cls = CompleteDirs
 
-        res = cls.__new__(cls)
-        vars(res).update(vars(source))
-        return res
+        source.__class__ = cls
+        return source
 
 
 class FastLookup(CompleteDirs):
@@ -2292,17 +2291,29 @@ class Path:
     __repr = "{self.__class__.__name__}({self.root.filename!r}, {self.at!r})"
 
     def __init__(self, root, at=""):
+        """
+        Construct a Path from a ZipFile or filename.
+
+        Note: When the source is an existing ZipFile object,
+        its type (__class__) will be mutated to a
+        specialized type. If the caller wishes to retain the
+        original type, the caller should either create a
+        separate ZipFile object or pass a filename.
+        """
         self.root = FastLookup.make(root)
         self.at = at
 
-    def open(self, mode='r', *args, **kwargs):
+    def open(self, mode='r', *args, pwd=None, **kwargs):
         """
         Open this entry as text or binary following the semantics
         of ``pathlib.Path.open()`` by passing arguments through
         to io.TextIOWrapper().
         """
-        pwd = kwargs.pop('pwd', None)
+        if self.is_dir():
+            raise IsADirectoryError(self)
         zip_mode = mode[0]
+        if not self.exists() and zip_mode == 'r':
+            raise FileNotFoundError(self)
         stream = self.root.open(self.at, zip_mode, pwd=pwd)
         if 'b' in mode:
             if args or kwargs:
diff --git a/Misc/NEWS.d/next/Library/2020-09-23-03-33-37.bpo-40564.iXQqMq.rst b/Misc/NEWS.d/next/Library/2020-09-23-03-33-37.bpo-40564.iXQqMq.rst
@@ -0,0 +1 @@
+In ``zipfile.Path``, mutate the passed ZipFile object type instead of making a copy. Prevents issues when both the local copy and the caller’s copy attempt to close the same file handle.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+In ``zipfile.Path``, mutate the passed ZipFile object type instead of making a copy. Prevents issues when both the local copy and the caller’s copy attempt to close the same file handle.