From 4be7eeea9fd257ad75008306fe9973ba9bd47107 Mon Sep 17 00:00:00 2001 From: lipengyu Date: Fri, 23 Oct 2026 11:18:09 +0800 Subject: [PATCH 1/4] update --- Lib/test/test_zipfile/test_core.py | 36 +++++++++++++++++++ Lib/zipfile/__init__.py | 19 ++++++++-- ...-07-23-19-51-05.gh-issue-154481.6cFDFx.rst | 2 ++ 3 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-07-23-19-51-05.gh-issue-154481.6cFDFx.rst diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py index 708db4d4387df5e..457e4a34114024d 100644 --- a/Lib/test/test_zipfile/test_core.py +++ b/Lib/test/test_zipfile/test_core.py @@ -719,6 +719,42 @@ class ZstdTestsWithSourceFile(AbstractTestsWithSourceFile, unittest.TestCase): compression = zipfile.ZIP_ZSTANDARD + def test_read_multiframe(self): + first = b'first frame' * 400 + second = b'second frame' * 400 + data = first + second + + class MultiframeCompressor: + def compress(self, value): + value = bytes(value) + return (zipfile.zstd.compress(value[:len(first)]) + + zipfile.zstd.compress(value[len(first):])) + + def flush(self): + return b'' + + archive = io.BytesIO() + with mock.patch.object(zipfile, '_get_compressor', + return_value=MultiframeCompressor()): + with zipfile.ZipFile(archive, 'w', self.compression) as zipfp: + zipfp.writestr('multiframe', data) + + with zipfile.ZipFile(archive) as zipfp: + self.assertEqual(zipfp.read('multiframe'), data) + + with zipfile.ZipFile(archive) as zipfp: + with zipfp.open('multiframe') as fp: + chunk = fp._read1(1) + self.assertLessEqual(len(chunk), fp.MIN_READ_SIZE) + self.assertEqual(chunk + fp.read(), data) + + # Exercise a frame boundary between two reads from the archive. + with zipfile.ZipFile(archive) as zipfp: + with zipfp.open('multiframe') as fp: + fp.MIN_READ_SIZE = 1 + self.assertEqual(b''.join(iter(lambda: fp.read(1), b'')), + data) + class AbstractTestZip64InSmallFiles: # These tests test the ZIP64 functionality without using large files, # see test_zipfile64 for proper tests. diff --git a/Lib/zipfile/__init__.py b/Lib/zipfile/__init__.py index 87337478912e577..485512fdd322a4e 100644 --- a/Lib/zipfile/__init__.py +++ b/Lib/zipfile/__init__.py @@ -1195,6 +1195,13 @@ def _read1(self, n): data = self._decompressor.unconsumed_tail if n > len(data): data += self._read2(n - len(data)) + elif (self._compress_type == ZIP_ZSTANDARD and + self._decompressor.eof): + # Continue with the next Zstandard frame. + data = self._decompressor.unused_data + self._decompressor = _get_decompressor(self._compress_type) + if not data: + data = self._read2(n) elif self._compress_type == ZIP_STORED: data = self._read2(n) else: @@ -1233,9 +1240,15 @@ def _read1(self, n): remove=(3, 21), stacklevel=4) data = self._decompressor.decompress(data) - self._eof = (self._decompressor.eof or - self._compress_left <= 0 and - getattr(self._decompressor, "needs_input", True)) + if self._compress_type == ZIP_ZSTANDARD: + self._eof = (self._compress_left <= 0 and + not self._decompressor.unused_data and + (self._decompressor.eof or + self._decompressor.needs_input)) + else: + self._eof = (self._decompressor.eof or + self._compress_left <= 0 and + getattr(self._decompressor, "needs_input", True)) data = data[:self._left] self._left -= len(data) diff --git a/Misc/NEWS.d/next/Library/2026-07-23-19-51-05.gh-issue-154481.6cFDFx.rst b/Misc/NEWS.d/next/Library/2026-07-23-19-51-05.gh-issue-154481.6cFDFx.rst new file mode 100644 index 000000000000000..1dff74bc2597fa2 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-23-19-51-05.gh-issue-154481.6cFDFx.rst @@ -0,0 +1,2 @@ +Fix :mod:`zipfile` to correctly read Zstandard-compressed archive members +that could previously be truncated or incorrectly rejected. From 2a7f493bb225d6f94ec17869dfd83a1659b8ab07 Mon Sep 17 00:00:00 2001 From: lipengyu Date: Fri, 23 Oct 2026 15:52:17 +0800 Subject: [PATCH 2/4] modify test and news --- Lib/test/test_zipfile/test_core.py | 11 +++-------- .../2026-07-23-19-51-05.gh-issue-154481.6cFDFx.rst | 5 +++-- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py index 457e4a34114024d..46b32d318d50584 100644 --- a/Lib/test/test_zipfile/test_core.py +++ b/Lib/test/test_zipfile/test_core.py @@ -720,8 +720,8 @@ class ZstdTestsWithSourceFile(AbstractTestsWithSourceFile, compression = zipfile.ZIP_ZSTANDARD def test_read_multiframe(self): - first = b'first frame' * 400 - second = b'second frame' * 400 + first = b'first frame' * 20 + second = b'second frame' * 20 data = first + second class MultiframeCompressor: @@ -742,12 +742,6 @@ def flush(self): with zipfile.ZipFile(archive) as zipfp: self.assertEqual(zipfp.read('multiframe'), data) - with zipfile.ZipFile(archive) as zipfp: - with zipfp.open('multiframe') as fp: - chunk = fp._read1(1) - self.assertLessEqual(len(chunk), fp.MIN_READ_SIZE) - self.assertEqual(chunk + fp.read(), data) - # Exercise a frame boundary between two reads from the archive. with zipfile.ZipFile(archive) as zipfp: with zipfp.open('multiframe') as fp: @@ -755,6 +749,7 @@ def flush(self): self.assertEqual(b''.join(iter(lambda: fp.read(1), b'')), data) + class AbstractTestZip64InSmallFiles: # These tests test the ZIP64 functionality without using large files, # see test_zipfile64 for proper tests. diff --git a/Misc/NEWS.d/next/Library/2026-07-23-19-51-05.gh-issue-154481.6cFDFx.rst b/Misc/NEWS.d/next/Library/2026-07-23-19-51-05.gh-issue-154481.6cFDFx.rst index 1dff74bc2597fa2..ae821409a55d349 100644 --- a/Misc/NEWS.d/next/Library/2026-07-23-19-51-05.gh-issue-154481.6cFDFx.rst +++ b/Misc/NEWS.d/next/Library/2026-07-23-19-51-05.gh-issue-154481.6cFDFx.rst @@ -1,2 +1,3 @@ -Fix :mod:`zipfile` to correctly read Zstandard-compressed archive members -that could previously be truncated or incorrectly rejected. +Fix :mod:`zipfile` reading of Zstandard-compressed archive members containing +multiple frames, which could previously return truncated data or raise a +spurious CRC error. From 2f358ab0ab631d2ee922ddd5f08de69508f30add Mon Sep 17 00:00:00 2001 From: lipengyu Date: Thu, 29 Oct 2026 09:13:52 +0800 Subject: [PATCH 3/4] gh-154481: Handle invalid Zstandard frames in zipfile Preserve BadZipFile with the ZstdError as its cause. Cover seeking across frames, skippable frames, trailing data, split reads, and replacement decompressors. --- Lib/test/test_zipfile/test_core.py | 65 +++++++++++++++++++++++++----- Lib/zipfile/__init__.py | 47 ++++++++++++--------- 2 files changed, 82 insertions(+), 30 deletions(-) diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py index 46b32d318d50584..b5caf7938e853f8 100644 --- a/Lib/test/test_zipfile/test_core.py +++ b/Lib/test/test_zipfile/test_core.py @@ -719,36 +719,79 @@ class ZstdTestsWithSourceFile(AbstractTestsWithSourceFile, unittest.TestCase): compression = zipfile.ZIP_ZSTANDARD - def test_read_multiframe(self): - first = b'first frame' * 20 - second = b'second frame' * 20 - data = first + second - - class MultiframeCompressor: + def _make_archive(self, data, compressed): + class Compressor: def compress(self, value): - value = bytes(value) - return (zipfile.zstd.compress(value[:len(first)]) + - zipfile.zstd.compress(value[len(first):])) + return compressed def flush(self): return b'' archive = io.BytesIO() with mock.patch.object(zipfile, '_get_compressor', - return_value=MultiframeCompressor()): + return_value=Compressor()): with zipfile.ZipFile(archive, 'w', self.compression) as zipfp: zipfp.writestr('multiframe', data) + return archive + + def test_read_multiframe(self): + first = b'first frame' * 20 + second = b'second frame' * 20 + data = first + second + compressed = (zipfile.zstd.compress(first) + + zipfile.zstd.compress(second)) + archive = self._make_archive(data, compressed) with zipfile.ZipFile(archive) as zipfp: self.assertEqual(zipfp.read('multiframe'), data) - # Exercise a frame boundary between two reads from the archive. + with zipfile.ZipFile(archive) as zipfp: + with zipfp.open('multiframe') as fp: + self.assertEqual(fp.seek(len(first) + 2), len(first) + 2) + self.assertEqual(fp.read(), second[2:]) + self.assertEqual(fp.seek(len(first) - 2), len(first) - 2) + self.assertEqual(fp.read(4), + data[len(first) - 2:len(first) + 2]) + + # Read one compressed byte at a time, splitting frames across reads. with zipfile.ZipFile(archive) as zipfp: with zipfp.open('multiframe') as fp: fp.MIN_READ_SIZE = 1 self.assertEqual(b''.join(iter(lambda: fp.read(1), b'')), data) + def test_read_multiframe_with_skippable_frame(self): + first = b'first frame' + second = b'second frame' + skippable = struct.pack(' len(data): data += self._read2(n - len(data)) elif (self._compress_type == ZIP_ZSTANDARD and - self._decompressor.eof): + getattr(self._decompressor, "eof", False)): # Continue with the next Zstandard frame. - data = self._decompressor.unused_data + data = getattr(self._decompressor, "unused_data", b'') self._decompressor = _get_decompressor(self._compress_type) if not data: data = self._read2(n) @@ -1227,24 +1227,33 @@ def _read1(self, n): # DEFLATE path above) so that a small compressed member cannot # expand into one unbounded read. try: - data = self._decompressor.decompress(data, max(n, self.MIN_READ_SIZE)) - except TypeError: - # See MonkeypatchedDecompressorTests in test_core.py - warnings._deprecated( - 'one-argument decompress()', - 'The decompress() method of ' - + type(self._decompressor).__name__ - + ' should take two arguments, data and max_length.' - + ' One-argument calls will stop working before' - + ' Python 3.21.', - remove=(3, 21), - stacklevel=4) - data = self._decompressor.decompress(data) + try: + data = self._decompressor.decompress(data, max(n, self.MIN_READ_SIZE)) + except TypeError: + # See MonkeypatchedDecompressorTests in test_core.py + warnings._deprecated( + 'one-argument decompress()', + 'The decompress() method of ' + + type(self._decompressor).__name__ + + ' should take two arguments, data and max_length.' + + ' One-argument calls will stop working before' + + ' Python 3.21.', + remove=(3, 21), + stacklevel=4) + data = self._decompressor.decompress(data) + except Exception as exc: + if (self._compress_type == ZIP_ZSTANDARD and + zstd is not None and isinstance(exc, zstd.ZstdError)): + raise BadZipFile( + f"Bad Zstandard data for file {self.name!r}") from exc + raise if self._compress_type == ZIP_ZSTANDARD: - self._eof = (self._compress_left <= 0 and - not self._decompressor.unused_data and - (self._decompressor.eof or - self._decompressor.needs_input)) + if getattr(self._decompressor, "eof", False): + self._eof = (self._compress_left <= 0 and + not getattr(self._decompressor, "unused_data", b'')) + else: + self._eof = (self._compress_left <= 0 and + getattr(self._decompressor, "needs_input", True)) else: self._eof = (self._decompressor.eof or self._compress_left <= 0 and From 98fdb816273bd81484e55959c5f57e68c6f1f091 Mon Sep 17 00:00:00 2001 From: lipengyu Date: Thu, 29 Oct 2026 11:29:58 +0800 Subject: [PATCH 4/4] update test --- Lib/test/test_zipfile/test_core.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py index b5caf7938e853f8..1cf589d4c99a53d 100644 --- a/Lib/test/test_zipfile/test_core.py +++ b/Lib/test/test_zipfile/test_core.py @@ -781,16 +781,30 @@ def test_read_multiframe_with_trailing_data(self): self.assertIsInstance(cm.exception.__cause__, zipfile.zstd.ZstdError) def test_read_monkeypatched_zstd_decompressor(self): - class Decompressor: + class FrameDecompressor: + eof = False + + def decompress(self, data, max_length=-1): + if data: + self.eof = True + return data + + class StreamDecompressor: def decompress(self, data, max_length=-1): return data data = b'content' archive = self._make_archive(data, data) - with mock.patch.object(zipfile, '_get_decompressor', - return_value=Decompressor()): - with zipfile.ZipFile(archive) as zipfp: - self.assertEqual(zipfp.read('multiframe'), data) + for decompressor in FrameDecompressor, StreamDecompressor: + with self.subTest(decompressor=decompressor.__name__): + with mock.patch.object(zipfile, '_get_decompressor', + side_effect=lambda _: decompressor()): + with zipfile.ZipFile(archive) as zipfp: + self.assertEqual(zipfp.read('multiframe'), data) + with zipfp.open('multiframe') as fp: + fp.MIN_READ_SIZE = 1 + self.assertEqual( + b''.join(iter(lambda: fp.read(1), b'')), data) class AbstractTestZip64InSmallFiles: