From 12f5a296cfa61cfb3e6d50149548ee6681ea6b73 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Thu, 19 Jan 2023 23:04:30 -0800 Subject: [PATCH] python/cpython#101144: Allow open and read_text encoding to be positional. (python/cpython#101145) The zipfile.Path open() and read_text() encoding parameter can be supplied as a positional argument without causing a TypeError again. 3.10.0b1 included a regression that made it keyword only. Documentation update included as users writing code to be compatible with a wide range of versions will need to consider this for some time. --- tests/test_zipp.py | 74 ++++++++++++++++++++++++++++++++++++++++++---- zipp/__init__.py | 15 ++++++---- 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/tests/test_zipp.py b/tests/test_zipp.py index 1e17870..cb03bca 100644 --- a/tests/test_zipp.py +++ b/tests/test_zipp.py @@ -1,13 +1,14 @@ import io -import zipfile +import itertools import contextlib import pathlib -import unittest import tempfile import shutil -import string import pickle -import itertools +import string +from test.support.script_helper import assert_python_ok +import unittest +import zipfile import jaraco.itertools import func_timeout @@ -140,7 +141,69 @@ def test_open(self, alpharep): a, b, g = root.iterdir() with a.open(encoding="utf-8") as strm: data = strm.read() - assert data == "content of a" + self.assertEqual(data, "content of a") + with a.open('r', "utf-8") as strm: # not a kw, no gh-101144 TypeError + data = strm.read() + self.assertEqual(data, "content of a") + + def test_open_encoding_utf16(self): + in_memory_file = io.BytesIO() + zf = zipfile.ZipFile(in_memory_file, "w") + zf.writestr("path/16.txt", "This was utf-16".encode("utf-16")) + zf.filename = "test_open_utf16.zip" + root = zipp.Path(zf) + (path,) = root.iterdir() + u16 = path.joinpath("16.txt") + with u16.open('r', "utf-16") as strm: + data = strm.read() + self.assertEqual(data, "This was utf-16") + with u16.open(encoding="utf-16") as strm: + data = strm.read() + self.assertEqual(data, "This was utf-16") + + def test_open_encoding_errors(self): + in_memory_file = io.BytesIO() + zf = zipfile.ZipFile(in_memory_file, "w") + zf.writestr("path/bad-utf8.bin", b"invalid utf-8: \xff\xff.") + zf.filename = "test_read_text_encoding_errors.zip" + root = zipp.Path(zf) + (path,) = root.iterdir() + u16 = path.joinpath("bad-utf8.bin") + + # encoding= as a positional argument for gh-101144. + data = u16.read_text("utf-8", errors="ignore") + self.assertEqual(data, "invalid utf-8: .") + with u16.open("r", "utf-8", errors="surrogateescape") as f: + self.assertEqual(f.read(), "invalid utf-8: \udcff\udcff.") + + # encoding= both positional and keyword is an error; gh-101144. + with self.assertRaisesRegex(TypeError, "encoding"): + data = u16.read_text("utf-8", encoding="utf-8") + + # both keyword arguments work. + with u16.open("r", encoding="utf-8", errors="strict") as f: + # error during decoding with wrong codec. + with self.assertRaises(UnicodeDecodeError): + f.read() + + def test_encoding_warnings(self): + """EncodingWarning must blame the read_text and open calls.""" + code = '''\ +import io, zipfile +with zipfile.ZipFile(io.BytesIO(), "w") as zf: + zf.filename = '' + zf.writestr("path/file.txt", b"Spanish Inquisition") + root = zipp.Path(zf) + (path,) = root.iterdir() + file_path = path.joinpath("file.txt") + unused = file_path.read_text() # should warn + file_path.open("r").close() # should warn +''' + proc = assert_python_ok('-X', 'warn_default_encoding', '-c', code) + warnings = proc.err.splitlines() + self.assertEqual(len(warnings), 2, proc.err) + self.assertRegex(warnings[0], rb"^:8: EncodingWarning:") + self.assertRegex(warnings[1], rb"^:9: EncodingWarning:") def test_open_write(self): """ @@ -182,6 +245,7 @@ def test_read(self, alpharep): root = zipp.Path(alpharep) a, b, g = root.iterdir() assert a.read_text(encoding="utf-8") == "content of a" + a.read_text("utf-8") # No positional arg TypeError per gh-101144. assert a.read_bytes() == b"content of a" @pass_alpharep diff --git a/zipp/__init__.py b/zipp/__init__.py index ad01e27..e98f1e1 100644 --- a/zipp/__init__.py +++ b/zipp/__init__.py @@ -152,6 +152,11 @@ def _name_set(self): return self.__lookup +def _extract_text_encoding(encoding=None, *args, **kwargs): + # stacklevel=3 so that the caller of the caller see any warning. + return text_encoding(encoding, 3), args, kwargs + + class Path: """ A pathlib-compatible interface for zip files. @@ -273,9 +278,9 @@ def open(self, mode='r', *args, pwd=None, **kwargs): if args or kwargs: raise ValueError("encoding args invalid for binary operation") return stream - else: - kwargs["encoding"] = text_encoding(kwargs.get("encoding")) - return io.TextIOWrapper(stream, *args, **kwargs) + # Text mode: + encoding, args, kwargs = _extract_text_encoding(*args, **kwargs) + return io.TextIOWrapper(stream, encoding, *args, **kwargs) @property def name(self): @@ -298,8 +303,8 @@ def filename(self): return pathlib.Path(self.root.filename).joinpath(self.at) def read_text(self, *args, **kwargs): - kwargs["encoding"] = text_encoding(kwargs.get("encoding")) - with self.open('r', *args, **kwargs) as strm: + encoding, args, kwargs = _extract_text_encoding(*args, **kwargs) + with self.open('r', encoding, *args, **kwargs) as strm: return strm.read() def read_bytes(self):