-
-
Notifications
You must be signed in to change notification settings - Fork 32.9k
gh-139134: Fix pathlib.Path.copy() Windows privilege errors with copyfile2 fallback #139135
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
base: main
Are you sure you want to change the base?
gh-139134: Fix pathlib.Path.copy() Windows privilege errors with copyfile2 fallback #139135
Conversation
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 |
43f3124
to
35e01f4
Compare
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 |
1461184
to
85637e2
Compare
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.
Thanks for this :-)
Lib/pathlib/__init__.py
Outdated
try: | ||
copyfile2(source_fspath, str(self)) | ||
except OSError as exc: | ||
winerror = getattr(exc, 'winerror', None) | ||
if (_winapi is not None and | ||
winerror in (_winapi.ERROR_PRIVILEGE_NOT_HELD, | ||
_winapi.ERROR_ACCESS_DENIED)): | ||
self._copy_from_file_fallback(source, preserve_metadata) | ||
return | ||
raise |
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.
try: | |
copyfile2(source_fspath, str(self)) | |
except OSError as exc: | |
winerror = getattr(exc, 'winerror', None) | |
if (_winapi is not None and | |
winerror in (_winapi.ERROR_PRIVILEGE_NOT_HELD, | |
_winapi.ERROR_ACCESS_DENIED)): | |
self._copy_from_file_fallback(source, preserve_metadata) | |
return | |
raise | |
try: | |
copyfile2(source_fspath, str(self)) | |
except OSError as exc: | |
# Use fallback on ERROR_ACCESS_DENIED | |
# or ERROR_PRIVILEGE_NOT_HELD | |
if exc.winerror not in (5, 1314): | |
raise | |
else: | |
return |
This code is Windows-specific because of the if copyfile2:
line above, so we can access exc.winerror
directly I think.
We usually hard-code Windows error numbers rather than using _winapi
in pathlib - I'm not sure if that's wise but I guess let's be consistent.
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.
Thanks—good call. I switched to exc.winerror and check for 5/1314. If we hit either, we fall back to _copy_from_file_fallback; otherwise we re-raise. I also added a tiny comment mapping those to ERROR_ACCESS_DENIED and ERROR_PRIVILEGE_NOT_HELD. Still under the if copyfile2: guard, so Windows-only. Happy to tweak wording or style if you prefer.
5ec9c09
to
f5866e7
Compare
f5866e7
to
9d80882
Compare
try: | ||
import _winapi | ||
except ImportError: | ||
_winapi = None |
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.
try: | |
import _winapi | |
except ImportError: | |
_winapi = None |
This is unused now
try: | ||
copyfile2(source_path, str(self)) | ||
except OSError as exc: | ||
if hasattr(exc, "winerror") and exc.winerror in (5, 1314): |
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.
if hasattr(exc, "winerror") and exc.winerror in (5, 1314): | |
if exc.winerror in (5, 1314): |
IIUC winerror
is guaranteed to exist for any OSError
raised from CopyFile2()
.
@@ -0,0 +1 @@ | |||
Fix ``pathlib.Path.copy()`` failing on Windows when copying files that require elevated privileges (e.g., hidden or system files) by adding a fallback mechanism when ``copyfile2`` encounters privilege-related errors. |
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.
Fix ``pathlib.Path.copy()`` failing on Windows when copying files that require elevated privileges (e.g., hidden or system files) by adding a fallback mechanism when ``copyfile2`` encounters privilege-related errors. | |
Fix :meth:`pathlib.Path.copy` failing on Windows when copying files that require elevated privileges (e.g., hidden or system files) by adding a fallback mechanism when ``CopyFile2()`` encounters privilege-related errors. |
Uh oh!
There was an error while loading. Please reload this page.