Skip to content

Consider migrating ureq from 2 to 3 (fixes the HTTP_PROXY/NO_PROXY gap in #630 natively) #636

Description

@yangyang-duolingo

Description

ureq 3 rewrote proxy handling, and it fixes #630 for free: the default Agent config now sets proxy: Proxy::try_from_env() unconditionally (config.rs, Config::default()), with no opt-in Cargo feature needed at all (default = ["rustls", "gzip"] in ureq's own Cargo.toml — there's no proxy-from-env feature in 3.x). Proxy::try_from_env() now also handles NO_PROXY/no_proxy natively (proxy.rs), which 2.x never had regardless of configuration — the gap #631 had to work around with its own http_agent()/NO_PROXY-matching code.

So rather than carrying that workaround indefinitely, this proposes bumping ureq to 3 instead, which makes it unnecessary — the plain ureq::get(uri).call() already in src/utils.rs would just work correctly, no custom proxy code needed in this buildpack at all.

What this touches

This is a major version bump of a core dependency, not a scoped bug fix, so flagging it separately per CONTRIBUTING.md's guidance on non-trivial changes, rather than folding it into #631.

I put together a working migration on a branch to make this concrete rather than purely theoretical: migrate-ureq-3 (diff).

Three mechanical API changes, no logic changes:

  • ureq::Error::Status(code, _) → ureq::Error::StatusCode(code) (no longer carries the Response) — one match arm in src/layers/python.rs.
  • response.into_reader() → response.into_body().into_reader() (.call() now returns http::Response<Body> instead of ureq 2's own Response type) — the two download functions in src/utils.rs.
  • Cargo.toml's ureq feature "tls" → "rustls" (renamed upstream).

cargo test (all 48 non-Docker-dependent tests) and cargo clippy --all-targets -- -D warnings both pass on this branch.

Verifying the proxy/NO_PROXY behavior, not just asserting it

Added a test (ureq_default_agent_honours_http_proxy_and_no_proxy in src/utils.rs) that proves this at the socket level rather than just inspecting config: it spins up a fake local HTTP proxy and a fake direct target, sets HTTP_PROXY/NO_PROXY env vars, and confirms:

  • A request to a target not covered by NO_PROXY arrives at the proxy as a CONNECT <host>:<port> tunnel request (ureq 3 tunnels every proxied request via CONNECT, even for a plain http:// target, rather than the older full-URI-in-the-request-line style).
  • A request to a target covered by NO_PROXY lands directly on the real target (an origin-form request line, no proxy involved), and nothing arrives at the proxy for it.

Open questions for maintainers

  • MSRV: ureq 3 requires Rust 1.85 (Cargo.toml); this buildpack already requires 1.99, so no MSRV conflict, but worth double-checking against any other MSRV constraints I'm not aware of.
  • ureq 3 is a from-scratch rewrite beyond just proxying (new http-crate-based API throughout) — worth a wider look at whether anything else in this crate's ureq usage needs attention beyond the three call sites above, and whether the change is worth it on its own merits independent of the proxy fix.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions