-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
gh-130379: Fix incorrect zipapp logic to avoid including the target in itself #130509
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
3532a44
60a96ff
b6ec2bf
39aebcb
43210be
8699b81
a9f627f
8eaf2d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -131,14 +131,38 @@ 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('*')) | ||
|
|
||
| # 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: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe it's fine. A file-like object will simply test as not equal to all the I was going to add a note to that effect in the comment, but it's long enough already and I wasn't sure it was worth it. The fact that you pointed it out suggests that it would be, though. |
||
| 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 | ||
| 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')) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think that
zipfileshould catch this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I'll remove that part of the comment. I think the same logic applies here - if adding the check can't be justified in zipfile, it's not worth adding it here either. We can stick with the simple but incomplete check.