Skip to content
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-127753: make the proxies handling of urllib.request cacheable #127767

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

NewUserHa
Copy link
Contributor

@NewUserHa NewUserHa commented Dec 9, 2024

this code makes proxy_bypass_registry is also cacheable, to avoid repeating read the registry of windows.

and also makes the comment about CVE of getproxies_environment() together and more obviously

Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

I think this needs a blurb entry.

Lib/urllib/request.py Outdated Show resolved Hide resolved
Lib/urllib/request.py Outdated Show resolved Hide resolved
@NewUserHa
Copy link
Contributor Author

actually, this structure of these functions feels no straightforward, new comers may do not know if use the original proxy_bypass() or getproxies(),
or use getproxies_environment() and getproxies_registry() and proxy_bypass_registry() and getproxy_bypass_registry() to build its own cacheable proxy_bypass() or getproxies() to avoid repeat reading from os.

But it looks not good to change or delete the original behavior of the original proxy_bypass() or getproxies() functions, so to make new users know how to correctly use and cache it if the users want to use, should re-arrange or re-name these original functions, or just add documents for it?

@ZeroIntensity
Copy link
Member

Sure, if the old APIs are too broken to improve, then add something new.

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