Skip to content

Handle proxy variables #2789

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

Merged
merged 6 commits into from
Mar 4, 2025

Conversation

jku
Copy link
Member

@jku jku commented Feb 14, 2025

This PR handles proxy variables (notably http_proxy, https_proxy, no_proxy, all_proxy) when using Urllib3Fetcher:

  • ProxyEnvironment is used like urllib3 PoolManager: it has the same request() method (we could easily implement the whole RequestMethods mixin but since we only use the one method, that didn't seem useful)
  • Internally ProxyEnvironment keeps track of typically 1-3 PoolManagers [non-proxied, http-proxied, https-proxied] and chooses between those based on the requested url and the environment variables
  • There's a bunch of tests: they do not test the actual proxied connection, only whether the correct pool managers were created based on the proxy environment variable values

I did not want to implement this as I think this component should be in urllib3 but it's not a lot of code (maybe 50 lines of actual code) so I can live with it. As far as I know this puts python-tuf back to feature parity with v5.1: this was the only feature that I know of that we lost with the move from requests to urllib3 only.

Fixes #2787

from __future__ import annotations

from typing import Any
from urllib.request import getproxies
Copy link
Member Author

Choose a reason for hiding this comment

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

we could implement this method ourselves as well but I tried to keep this minimal

@jku jku mentioned this pull request Feb 19, 2025
jku added 6 commits February 20, 2025 10:56
urllib3 does not handle this but we do want to support proxy users.

The environment variable handling is slightly simplified from the
requests implementation.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
This does not actually test using tuf through proxies: it only tests
that ProxyEnvironment creates the ProxyManagers that we expect to be
created based on the proxy environment variables.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Add support for leading dots in no_proxy and "*" as a no_proxy value.

Both are supported in requests and based on
https://about.gitlab.com/blog/2021/01/27/we-need-to-talk-no-proxy/
both are somewhat common.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
@jku jku force-pushed the handle-proxy-variables branch from e2b2aec to 98fcd71 Compare February 20, 2025 08:58
@jku
Copy link
Member Author

jku commented Feb 20, 2025

rebased on develop after #2773 merged

@jku jku marked this pull request as ready for review February 20, 2025 09:00
@jku jku requested a review from a team as a code owner February 20, 2025 09:00
@kairoaraujo
Copy link
Contributor

@jku, this is an impressive work! Thank you for dedicating time on it.
I will start reviewing this weekend.

@jku
Copy link
Member Author

jku commented Feb 23, 2025

I think the no_proxy support in requests actually includes specifying ports as well: this implementation does not and I'm thinking of not adding it.

@kairoaraujo kairoaraujo merged commit fee5148 into theupdateframework:develop Mar 4, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

urllib3: Handle proxy environment variables
2 participants