Repository navigation
Honour HTTP(S)_PROXY and NO_PROXY for the Python and uv runtime downloads - #631
Closed
yangyang-duolingo wants to merge 2 commits into
Closed
yangyang-duolingo wants to merge 2 commits into
yangyang-duolingo wants to merge 2 commits into
Conversation
The Python and uv runtime downloads (download_and_unpack_zstd_archive / download_and_unpack_nested_gzip_archive in src/utils.rs) use ureq::get(), which ignores HTTP_PROXY/HTTPS_PROXY entirely: ureq's environment-based proxy detection is gated behind its own proxy-from-env Cargo feature, which isn't part of ureq's default feature set and wasn't enabled here either. proxy-from-env activates no additional optional dependencies (it's a plain marker feature), so Cargo.lock needs no changes. Fixes heroku#630
The proxy-from-env Cargo feature only toggles AgentBuilder's default for try_proxy_from_env -- the underlying Proxy::try_from_system() and the try_proxy_from_env() setter are unconditionally compiled in regardless of the feature flag, so calling the setter explicitly needs no Cargo.toml change at all. More importantly, ureq has no NO_PROXY support of its own (confirmed against 2.12.1's src/proxy.rs): the previous approach would have sent every request through the configured proxy unconditionally, with no way to bypass it for any host. This adds a small NO_PROXY matcher (entries matched by exact host, subdomain, or '*') so a host in NO_PROXY still connects directly, and tests both the URI-authority parsing and the NO_PROXY matching logic directly. Adapted from the same fix already written, tested, and merged in our downstream fork (duolingo/buildpacks-python#18), which hit this exact issue running behind a mandatory HTTP proxy with its own NO_PROXY exclusions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
yangyang-duolingo
changed the title
3 of 4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #630.
ureq::get()(used for both the Python runtime download and the uv download,download_and_unpack_zstd_archive/download_and_unpack_nested_gzip_archiveinsrc/utils.rs) ignoresHTTP_PROXY/HTTPS_PROXYentirely — it uses the crate's default globalAgent, which never looks at the environment unless told to.Earlier revision of this PR tried fixing this by enabling
ureq'sproxy-from-envCargo feature. That turned out to be unnecessary and insufficient:proxy-from-envonly changes the default valueAgentBuilderinitializestry_proxy_from_envto. The setter (AgentBuilder::try_proxy_from_env()) andProxy::try_from_system()are compiled in regardless of the feature flag, so calling the setter explicitly needs noCargo.tomlchange at all.ureqhas noNO_PROXYsupport of its own (checkedsrc/proxy.rsin 2.12.1 —Proxy::try_from_system()only ever readsALL_PROXY/HTTPS_PROXY/HTTP_PROXY). Enabling the feature as a blanket default would've sent every request through the configured proxy unconditionally, with no way to bypass it for any host.This revision instead builds a small per-request
Agentviahttp_agent(): it checks the request's host againstNO_PROXY/no_proxy(matching by exact host, subdomain, or a bare*) and only asks the agent to look at the proxy env vars when the host isn't excluded. Includes unit tests for both the URI-authority parsing and theNO_PROXYmatching logic.Verification steps
ureq2.12.1 source (Cargo.toml,src/agent.rs,src/proxy.rs) thattry_proxy_from_env()/Proxy::try_from_system()aren't gated by theproxy-from-envfeature, and thattry_from_system()has noNO_PROXYhandling of its owncargo test(all 49 non-Docker-dependent tests pass, including the two new ones) andcargo clippy --all-targets -- -D warnings(clean) in a freshly-installed local toolchain