Skip to content

Conversation

@pbsds
Copy link
Contributor

@pbsds pbsds commented Dec 11, 2023

@bedevere-app
Copy link

bedevere-app bot commented Dec 11, 2023

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@pbsds pbsds changed the title zipfile: fix extractall makedirs race condition gh-112998: zipfile: fix extractall makedirs race condition Dec 12, 2023
@pbsds pbsds force-pushed the fix-zipfile-makedirs branch 3 times, most recently from dc3f81e to ca20b8e Compare December 12, 2023 09:31
@pbsds pbsds force-pushed the fix-zipfile-makedirs branch from ca20b8e to 5e81a45 Compare December 12, 2023 15:03
Comment on lines 1787 to +1791
if not os.path.isdir(targetpath):
os.mkdir(targetpath)
try:
os.mkdir(targetpath)
except FileExistsError:
pass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently it raises FileExistsError if targetpath exists, but is not a directory. With this change it will be silent. It is not good, because it is an error.

Please add a test for extracting a directory in place of existing regular file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such test was added in #114960, and it fails with this PR.

@serhiy-storchaka
Copy link
Member

Thank you for your PR @pbsds. This change needs tests, and writing tests for it is something non-trivial, especially for first-time contributor, so I created a separate PR #115082 with tests and fixes. I added you as a co-author.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants