-
-
Notifications
You must be signed in to change notification settings - Fork 30k
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
gh-100256: Skip inaccessible registry keys in the WinAPI mimetype implementation #122047
gh-100256: Skip inaccessible registry keys in the WinAPI mimetype implementation #122047
Conversation
0ffc526
to
7b2e83b
Compare
7b2e83b
to
6922a1d
Compare
78a4bad
to
b19eac6
Compare
…PI implementation
Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
b19eac6
to
ad3c57c
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.
Nice PR, LGTM
Modules/_winapi.c
Outdated
@@ -2806,6 +2806,11 @@ _winapi__mimetypes_read_windows_registry_impl(PyObject *module, | |||
if (err == ERROR_FILE_NOT_FOUND) { |
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 (err == ERROR_FILE_NOT_FOUND) { | |
if (err == ERROR_FILE_NOT_FOUND || err == ERROR_ACCESS_DENIED) { |
May as well just do this instead of adding the extra case (we can't PyErr_Set...
here anyway because the GIL is released).
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 the improvement. It's fixed now
Misc/ACKS
Outdated
@@ -2104,5 +2104,6 @@ Jelle Zijlstra | |||
Gennadiy Zlobin | |||
Doug Zongker | |||
Peter Åstrand | |||
Lucas Esposito |
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.
Find the E
names to add yourself to (line immediately below here says to use alphabetical order, which for English alphabets doesn't have to be "rough").
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.
Fixed! My bad.
In a quick look I though the list had no apparent order... but it was just that the list started with first name even though it's sorted by last name. I didn't notice at first
Misc/NEWS.d/next/Windows/2024-07-19-21-50-54.gh-issue-100256.GDrKba.rst
Outdated
Show resolved
Hide resolved
…DrKba.rst Co-authored-by: Steve Dower <steve.dower@microsoft.com>
If CI passes with that little fix I committed myself, we should be good to merge. |
Thanks @LucasEsposito for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13. |
…pe implementation (pythonGH-122047) (cherry picked from commit 0bd9375) Co-authored-by: Lucas Esposito <LucasEsposito@users.noreply.github.com>
GH-122786 is a backport of this pull request to the 3.13 branch. |
…pe implementation (pythonGH-122047) (cherry picked from commit 0bd9375) Co-authored-by: Lucas Esposito <LucasEsposito@users.noreply.github.com>
GH-122787 is a backport of this pull request to the 3.12 branch. |
…pe implementation (pythonGH-122047)
Currently there are two implementations to load mimetypes in Windows from registry keys:
The current behavior of WinAPI results in some problems, as you can read in the issue description and comments.
This PR addresses those discrepancies in the behavior across different implenentations, making the new WinAPI behave on the same way as the fallback WinReg solution.