Skip to content

Commit 77c81d7

Browse files
committed
gh-156002: Keep reading through third-party zipfile decompressors
GH-156003 made ZipExtFile._read1() call decompress(data, max_length) on non-deflate decompressors and consult needs_input before reading more. A decompressor installed by replacing _get_decompressor() (zipfile-zstd, zipfile-deflate64, ...) may support neither, and every read through it then failed with AttributeError. Give _decompressor_needs_input() a default for decompressors that report nothing, and only take the bounded path for decompressors that do report needs_input (the stdlib ones); others are read unbounded, as before.
1 parent 024b6bc commit 77c81d7

3 files changed

Lines changed: 89 additions & 7 deletions

File tree

Lib/test/test_zipfile/test_core.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4916,6 +4916,64 @@ class ZstdBoundedDecompressTests(AbstractBoundedDecompressTests,
49164916
compression = zipfile.ZIP_ZSTANDARD
49174917

49184918

4919+
class ThirdPartyDecompressorTests(unittest.TestCase):
4920+
# A decompressor installed by replacing _get_decompressor() may support
4921+
# neither decompress(data, max_length) nor needs_input. ZipExtFile must
4922+
# still read through it (unbounded, as before bounded decompression).
4923+
COMPRESSION = 99
4924+
4925+
class Compressor:
4926+
def compress(self, data):
4927+
return data
4928+
4929+
def flush(self):
4930+
return b''
4931+
4932+
class Decompressor:
4933+
eof = False
4934+
4935+
def decompress(self, data):
4936+
return data
4937+
4938+
def setUp(self):
4939+
orig_check_compression = zipfile._check_compression
4940+
orig_get_compressor = zipfile._get_compressor
4941+
orig_get_decompressor = zipfile._get_decompressor
4942+
4943+
def check_compression(compression):
4944+
if compression != self.COMPRESSION:
4945+
orig_check_compression(compression)
4946+
4947+
def get_compressor(compress_type, compresslevel=None):
4948+
if compress_type == self.COMPRESSION:
4949+
return self.Compressor()
4950+
return orig_get_compressor(compress_type, compresslevel)
4951+
4952+
def get_decompressor(compress_type):
4953+
if compress_type == self.COMPRESSION:
4954+
return self.Decompressor()
4955+
return orig_get_decompressor(compress_type)
4956+
4957+
self.enterContext(mock.patch.object(
4958+
zipfile, '_check_compression', check_compression))
4959+
self.enterContext(mock.patch.object(
4960+
zipfile, '_get_compressor', get_compressor))
4961+
self.enterContext(mock.patch.object(
4962+
zipfile, '_get_decompressor', get_decompressor))
4963+
4964+
def test_read_through_third_party_decompressor(self):
4965+
data = bytes(range(256)) * 256
4966+
buf = io.BytesIO()
4967+
with zipfile.ZipFile(buf, "w", compression=self.COMPRESSION) as zf:
4968+
zf.writestr("member", data)
4969+
with zipfile.ZipFile(io.BytesIO(buf.getvalue())) as zf:
4970+
self.assertEqual(zf.read("member"), data)
4971+
with zf.open("member") as f:
4972+
self.assertEqual(f.read(100), data[:100])
4973+
f.seek(-100, os.SEEK_END)
4974+
self.assertEqual(f.read(), data[-100:])
4975+
4976+
49194977
class AbstractBadCrcTests:
49204978
def test_testzip_with_bad_crc(self):
49214979
"""Tests that files with bad CRCs return their name from testzip."""

Lib/zipfile/__init__.py

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -893,11 +893,23 @@ def _get_compressor(compress_type, compresslevel=None):
893893
return None
894894

895895

896-
def _decompressor_needs_input(decompressor):
896+
def _decompressor_needs_input(decompressor, default):
897897
# bz2/zstd expose the stdlib decompressor's public needs_input; the LZMA
898898
# wrapper keeps it private (_needs_input) to avoid adding public API.
899+
# A decompressor with neither attribute reports *default*.
899900
needs_input = getattr(decompressor, "needs_input", None)
900-
return decompressor._needs_input if needs_input is None else needs_input
901+
if needs_input is None:
902+
needs_input = getattr(decompressor, "_needs_input", default)
903+
return needs_input
904+
905+
906+
def _decompressor_bounds_output(decompressor):
907+
# The stdlib bzip2/LZMA/Zstandard decompressors report needs_input and
908+
# accept decompress(data, max_length). A third-party decompressor
909+
# installed by replacing _get_decompressor() may support neither; it is
910+
# then read unbounded, as before the bounded-decompression fix.
911+
return (hasattr(decompressor, "needs_input")
912+
or hasattr(decompressor, "_needs_input"))
901913

902914

903915
def _get_decompressor(compress_type):
@@ -1007,7 +1019,7 @@ def __init__(self, fileobj, mode, zipinfo, pwd=None,
10071019
self._compress_left = zipinfo.compress_size
10081020
self._left = zipinfo.file_size
10091021

1010-
self._decompressor = _get_decompressor(self._compress_type)
1022+
self._set_decompressor()
10111023

10121024
self._eof = False
10131025
self._readbuffer = b''
@@ -1190,6 +1202,10 @@ def read1(self, n):
11901202
break
11911203
return buf
11921204

1205+
def _set_decompressor(self):
1206+
self._decompressor = _get_decompressor(self._compress_type)
1207+
self._decompress_bounded = _decompressor_bounds_output(self._decompressor)
1208+
11931209
def _read1(self, n):
11941210
# Read up to n compressed bytes with at most one read() system call,
11951211
# decrypt and decompress them.
@@ -1207,7 +1223,7 @@ def _read1(self, n):
12071223
else:
12081224
# bzip2/lzma/zstd: a bounded decompress() call may leave input
12091225
# buffered inside the decompressor; drain that before reading more.
1210-
if _decompressor_needs_input(self._decompressor):
1226+
if _decompressor_needs_input(self._decompressor, default=True):
12111227
data = self._read2(n)
12121228
else:
12131229
data = b''
@@ -1222,14 +1238,17 @@ def _read1(self, n):
12221238
not self._decompressor.unconsumed_tail)
12231239
if self._eof:
12241240
data += self._decompressor.flush()
1225-
else:
1241+
elif self._decompress_bounded:
12261242
# Bound the output of a single decompress() call (mirroring the
12271243
# DEFLATE path above) so that a small compressed member cannot
12281244
# expand into one unbounded read.
12291245
data = self._decompressor.decompress(data, max(n, self.MIN_READ_SIZE))
12301246
self._eof = (self._decompressor.eof or
12311247
self._compress_left <= 0 and
1232-
_decompressor_needs_input(self._decompressor))
1248+
_decompressor_needs_input(self._decompressor, default=True))
1249+
else:
1250+
data = self._decompressor.decompress(data)
1251+
self._eof = self._decompressor.eof or self._compress_left <= 0
12331252

12341253
data = data[:self._left]
12351254
self._left -= len(data)
@@ -1318,7 +1337,7 @@ def seek(self, offset, whence=os.SEEK_SET):
13181337
self._left = self._orig_file_size
13191338
self._readbuffer = b''
13201339
self._offset = 0
1321-
self._decompressor = _get_decompressor(self._compress_type)
1340+
self._set_decompressor()
13221341
self._eof = False
13231342
read_offset = new_pos
13241343
if self._decrypter is not None:
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
:mod:`zipfile` again reads members through a third-party decompressor
2+
installed by replacing ``_get_decompressor()``, unbounded as before the
3+
bounded-decompression fix, instead of raising :exc:`AttributeError`. The
4+
bound still applies to the standard library's bzip2, LZMA and Zstandard
5+
decompressors.

0 commit comments

Comments
 (0)