gh-100256: Skip inaccessible registry keys in the WinAPI mimetype implementation #122047

LucasEsposito · 2024-07-19T19:12:54Z

Issue: Ensurepip fails ungracefully when mimetype is missing from Windows registry #100256

Currently there are two implementations to load mimetypes in Windows from registry keys:

WinReg: Former and slower implementation written in Python. Currently used as fallback. If it can't access a registry key, that key is just skipped and continues with the next.
WinAPI: Faster implementation in C. It's prioritized over WinReg one. If it can't access a registry key, it fails.

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.

cpython-cla-bot · 2024-07-19T19:12:57Z

All commit authors signed the Contributor License Agreement.

Modules/_winapi.c

…PI implementation

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

ZeroIntensity

Nice PR, LGTM

zooba · 2024-08-06T22:53:17Z

Modules/_winapi.c

@@ -2806,6 +2806,11 @@ _winapi__mimetypes_read_windows_registry_impl(PyObject *module,
        if (err == ERROR_FILE_NOT_FOUND) {


Suggested change

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).

Thanks for the improvement. It's fixed now

zooba · 2024-08-06T22:54:45Z

Misc/ACKS

@@ -2104,5 +2104,6 @@ Jelle Zijlstra
 Gennadiy Zlobin
 Doug Zongker
 Peter Åstrand
+Lucas Esposito


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").

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

…DrKba.rst Co-authored-by: Steve Dower <steve.dower@microsoft.com>

Modules/_winapi.c

zooba · 2024-08-07T12:07:38Z

If CI passes with that little fix I committed myself, we should be good to merge.

miss-islington-app · 2024-08-07T15:07:29Z

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>

bedevere-app · 2024-08-07T15:07:44Z

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>

bedevere-app · 2024-08-07T15:07:51Z

GH-122787 is a backport of this pull request to the 3.12 branch.

…lementation (GH-122047) (cherry picked from commit 0bd9375) Co-authored-by: Lucas Esposito <LucasEsposito@users.noreply.github.com>

…pe implementation (pythonGH-122047)

…ype implementation (GH-122047) (#122786) gh-100256: Skip inaccessible registry keys in the WinAPI mimetype implementation (GH-122047) (cherry picked from commit 0bd9375) Co-authored-by: Lucas Esposito <LucasEsposito@users.noreply.github.com>

LucasEsposito requested a review from a team as a code owner July 19, 2024 19:12

bedevere-app bot mentioned this pull request Jul 19, 2024

Ensurepip fails ungracefully when mimetype is missing from Windows registry #100256

Open

bedevere-app bot added the awaiting review label Jul 19, 2024

LucasEsposito changed the title ~~[3.13pythongh-100256: Read Windows registry with the fallback implementati…~~ [3.13] gh-100256: Read Windows registry with the fallback implementation if the accelerated one fails Jul 19, 2024

LucasEsposito force-pushed the gh-100256-denied_access_to_windows_mimetype_registry_keys branch from 0ffc526 to 7b2e83b Compare July 19, 2024 19:53

LucasEsposito changed the title ~~[3.13] gh-100256: Read Windows registry with the fallback implementation if the accelerated one fails~~ [3.13] gh-100256: Skip inaccessible registry keys in the WinAPI implementation Jul 19, 2024

LucasEsposito changed the title ~~[3.13] gh-100256: Skip inaccessible registry keys in the WinAPI implementation~~ [3.13] gh-100256: Skip inaccessible registry keys in the WinAPI mimetype implementation Jul 19, 2024

LucasEsposito force-pushed the gh-100256-denied_access_to_windows_mimetype_registry_keys branch from 7b2e83b to 6922a1d Compare July 19, 2024 20:07

LucasEsposito changed the title ~~[3.13] gh-100256: Skip inaccessible registry keys in the WinAPI mimetype implementation~~ gh-100256: Skip inaccessible registry keys in the WinAPI mimetype implementation Jul 19, 2024

ZeroIntensity reviewed Jul 21, 2024

View reviewed changes

Modules/_winapi.c Outdated Show resolved Hide resolved

LucasEsposito requested a review from ZeroIntensity July 21, 2024 23:20

LucasEsposito force-pushed the gh-100256-denied_access_to_windows_mimetype_registry_keys branch from 78a4bad to b19eac6 Compare July 21, 2024 23:27

LucasEsposito and others added 2 commits July 22, 2024 01:57

pythongh-100256: Skip inaccessible registry keys in the mimetype WinA…

f70f185

…PI implementation

Update Modules/_winapi.c

ad3c57c

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

LucasEsposito force-pushed the gh-100256-denied_access_to_windows_mimetype_registry_keys branch from b19eac6 to ad3c57c Compare July 21, 2024 23:57

ZeroIntensity approved these changes Jul 22, 2024

View reviewed changes

bedevere-app bot added awaiting core review and removed awaiting review labels Jul 22, 2024

zooba reviewed Aug 6, 2024

View reviewed changes

Misc/NEWS.d/next/Windows/2024-07-19-21-50-54.gh-issue-100256.GDrKba.rst Outdated Show resolved Hide resolved

LucasEsposito and others added 3 commits August 7, 2024 01:10

Update Misc/NEWS.d/next/Windows/2024-07-19-21-50-54.gh-issue-100256.G…

e07dbdf

…DrKba.rst Co-authored-by: Steve Dower <steve.dower@microsoft.com>

merge if branches

8fc0a5d

Fixed position in ACK list

51911a2

LucasEsposito requested a review from zooba August 6, 2024 23:24

zooba added needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes labels Aug 7, 2024

zooba reviewed Aug 7, 2024

View reviewed changes

Modules/_winapi.c Outdated Show resolved Hide resolved

Update Modules/_winapi.c

0d0af5a

zooba merged commit 0bd9375 into python:main Aug 7, 2024
37 checks passed

bedevere-app bot removed the awaiting core review label Aug 7, 2024

bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Aug 7, 2024

bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Aug 7, 2024

zooba pushed a commit that referenced this pull request Aug 7, 2024

gh-100256: Skip inaccessible registry keys in the WinAPI mimetype imp…

8f4892a

…lementation (GH-122047) (cherry picked from commit 0bd9375) Co-authored-by: Lucas Esposito <LucasEsposito@users.noreply.github.com>

blhsing pushed a commit to blhsing/cpython that referenced this pull request Aug 22, 2024

pythongh-100256: Skip inaccessible registry keys in the WinAPI mimety…

b99701a

…pe implementation (pythonGH-122047)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-100256: Skip inaccessible registry keys in the WinAPI mimetype implementation #122047

gh-100256: Skip inaccessible registry keys in the WinAPI mimetype implementation #122047

LucasEsposito commented Jul 19, 2024 •

edited

Loading

cpython-cla-bot bot commented Jul 19, 2024 •

edited

Loading

ZeroIntensity left a comment

zooba Aug 6, 2024

LucasEsposito Aug 6, 2024

zooba Aug 6, 2024

LucasEsposito Aug 6, 2024

zooba commented Aug 7, 2024

miss-islington-app bot commented Aug 7, 2024

bedevere-app bot commented Aug 7, 2024

bedevere-app bot commented Aug 7, 2024

		@@ -2806,6 +2806,11 @@ _winapi__mimetypes_read_windows_registry_impl(PyObject *module,
		if (err == ERROR_FILE_NOT_FOUND) {

	if (err == ERROR_FILE_NOT_FOUND) {
	if (err == ERROR_FILE_NOT_FOUND \|\| err == ERROR_ACCESS_DENIED) {

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

LucasEsposito commented Jul 19, 2024 • edited Loading

cpython-cla-bot bot commented Jul 19, 2024 • edited Loading

ZeroIntensity left a comment

Choose a reason for hiding this comment

zooba Aug 6, 2024

Choose a reason for hiding this comment

LucasEsposito Aug 6, 2024

Choose a reason for hiding this comment

zooba Aug 6, 2024

Choose a reason for hiding this comment

LucasEsposito Aug 6, 2024

Choose a reason for hiding this comment

zooba commented Aug 7, 2024

miss-islington-app bot commented Aug 7, 2024

bedevere-app bot commented Aug 7, 2024

bedevere-app bot commented Aug 7, 2024

LucasEsposito commented Jul 19, 2024 •

edited

Loading

cpython-cla-bot bot commented Jul 19, 2024 •

edited

Loading