From 3532a442acc6d9a3793c0b7355060780efd84d50 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Mon, 24 Feb 2025 14:18:30 +0000 Subject: [PATCH 1/5] Fix incorrect zipapp logic to avoid including the target in itself --- Lib/test/test_zipapp.py | 14 ++++++++++++++ Lib/zipapp.py | 8 ++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_zipapp.py b/Lib/test/test_zipapp.py index 00a5ed6626ddc5..c06e4421aa06ef 100644 --- a/Lib/test/test_zipapp.py +++ b/Lib/test/test_zipapp.py @@ -89,6 +89,20 @@ def skip_pyc_files(path): self.assertIn('test.py', z.namelist()) self.assertNotIn('test.pyc', z.namelist()) + def test_create_archive_self_insertion(self): + # When creating an archive, we shouldn't + # include the archive in the lis of files to add. + source = self.tmpdir + (source / '__main__.py').touch() + (source / 'test.py').touch() + target = self.tmpdir / 'target.pyz' + + zipapp.create_archive(source, target) + with zipfile.ZipFile(target, 'r') as z: + self.assertEqual(len(z.namelist()), 2) + self.assertIn('__main__.py', z.namelist()) + self.assertIn('test.py', z.namelist()) + def test_create_archive_filter_exclude_dir(self): # Test packing a directory and using a filter to exclude a # subdirectory (ensures that the path supplied to include diff --git a/Lib/zipapp.py b/Lib/zipapp.py index 03a214efa10a20..7803e0a9021dae 100644 --- a/Lib/zipapp.py +++ b/Lib/zipapp.py @@ -131,14 +131,18 @@ def create_archive(source, target=None, interpreter=None, main=None, elif not hasattr(target, 'write'): target = pathlib.Path(target) + # Create the list of files to add to the archive now, in case + # the target is being created in the source directory - we + # don't want the target being added to itself + files_to_add = sorted(source.rglob('*')) with _maybe_open(target, 'wb') as fd: _write_file_prefix(fd, interpreter) compression = (zipfile.ZIP_DEFLATED if compressed else zipfile.ZIP_STORED) with zipfile.ZipFile(fd, 'w', compression=compression) as z: - for child in sorted(source.rglob('*')): + for child in files_to_add: arcname = child.relative_to(source) - if filter is None or filter(arcname) and child.resolve() != arcname.resolve(): + if filter is None or filter(arcname): z.write(child, arcname.as_posix()) if main_py: z.writestr('__main__.py', main_py.encode('utf-8')) From 60a96ffd86f3d008ae8e9e4a21f94b30a528d039 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Mon, 24 Feb 2025 14:46:34 +0000 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2025-02-24-14-46-20.gh-issue-130379.lsef7A.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2025-02-24-14-46-20.gh-issue-130379.lsef7A.rst diff --git a/Misc/NEWS.d/next/Library/2025-02-24-14-46-20.gh-issue-130379.lsef7A.rst b/Misc/NEWS.d/next/Library/2025-02-24-14-46-20.gh-issue-130379.lsef7A.rst new file mode 100644 index 00000000000000..157b2836491c1d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-02-24-14-46-20.gh-issue-130379.lsef7A.rst @@ -0,0 +1 @@ +The zipapp module now calculates the list of files to be added to the archive before creating the archive. This avoids accidentally including the target when it is being created in the source directory. From b6ec2bf1806e5aa28fc4d2d55d84a356d9e39259 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Mon, 24 Feb 2025 14:50:50 +0000 Subject: [PATCH 3/5] Fix a whitespace issue --- Lib/test/test_zipapp.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_zipapp.py b/Lib/test/test_zipapp.py index c06e4421aa06ef..48f1dbaaeff3f1 100644 --- a/Lib/test/test_zipapp.py +++ b/Lib/test/test_zipapp.py @@ -102,7 +102,7 @@ def test_create_archive_self_insertion(self): self.assertEqual(len(z.namelist()), 2) self.assertIn('__main__.py', z.namelist()) self.assertIn('test.py', z.namelist()) - + def test_create_archive_filter_exclude_dir(self): # Test packing a directory and using a filter to exclude a # subdirectory (ensures that the path supplied to include From 43210be9e9f846445b642bc9edde2d8a640e637c Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Mon, 24 Feb 2025 23:04:23 +0000 Subject: [PATCH 4/5] Report an error if the target overwrites a source file --- Lib/test/test_zipapp.py | 12 +++++++++++- Lib/zipapp.py | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_zipapp.py b/Lib/test/test_zipapp.py index 48f1dbaaeff3f1..ad132839622f48 100644 --- a/Lib/test/test_zipapp.py +++ b/Lib/test/test_zipapp.py @@ -91,7 +91,7 @@ def skip_pyc_files(path): def test_create_archive_self_insertion(self): # When creating an archive, we shouldn't - # include the archive in the lis of files to add. + # include the archive in the list of files to add. source = self.tmpdir (source / '__main__.py').touch() (source / 'test.py').touch() @@ -103,6 +103,16 @@ def test_create_archive_self_insertion(self): self.assertIn('__main__.py', z.namelist()) self.assertIn('test.py', z.namelist()) + def test_target_overwrites_source_file(self): + # The target cannot be one of the files to add. + source = self.tmpdir + (source / '__main__.py').touch() + target = source / 'target.pyz' + target.touch() + + with self.assertRaises(zipapp.ZipAppError): + zipapp.create_archive(source, target) + def test_create_archive_filter_exclude_dir(self): # Test packing a directory and using a filter to exclude a # subdirectory (ensures that the path supplied to include diff --git a/Lib/zipapp.py b/Lib/zipapp.py index 7803e0a9021dae..e78928ba3340aa 100644 --- a/Lib/zipapp.py +++ b/Lib/zipapp.py @@ -135,6 +135,26 @@ def create_archive(source, target=None, interpreter=None, main=None, # the target is being created in the source directory - we # don't want the target being added to itself files_to_add = sorted(source.rglob('*')) + + # The target cannot be in the list of files to add. If it were, we'd + # end up overwriting the source file and writing the archive into + # itself, which is an error. We therefore check for that case and + # provide a helpful message for the user. + + # Note that we only do a simple path equality check. This won't + # catch every case, but it will catch the common case where the + # source is the CWD and the target is a file in the CWD. More + # thorough checks don't provide enough value to justify the extra + # cost. + + # https://github.com/python/cpython/issues/104527 tracks making + # the zipfile module catch writing an archive to itself at a + # lower level, which could help here in cases that our check + # doesn't catch. + if target in files_to_add: + raise ZipAppError( + f"The target archive {target} overwrites one of the source files.") + with _maybe_open(target, 'wb') as fd: _write_file_prefix(fd, interpreter) compression = (zipfile.ZIP_DEFLATED if compressed else From 8eaf2d83008070e784e6cb10e5c7718437f14e0a Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 25 Feb 2025 12:38:01 +0000 Subject: [PATCH 5/5] Clarify comments based on feedback --- Lib/zipapp.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Lib/zipapp.py b/Lib/zipapp.py index e78928ba3340aa..4ffacc49fa753d 100644 --- a/Lib/zipapp.py +++ b/Lib/zipapp.py @@ -147,10 +147,9 @@ def create_archive(source, target=None, interpreter=None, main=None, # thorough checks don't provide enough value to justify the extra # cost. - # https://github.com/python/cpython/issues/104527 tracks making - # the zipfile module catch writing an archive to itself at a - # lower level, which could help here in cases that our check - # doesn't catch. + # If target is a file-like object, it will simply fail to compare + # equal to any of the entries in files_to_add, so there's no need + # to add a special check for that. if target in files_to_add: raise ZipAppError( f"The target archive {target} overwrites one of the source files.")