From f53ae11358758c3a1d8eb1b98232cc4bbb01549b Mon Sep 17 00:00:00 2001 From: Ma Lin Date: Mon, 8 Nov 2021 19:19:39 +0800 Subject: [PATCH 1/6] 1. _ZipWriteFile.write() handle buffer protocol correctly No longer use len() to get the length of the input data. For some buffer protocol objects, the length obtained by using len() is wrong. Co-authored-by: Marco Ribeiro --- Lib/test/test_zipfile.py | 9 +++++++++ Lib/zipfile.py | 9 ++++++++- .../Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst | 2 ++ 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst diff --git a/Lib/test/test_zipfile.py b/Lib/test/test_zipfile.py index df48fabff951d8..f9fb599a1db31e 100644 --- a/Lib/test/test_zipfile.py +++ b/Lib/test/test_zipfile.py @@ -1,3 +1,4 @@ +import array import contextlib import importlib.util import io @@ -1718,6 +1719,14 @@ def test_non_existent_file_raises_OSError(self): # quickly. self.assertRaises(OSError, zipfile.ZipFile, TESTFN) + def test_issue44439(self): + q = array.array('Q', [1, 2, 3, 4, 5]) + LENGTH = len(q) * q.itemsize + with zipfile.ZipFile(io.BytesIO(), 'w') as zip: + with zip.open('data', 'w') as data: + self.assertEqual(data.write(q), LENGTH) + self.assertEqual(data._file_size, LENGTH) + def test_empty_file_raises_BadZipFile(self): f = open(TESTFN, 'w', encoding='utf-8') f.close() diff --git a/Lib/zipfile.py b/Lib/zipfile.py index 8e9325b9343260..41bf49a8fe6850 100644 --- a/Lib/zipfile.py +++ b/Lib/zipfile.py @@ -1147,8 +1147,15 @@ def writable(self): def write(self, data): if self.closed: raise ValueError('I/O operation on closed file.') - nbytes = len(data) + + # Accept any data that supports the buffer protocol + if isinstance(data, (bytes, bytearray)): + nbytes = len(data) + else: + data = memoryview(data) + nbytes = data.nbytes self._file_size += nbytes + self._crc = crc32(data, self._crc) if self._compressor: data = self._compressor.compress(data) diff --git a/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst b/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst new file mode 100644 index 00000000000000..fd3cad3a8fd847 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst @@ -0,0 +1,2 @@ +Fix in `_ZipWriteFile.write()` method, when the input data is an object that +supports the buffer protocol, the file length may be wrong. From d103eb0c4216d30f116f322f5dc14dae2ab74fcf Mon Sep 17 00:00:00 2001 From: Ma Lin Date: Mon, 8 Nov 2021 20:09:50 +0800 Subject: [PATCH 2/6] 2. zlib: fix thread lock may go wrong in rare cases Getting `self->unconsumed_tail` before acquiring the thread lock may mix up decompress state. --- Modules/zlibmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/zlibmodule.c b/Modules/zlibmodule.c index 67bde701fa608b..dd5cea2dfa3b5b 100644 --- a/Modules/zlibmodule.c +++ b/Modules/zlibmodule.c @@ -1269,12 +1269,12 @@ zlib_Decompress_flush_impl(compobject *self, PyTypeObject *cls, return NULL; } + ENTER_ZLIB(self); + if (PyObject_GetBuffer(self->unconsumed_tail, &data, PyBUF_SIMPLE) == -1) { return NULL; } - ENTER_ZLIB(self); - self->zst.next_in = data.buf; ibuflen = data.len; From d6d554801b65fc00e396e8e9cd99aea63b6f5661 Mon Sep 17 00:00:00 2001 From: Ma Lin Date: Mon, 15 Nov 2021 04:47:24 +0800 Subject: [PATCH 3/6] Revert "2. zlib: fix thread lock may go wrong in rare cases" Fix this in another issue. --- Modules/zlibmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/zlibmodule.c b/Modules/zlibmodule.c index dd5cea2dfa3b5b..67bde701fa608b 100644 --- a/Modules/zlibmodule.c +++ b/Modules/zlibmodule.c @@ -1269,12 +1269,12 @@ zlib_Decompress_flush_impl(compobject *self, PyTypeObject *cls, return NULL; } - ENTER_ZLIB(self); - if (PyObject_GetBuffer(self->unconsumed_tail, &data, PyBUF_SIMPLE) == -1) { return NULL; } + ENTER_ZLIB(self); + self->zst.next_in = data.buf; ibuflen = data.len; From 6ba267a5865e3bafdd9cb431f18c0bda9a8f6357 Mon Sep 17 00:00:00 2001 From: Ma Lin Date: Mon, 7 Mar 2022 19:58:48 +0800 Subject: [PATCH 4/6] address comments --- Lib/test/test_zipfile.py | 16 ++++++++-------- .../2021-11-08-20-27-41.bpo-44439.I_8qro.rst | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_zipfile.py b/Lib/test/test_zipfile.py index f9fb599a1db31e..536412663fe7ba 100644 --- a/Lib/test/test_zipfile.py +++ b/Lib/test/test_zipfile.py @@ -1120,6 +1120,14 @@ def test_write_after_close(self): self.assertRaises(ValueError, w.write, b'') self.assertEqual(zipf.read('test'), data) + def test_issue44439(self): + q = array.array('Q', [1, 2, 3, 4, 5]) + LENGTH = len(q) * q.itemsize + with zipfile.ZipFile(io.BytesIO(), 'w', self.compression) as zip: + with zip.open('data', 'w') as data: + self.assertEqual(data.write(q), LENGTH) + self.assertEqual(zip.getinfo('data').file_size, LENGTH) + class StoredWriterTests(AbstractWriterTests, unittest.TestCase): compression = zipfile.ZIP_STORED @@ -1719,14 +1727,6 @@ def test_non_existent_file_raises_OSError(self): # quickly. self.assertRaises(OSError, zipfile.ZipFile, TESTFN) - def test_issue44439(self): - q = array.array('Q', [1, 2, 3, 4, 5]) - LENGTH = len(q) * q.itemsize - with zipfile.ZipFile(io.BytesIO(), 'w') as zip: - with zip.open('data', 'w') as data: - self.assertEqual(data.write(q), LENGTH) - self.assertEqual(data._file_size, LENGTH) - def test_empty_file_raises_BadZipFile(self): f = open(TESTFN, 'w', encoding='utf-8') f.close() diff --git a/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst b/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst index fd3cad3a8fd847..8775f930ba4c17 100644 --- a/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst +++ b/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst @@ -1,2 +1,2 @@ -Fix in `_ZipWriteFile.write()` method, when the input data is an object that -supports the buffer protocol, the file length may be wrong. +Fix `.write()` method of a member file in `ZipFile`, when the input data is an +object that supports the buffer protocol, the file length may be wrong. \ No newline at end of file From bdb848705a4658c9660b3737bd46fc93f21d174e Mon Sep 17 00:00:00 2001 From: Ma Lin Date: Mon, 7 Mar 2022 20:04:42 +0800 Subject: [PATCH 5/6] add a newline to NEWS --- .../next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst b/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst index 8775f930ba4c17..f3370f4587d628 100644 --- a/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst +++ b/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst @@ -1,2 +1,2 @@ Fix `.write()` method of a member file in `ZipFile`, when the input data is an -object that supports the buffer protocol, the file length may be wrong. \ No newline at end of file +object that supports the buffer protocol, the file length may be wrong. From 451e24f4ae427e844783dc05d7477e0cad5f185c Mon Sep 17 00:00:00 2001 From: Ma Lin Date: Mon, 7 Mar 2022 20:35:30 +0800 Subject: [PATCH 6/6] fix NEWS --- .../next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst b/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst index f3370f4587d628..f4e562c4236d2f 100644 --- a/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst +++ b/Misc/NEWS.d/next/Library/2021-11-08-20-27-41.bpo-44439.I_8qro.rst @@ -1,2 +1,2 @@ -Fix `.write()` method of a member file in `ZipFile`, when the input data is an -object that supports the buffer protocol, the file length may be wrong. +Fix ``.write()`` method of a member file in ``ZipFile``, when the input data is +an object that supports the buffer protocol, the file length may be wrong.