From b95ccebaf046675e0aa4f5a7b2f92634806a6a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 28 Aug 2024 16:08:21 +0200 Subject: [PATCH 1/7] add `ZipFile.for_name` helper --- Lib/test/test_zipfile/_path/test_path.py | 17 ------------ Lib/zipfile/__init__.py | 34 ++++++++++++++++-------- 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/Lib/test/test_zipfile/_path/test_path.py b/Lib/test/test_zipfile/_path/test_path.py index aba515536f0c1a..fdbab050ffeaa2 100644 --- a/Lib/test/test_zipfile/_path/test_path.py +++ b/Lib/test/test_zipfile/_path/test_path.py @@ -657,20 +657,3 @@ class DirtyZipInfo(zipfile.ZipInfo): def __init__(self, filename, *args, **kwargs): super().__init__(filename, *args, **kwargs) self.filename = filename - - @classmethod - def for_name(cls, name, archive): - """ - Construct the same way that ZipFile.writestr does. - - TODO: extract this functionality and re-use - """ - self = cls(filename=name, date_time=time.localtime(time.time())[:6]) - self.compress_type = archive.compression - self.compress_level = archive.compresslevel - if self.filename.endswith('/'): # pragma: no cover - self.external_attr = 0o40775 << 16 # drwxrwxr-x - self.external_attr |= 0x10 # MS-DOS directory flag - else: - self.external_attr = 0o600 << 16 # ?rw------- - return self diff --git a/Lib/zipfile/__init__.py b/Lib/zipfile/__init__.py index e2aaf8bab4913d..e5dd5da3129252 100644 --- a/Lib/zipfile/__init__.py +++ b/Lib/zipfile/__init__.py @@ -603,6 +603,26 @@ def from_file(cls, filename, arcname=None, *, strict_timestamps=True): return zinfo + @classmethod + def for_name(cls, filename, archive, *, date_time=None): + """Construct an appropriate ZipInfo from a filename and a ZipFile. + + The *filename* is expected to be the name of a file in the archive. + + If *date_time* is not specified, the current local time is used. + """ + if date_time is None: + date_time = time.localtime(time.time())[:6] + self = cls(filename=filename, date_time=date_time) + self.compress_type = archive.compression + self.compress_level = archive.compresslevel + if self.filename.endswith('/'): # pragma: no cover + self.external_attr = 0o40775 << 16 # drwxrwxr-x + self.external_attr |= 0x10 # MS-DOS directory flag + else: + self.external_attr = 0o600 << 16 # ?rw------- + return self + def is_dir(self): """Return True if this archive member is a directory.""" if self.filename.endswith('/'): @@ -1903,18 +1923,10 @@ def writestr(self, zinfo_or_arcname, data, the name of the file in the archive.""" if isinstance(data, str): data = data.encode("utf-8") - if not isinstance(zinfo_or_arcname, ZipInfo): - zinfo = ZipInfo(filename=zinfo_or_arcname, - date_time=time.localtime(time.time())[:6]) - zinfo.compress_type = self.compression - zinfo.compress_level = self.compresslevel - if zinfo.filename.endswith('/'): - zinfo.external_attr = 0o40775 << 16 # drwxrwxr-x - zinfo.external_attr |= 0x10 # MS-DOS directory flag - else: - zinfo.external_attr = 0o600 << 16 # ?rw------- - else: + if isinstance(zinfo_or_arcname, ZipInfo): zinfo = zinfo_or_arcname + else: + zinfo = ZipInfo.for_name(zinfo_or_arcname, self) if not self.fp: raise ValueError( From 4acd888a4d52b1124d6366c771658c0acee80325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 28 Aug 2024 16:09:17 +0200 Subject: [PATCH 2/7] add tests --- Lib/test/test_zipfile/test_core.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py index 423974aada4ac1..c6abd17567a31c 100644 --- a/Lib/test/test_zipfile/test_core.py +++ b/Lib/test/test_zipfile/test_core.py @@ -5,6 +5,7 @@ import itertools import os import posixpath +import stat import struct import subprocess import sys @@ -2205,6 +2206,34 @@ def test_create_empty_zipinfo_repr(self): zi = zipfile.ZipInfo(filename="empty") self.assertEqual(repr(zi), "") + def test_create_zipinfo_for_name(self): + base_filename = TESTFN2.rstrip('/') + + with zipfile.ZipFile(TESTFN, mode="w", compresslevel=1, + compression=zipfile.ZIP_DEFLATED) as zf: + # no trailing forward slash + zi = zipfile.ZipInfo.for_name(base_filename, zf) + self.assertEqual(zi.compress_level, 1) + self.assertEqual(zi.compress_type, zipfile.ZIP_DEFLATED) + # ?rw- --- --- + filemode = stat.S_IRUSR | stat.S_IWUSR + # filemode is stored as the highest 16 bits of external_attr + self.assertEqual(zi.external_attr >> 16, filemode) + self.assertEqual(zi.external_attr & 0xFF, 0) # no MS-DOS flag + + with zipfile.ZipFile(TESTFN, mode="w", compresslevel=1, + compression=zipfile.ZIP_DEFLATED) as zf: + # with a trailing slash + zi = zipfile.ZipInfo.for_name(f'{base_filename}/', zf) + self.assertEqual(zi.compress_level, 1) + self.assertEqual(zi.compress_type, zipfile.ZIP_DEFLATED) + # d rwx rwx r-x + filemode = stat.S_IFDIR + filemode |= stat.S_IRWXU | stat.S_IRWXG + filemode |= stat.S_IROTH | stat.S_IXOTH + self.assertEqual(zi.external_attr >> 16, filemode) + self.assertEqual(zi.external_attr & 0xFF, 0x10) # MS-DOS flag + def test_create_empty_zipinfo_default_attributes(self): """Ensure all required attributes are set.""" zi = zipfile.ZipInfo() From d6c594bcc8884e85ac6ddabf5ac473d8e3b2b87c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 28 Aug 2024 16:10:40 +0200 Subject: [PATCH 3/7] blurb --- .../Library/2024-08-28-16-10-37.gh-issue-123424.u96_i6.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2024-08-28-16-10-37.gh-issue-123424.u96_i6.rst diff --git a/Misc/NEWS.d/next/Library/2024-08-28-16-10-37.gh-issue-123424.u96_i6.rst b/Misc/NEWS.d/next/Library/2024-08-28-16-10-37.gh-issue-123424.u96_i6.rst new file mode 100644 index 00000000000000..1167887daa939f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-08-28-16-10-37.gh-issue-123424.u96_i6.rst @@ -0,0 +1,3 @@ +Add :meth:`zipfile.ZipInfo.for_name` factory for constructing +:class:`~zipfile.ZipInfo` objects from a filename and an archive. Patch by +Bénédikt Tran. From b97016fd73d38e2a801583bf5571926ea7c673a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 28 Aug 2024 16:17:25 +0200 Subject: [PATCH 4/7] add What's New entry --- Doc/whatsnew/3.14.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Doc/whatsnew/3.14.rst b/Doc/whatsnew/3.14.rst index ba18991dee89d7..c2d38e15023682 100644 --- a/Doc/whatsnew/3.14.rst +++ b/Doc/whatsnew/3.14.rst @@ -225,6 +225,16 @@ symtable (Contributed by Bénédikt Tran in :gh:`120029`.) + +zipinfo +------- + +* Added :func:`ZipInfo.for_name ` for + constructing a :class:`~zipfile.ZipInfo` object from a filename + and an archive. + + (Contributed by Bénédikt Tran in :gh:`123424`.) + .. Add improved modules above alphabetically, not here at the end. Optimizations From 91a421e8efc7fdae2cdfe91c99be80f32a055868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 28 Aug 2024 16:17:27 +0200 Subject: [PATCH 5/7] update docs --- Doc/library/zipfile.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Doc/library/zipfile.rst b/Doc/library/zipfile.rst index 5583c6b24be5c6..e888f04ff806ee 100644 --- a/Doc/library/zipfile.rst +++ b/Doc/library/zipfile.rst @@ -84,6 +84,18 @@ The module defines the following items: formerly protected :attr:`!_compresslevel`. The older protected name continues to work as a property for backwards compatibility. + + .. classmethod:: for_name(filename, archive, *, date_time=None) + + Construct an appropriate :class:`ZipInfo` from a *filename*, + a :class:`ZipFile` archive and an optional *date_time*. + + If *date_time* is not specified, the current local time is used + instead, namely ``date_time = time.localtime(time.time())[:6]``. + + .. versionadded:: 3.14 + + .. function:: is_zipfile(filename) Returns ``True`` if *filename* is a valid ZIP file based on its magic number, From 2af18d896f76cd54fbd3765e794b463f1a1f4624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 28 Aug 2024 16:39:30 +0200 Subject: [PATCH 6/7] fix tests --- Lib/test/test_zipfile/test_core.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py index c6abd17567a31c..bbfa4323365f78 100644 --- a/Lib/test/test_zipfile/test_core.py +++ b/Lib/test/test_zipfile/test_core.py @@ -2210,11 +2210,11 @@ def test_create_zipinfo_for_name(self): base_filename = TESTFN2.rstrip('/') with zipfile.ZipFile(TESTFN, mode="w", compresslevel=1, - compression=zipfile.ZIP_DEFLATED) as zf: + compression=zipfile.ZIP_STORED) as zf: # no trailing forward slash zi = zipfile.ZipInfo.for_name(base_filename, zf) self.assertEqual(zi.compress_level, 1) - self.assertEqual(zi.compress_type, zipfile.ZIP_DEFLATED) + self.assertEqual(zi.compress_type, zipfile.ZIP_STORED) # ?rw- --- --- filemode = stat.S_IRUSR | stat.S_IWUSR # filemode is stored as the highest 16 bits of external_attr @@ -2222,11 +2222,11 @@ def test_create_zipinfo_for_name(self): self.assertEqual(zi.external_attr & 0xFF, 0) # no MS-DOS flag with zipfile.ZipFile(TESTFN, mode="w", compresslevel=1, - compression=zipfile.ZIP_DEFLATED) as zf: + compression=zipfile.ZIP_STORED) as zf: # with a trailing slash zi = zipfile.ZipInfo.for_name(f'{base_filename}/', zf) self.assertEqual(zi.compress_level, 1) - self.assertEqual(zi.compress_type, zipfile.ZIP_DEFLATED) + self.assertEqual(zi.compress_type, zipfile.ZIP_STORED) # d rwx rwx r-x filemode = stat.S_IFDIR filemode |= stat.S_IRWXU | stat.S_IRWXG From 44e7b75ffe61a735be89325c3b1f9a312124ad9c Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sun, 29 Dec 2024 13:02:49 -0500 Subject: [PATCH 7/7] To avoid passthrough parameters, use _for_archive to mutate a constructed ZipInfo. --- Doc/library/zipfile.rst | 9 ++++----- Doc/whatsnew/3.14.rst | 6 +++--- Lib/test/test_zipfile/_path/test_path.py | 2 +- Lib/test/test_zipfile/test_core.py | 6 +++--- Lib/zipfile/__init__.py | 17 ++++++++--------- ...24-08-28-16-10-37.gh-issue-123424.u96_i6.rst | 4 +--- 6 files changed, 20 insertions(+), 24 deletions(-) diff --git a/Doc/library/zipfile.rst b/Doc/library/zipfile.rst index e888f04ff806ee..afe1cd5c75fcbb 100644 --- a/Doc/library/zipfile.rst +++ b/Doc/library/zipfile.rst @@ -85,13 +85,12 @@ The module defines the following items: continues to work as a property for backwards compatibility. - .. classmethod:: for_name(filename, archive, *, date_time=None) + .. method:: _for_archive(archive) - Construct an appropriate :class:`ZipInfo` from a *filename*, - a :class:`ZipFile` archive and an optional *date_time*. + Resolve the date_time, compression attributes, and external attributes + to suitable defaults as used by :meth:`ZipFile.writestr`. - If *date_time* is not specified, the current local time is used - instead, namely ``date_time = time.localtime(time.time())[:6]``. + Returns self for chaining. .. versionadded:: 3.14 diff --git a/Doc/whatsnew/3.14.rst b/Doc/whatsnew/3.14.rst index ab97e1bd888eb3..53415bb09bf080 100644 --- a/Doc/whatsnew/3.14.rst +++ b/Doc/whatsnew/3.14.rst @@ -664,9 +664,9 @@ uuid zipinfo ------- -* Added :func:`ZipInfo.for_name ` for - constructing a :class:`~zipfile.ZipInfo` object from a filename - and an archive. +* Added :func:`ZipInfo._for_archive ` + to resolve suitable defaults for a :class:`~zipfile.ZipInfo` object + as used by :func:`ZipFile.writestr `. (Contributed by Bénédikt Tran in :gh:`123424`.) diff --git a/Lib/test/test_zipfile/_path/test_path.py b/Lib/test/test_zipfile/_path/test_path.py index fdbab050ffeaa2..1ee45f5fc57104 100644 --- a/Lib/test/test_zipfile/_path/test_path.py +++ b/Lib/test/test_zipfile/_path/test_path.py @@ -634,7 +634,7 @@ def test_backslash_not_separator(self): """ data = io.BytesIO() zf = zipfile.ZipFile(data, "w") - zf.writestr(DirtyZipInfo.for_name("foo\\bar", zf), b"content") + zf.writestr(DirtyZipInfo("foo\\bar")._for_archive(zf), b"content") zf.filename = '' root = zipfile.Path(zf) (first,) = root.iterdir() diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py index c78aca83714635..49f39b9337df85 100644 --- a/Lib/test/test_zipfile/test_core.py +++ b/Lib/test/test_zipfile/test_core.py @@ -2212,13 +2212,13 @@ def test_create_empty_zipinfo_repr(self): zi = zipfile.ZipInfo(filename="empty") self.assertEqual(repr(zi), "") - def test_create_zipinfo_for_name(self): + def test_for_archive(self): base_filename = TESTFN2.rstrip('/') with zipfile.ZipFile(TESTFN, mode="w", compresslevel=1, compression=zipfile.ZIP_STORED) as zf: # no trailing forward slash - zi = zipfile.ZipInfo.for_name(base_filename, zf) + zi = zipfile.ZipInfo(base_filename)._for_archive(zf) self.assertEqual(zi.compress_level, 1) self.assertEqual(zi.compress_type, zipfile.ZIP_STORED) # ?rw- --- --- @@ -2230,7 +2230,7 @@ def test_create_zipinfo_for_name(self): with zipfile.ZipFile(TESTFN, mode="w", compresslevel=1, compression=zipfile.ZIP_STORED) as zf: # with a trailing slash - zi = zipfile.ZipInfo.for_name(f'{base_filename}/', zf) + zi = zipfile.ZipInfo(f'{base_filename}/')._for_archive(zf) self.assertEqual(zi.compress_level, 1) self.assertEqual(zi.compress_type, zipfile.ZIP_STORED) # d rwx rwx r-x diff --git a/Lib/zipfile/__init__.py b/Lib/zipfile/__init__.py index 8ed980981d4996..052ef47b8f6598 100644 --- a/Lib/zipfile/__init__.py +++ b/Lib/zipfile/__init__.py @@ -13,6 +13,7 @@ import sys import threading import time +from typing import Self try: import zlib # We may need its compression method @@ -605,17 +606,15 @@ def from_file(cls, filename, arcname=None, *, strict_timestamps=True): return zinfo - @classmethod - def for_name(cls, filename, archive, *, date_time=None): - """Construct an appropriate ZipInfo from a filename and a ZipFile. + def _for_archive(self, archive: ZipFile) -> Self: + """Resolve suitable defaults from the archive. - The *filename* is expected to be the name of a file in the archive. + Resolve the date_time, compression attributes, and external attributes + to suitable defaults as used by :method:`ZipFile.writestr`. - If *date_time* is not specified, the current local time is used. + Return self. """ - if date_time is None: - date_time = time.localtime(time.time())[:6] - self = cls(filename=filename, date_time=date_time) + self.date_time = time.localtime(time.time())[:6] self.compress_type = archive.compression self.compress_level = archive.compresslevel if self.filename.endswith('/'): # pragma: no cover @@ -1931,7 +1930,7 @@ def writestr(self, zinfo_or_arcname, data, if isinstance(zinfo_or_arcname, ZipInfo): zinfo = zinfo_or_arcname else: - zinfo = ZipInfo.for_name(zinfo_or_arcname, self) + zinfo = ZipInfo(zinfo_or_arcname)._for_archive(self) if not self.fp: raise ValueError( diff --git a/Misc/NEWS.d/next/Library/2024-08-28-16-10-37.gh-issue-123424.u96_i6.rst b/Misc/NEWS.d/next/Library/2024-08-28-16-10-37.gh-issue-123424.u96_i6.rst index 1167887daa939f..4df4bbf2ba2b73 100644 --- a/Misc/NEWS.d/next/Library/2024-08-28-16-10-37.gh-issue-123424.u96_i6.rst +++ b/Misc/NEWS.d/next/Library/2024-08-28-16-10-37.gh-issue-123424.u96_i6.rst @@ -1,3 +1 @@ -Add :meth:`zipfile.ZipInfo.for_name` factory for constructing -:class:`~zipfile.ZipInfo` objects from a filename and an archive. Patch by -Bénédikt Tran. +Add :meth:`zipfile.ZipInfo._for_archive` setting default properties on :class:`~zipfile.ZipInfo` objects. Patch by Bénédikt Tran and Jason R. Coombs.