Skip to content

Commit

Permalink
bpo-40564: Avoid copying state from extant ZipFile. (GH-22371)
Browse files Browse the repository at this point in the history
bpo-40564: Avoid copying state from extant ZipFile.
  • Loading branch information
jaraco authored Oct 3, 2020
1 parent c111355 commit ebbe803
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 6 deletions.
33 changes: 33 additions & 0 deletions Lib/test/test_zipfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()
23 changes: 17 additions & 6 deletions Lib/zipfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit ebbe803

Please sign in to comment.