From 58f7962a2accb93ab54949a66f3514307a2080d2 Mon Sep 17 00:00:00 2001 From: Eric Ratliff Date: Sun, 1 Feb 2026 09:58:02 -0600 Subject: [PATCH] v1.1.0-beta.1: add proxy integration tests (62 tests, all green) Nine end-to-end tests for the proxy feature: 6 network tests exercising every proxy code path through a real hyper forward proxy (TestProxy) and a mockito origin, plus 3 CLI tests verifying flag parsing and error handling. TestProxy binds to 127.0.0.1:0, forwards in absolute-form, counts requests via an atomic so we can assert traffic actually traversed the proxy. Key issues resolved during implementation: - ENV_MUTEX serializes all tests that mutate HTTPS_PROXY/HTTP_PROXY in both the unit test module and the integration suite. Without it, parallel test execution within a single binary produces nondeterministic failures. - reqwest's blocking::Client owns an internal tokio Runtime. Dropping it inside a #[tokio::test] async fn panics on tokio >= 1.49. All reqwest work runs inside spawn_blocking so the Client drops on a thread-pool thread where that's permitted. - service_fn's closure can't carry a return-type annotation, and async blocks don't support one either. The handler is extracted to a named async fn (proxy_handler) so the compiler can see the concrete Result and satisfy serve_connection's Error bound. --- Cargo.toml | 11 ++ src/sdk/proxy.rs | 13 ++ tests/proxy_integration.rs | 390 +++++++++++++++++++++++++++++++++++++ 3 files changed, 414 insertions(+) create mode 100644 tests/proxy_integration.rs diff --git a/Cargo.toml b/Cargo.toml index 54f25fc..1448163 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,6 +63,17 @@ assert_cmd = "2.0" predicates = "3.1" insta = "1.41" +# Proxy integration tests: mockito is the mock origin server; hyper + friends +# are the forward proxy. All three are already in Cargo.lock as transitive +# deps of reqwest — we're just promoting them to explicit dev-deps with the +# features we actually need. +mockito = "1.7" +hyper = { version = "1", features = ["server", "http1", "client"] } +hyper-util = { version = "0.1", features = ["tokio", "client-legacy", "http1"] } +http-body-util = "0.1" +bytes = "1" +tokio = { version = "1", features = ["full"] } + [build-dependencies] ureq = { version = "2.10", features = ["json"] } zip = "2.2" diff --git a/src/sdk/proxy.rs b/src/sdk/proxy.rs index 3a4e671..c7accbb 100644 --- a/src/sdk/proxy.rs +++ b/src/sdk/proxy.rs @@ -209,9 +209,16 @@ pub fn print_offline_instructions() { #[cfg(test)] mod tests { use super::*; + use std::sync::Mutex; + + // Env vars are process-global. cargo test runs tests in parallel within + // a single binary, so any test that sets/removes HTTPS_PROXY or HTTP_PROXY + // must hold this lock for its entire duration. + static ENV_MUTEX: Mutex<()> = Mutex::new(()); #[test] fn no_proxy_flag_forces_direct() { + let _env_lock = ENV_MUTEX.lock().unwrap(); std::env::set_var("HTTPS_PROXY", "http://proxy.example.com:3128"); let config = ProxyConfig::resolve(None, true).unwrap(); assert!(config.url.is_none()); @@ -220,6 +227,7 @@ mod tests { #[test] fn explicit_flag_overrides_env() { + let _env_lock = ENV_MUTEX.lock().unwrap(); std::env::set_var("HTTPS_PROXY", "http://env-proxy.example.com:3128"); let config = ProxyConfig::resolve(Some("http://flag-proxy.example.com:8080"), false).unwrap(); assert_eq!(config.url.as_ref().unwrap().host_str(), Some("flag-proxy.example.com")); @@ -229,6 +237,7 @@ mod tests { #[test] fn picks_up_env_var() { + let _env_lock = ENV_MUTEX.lock().unwrap(); std::env::remove_var("HTTPS_PROXY"); std::env::remove_var("https_proxy"); std::env::set_var("HTTP_PROXY", "http://env-proxy.example.com:3128"); @@ -240,6 +249,7 @@ mod tests { #[test] fn direct_when_nothing_set() { + let _env_lock = ENV_MUTEX.lock().unwrap(); std::env::remove_var("HTTPS_PROXY"); std::env::remove_var("https_proxy"); std::env::remove_var("HTTP_PROXY"); @@ -262,6 +272,7 @@ mod tests { #[test] fn client_builds_direct() { + let _env_lock = ENV_MUTEX.lock().unwrap(); std::env::remove_var("HTTPS_PROXY"); std::env::remove_var("https_proxy"); std::env::remove_var("HTTP_PROXY"); @@ -272,6 +283,7 @@ mod tests { #[test] fn git_proxy_guard_sets_and_restores() { + let _env_lock = ENV_MUTEX.lock().unwrap(); std::env::set_var("HTTPS_PROXY", "http://original:1111"); std::env::set_var("HTTP_PROXY", "http://original:2222"); @@ -291,6 +303,7 @@ mod tests { #[test] fn git_proxy_guard_clears_for_direct() { + let _env_lock = ENV_MUTEX.lock().unwrap(); std::env::set_var("HTTPS_PROXY", "http://should-be-cleared:1111"); std::env::set_var("HTTP_PROXY", "http://should-be-cleared:2222"); diff --git a/tests/proxy_integration.rs b/tests/proxy_integration.rs new file mode 100644 index 0000000..caa854b --- /dev/null +++ b/tests/proxy_integration.rs @@ -0,0 +1,390 @@ +//! Integration tests for proxy support. +//! +//! Architecture per test: +//! +//! weevil (or ProxyConfig::client()) +//! │ --proxy http://127.0.0.1: +//! ▼ +//! TestProxy ← real forwarding proxy, hyper HTTP/1.1 server +//! │ forwards absolute-form URI to origin +//! ▼ +//! mockito origin ← fake dl.google.com / github.com, returns canned bytes +//! +//! This proves traffic actually traverses the proxy, not just that the download +//! works. The TestProxy struct is the only custom code; everything else is +//! standard mockito + assert_cmd. + +use std::convert::Infallible; +use std::net::SocketAddr; +use std::sync::Arc; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::Mutex; + +// Tests that mutate HTTPS_PROXY / HTTP_PROXY must not run concurrently — +// those env vars are process-global. cargo test runs tests in parallel +// within a single binary by default, so we serialize access with this lock. +// Tests that don't touch env vars (or that only use --proxy flag) skip it. +static ENV_MUTEX: Mutex<()> = Mutex::new(()); + +use hyper::body::Bytes; +use hyper::server::conn::http1; +use hyper::service::service_fn; +use hyper_util::client::legacy::Client; +use hyper_util::client::legacy::connect::HttpConnector; +use hyper_util::rt::{TokioExecutor, TokioIo}; +use http_body_util::{BodyExt, Full}; +use tokio::net::TcpListener; + +use weevil::sdk::proxy::ProxyConfig; + +// --------------------------------------------------------------------------- +// TestProxy — a minimal HTTP forward proxy for use in tests. +// +// Binds to 127.0.0.1:0 (OS picks the port), spawns a tokio task to serve +// connections, and shuts down when dropped. Also counts how many requests +// it forwarded so tests can assert the proxy was actually hit. +// --------------------------------------------------------------------------- + +/// A live forwarding proxy bound to a random local port. +struct TestProxy { + addr: SocketAddr, + request_count: Arc, + /// Dropping this handle shuts the server down. + _shutdown: tokio::sync::mpsc::Sender<()>, +} + +/// The actual proxy handler, extracted to a named async fn. +/// +/// You cannot put a return-type annotation on an `async move { }` block, and +/// you cannot use `-> impl Future` on a closure inside `service_fn` because +/// the opaque future type is unnameable. A named `async fn` is the one place +/// Rust lets you write both `async` and an explicit return type in the same +/// spot — the compiler knows the concrete future type and can verify the +/// `Into>` bound that `serve_connection` requires. +async fn proxy_handler( + client: Client>, + counter: Arc, + req: hyper::Request, +) -> Result>, Infallible> { + counter.fetch_add(1, Ordering::Relaxed); + + // The client sends the request in absolute-form: + // GET http://origin:PORT/path HTTP/1.1 + // hyper parses that into req.uri() for us. + let uri = req.uri().clone(); + + // Collect the incoming body so we can forward it. + let body_bytes = req.into_body().collect().await + .map(|b| b.to_bytes()) + .unwrap_or_default(); + + let forwarded = hyper::Request::builder() + .method("GET") + .uri(uri) + .body(Full::new(body_bytes)) + .unwrap(); + + match client.request(forwarded).await { + Ok(upstream_resp) => { + // Collect the upstream body and re-wrap so we return a concrete + // body type (not Incoming). + let status = upstream_resp.status(); + let headers = upstream_resp.headers().clone(); + let collected = upstream_resp.into_body().collect().await + .map(|b| b.to_bytes()) + .unwrap_or_default(); + + let mut resp = hyper::Response::builder() + .status(status) + .body(Full::new(collected)) + .unwrap(); + *resp.headers_mut() = headers; + Ok(resp) + } + Err(_) => Ok( + hyper::Response::builder() + .status(502) + .body(Full::new(Bytes::from("Bad Gateway"))) + .unwrap() + ), + } +} + +impl TestProxy { + async fn start() -> Self { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + + let count = Arc::new(AtomicU64::new(0)); + let count_clone = count.clone(); + + let (shutdown_tx, mut shutdown_rx) = tokio::sync::mpsc::channel::<()>(1); + + let hyper_client: Client> = + Client::builder(TokioExecutor::new()).build_http(); + + tokio::spawn(async move { + loop { + let stream = tokio::select! { + Ok((stream, _peer)) = listener.accept() => stream, + _ = shutdown_rx.recv() => break, + else => break, + }; + + let client = hyper_client.clone(); + let counter = count_clone.clone(); + + tokio::spawn(async move { + let io = TokioIo::new(stream); + // The closure just delegates to proxy_handler. The named + // fn's return type is visible to the compiler so it can + // resolve the Error associated type that serve_connection + // needs. + let _ = http1::Builder::new() + .serve_connection(io, service_fn(move |req| { + proxy_handler(client.clone(), counter.clone(), req) + })) + .await; + }); + } + }); + + Self { + addr, + request_count: count, + _shutdown: shutdown_tx, + } + } + + fn proxy_url(&self) -> String { + format!("http://{}", self.addr) + } + + fn requests_forwarded(&self) -> u64 { + self.request_count.load(Ordering::Relaxed) + } +} + +// --------------------------------------------------------------------------- +// Tests: ProxyConfig::client() talking through TestProxy to mockito. +// +// reqwest's blocking::Client owns an internal tokio Runtime. Dropping it +// inside an async context panics on tokio ≥ 1.49 ("Cannot drop a runtime in +// a context where blocking is not allowed"). Each test therefore does its +// reqwest work — client creation, HTTP calls, and drop — inside +// spawn_blocking, which runs on a thread-pool thread where blocking is fine. +// The mockito origin and TestProxy stay in the async body because they need +// the tokio runtime for their listeners. +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn proxy_forwards_request_to_origin() { + let mut origin = mockito::Server::new_async().await; + let _mock = origin + .mock("GET", "/sdk.zip") + .with_status(200) + .with_header("content-type", "application/octet-stream") + .with_body(b"fake-sdk-bytes".as_slice()) + .create_async() + .await; + + let proxy = TestProxy::start().await; + let proxy_url = proxy.proxy_url(); + let origin_url = origin.url(); + + let (status, body) = tokio::task::spawn_blocking(move || { + let config = ProxyConfig::resolve(Some(&proxy_url), false).unwrap(); + let client = config.client().unwrap(); + let resp = client.get(format!("{}/sdk.zip", origin_url)).send().unwrap(); + (resp.status().as_u16(), resp.text().unwrap()) + }).await.unwrap(); + + assert_eq!(status, 200); + assert_eq!(body, "fake-sdk-bytes"); + assert_eq!(proxy.requests_forwarded(), 1); +} + +#[tokio::test] +async fn no_proxy_bypasses_proxy_entirely() { + let _env_lock = ENV_MUTEX.lock().unwrap(); + + let mut origin = mockito::Server::new_async().await; + let _mock = origin + .mock("GET", "/direct.txt") + .with_status(200) + .with_body("direct-hit") + .create_async() + .await; + + let proxy = TestProxy::start().await; + let proxy_url = proxy.proxy_url(); + let origin_url = origin.url(); + + let (status, body) = tokio::task::spawn_blocking(move || { + std::env::set_var("HTTPS_PROXY", &proxy_url); + let config = ProxyConfig::resolve(None, true).unwrap(); + std::env::remove_var("HTTPS_PROXY"); + let client = config.client().unwrap(); + let resp = client.get(format!("{}/direct.txt", origin_url)).send().unwrap(); + (resp.status().as_u16(), resp.text().unwrap()) + }).await.unwrap(); + + assert_eq!(status, 200); + assert_eq!(body, "direct-hit"); + assert_eq!(proxy.requests_forwarded(), 0); +} + +#[tokio::test] +async fn env_var_proxy_is_picked_up() { + let _env_lock = ENV_MUTEX.lock().unwrap(); + + let mut origin = mockito::Server::new_async().await; + let _mock = origin + .mock("GET", "/env.txt") + .with_status(200) + .with_body("via-env-proxy") + .create_async() + .await; + + let proxy = TestProxy::start().await; + let proxy_url = proxy.proxy_url(); + let origin_url = origin.url(); + + let (status, body) = tokio::task::spawn_blocking(move || { + std::env::remove_var("HTTP_PROXY"); + std::env::remove_var("http_proxy"); + std::env::remove_var("https_proxy"); + std::env::set_var("HTTPS_PROXY", &proxy_url); + let config = ProxyConfig::resolve(None, false).unwrap(); + std::env::remove_var("HTTPS_PROXY"); + let client = config.client().unwrap(); + let resp = client.get(format!("{}/env.txt", origin_url)).send().unwrap(); + (resp.status().as_u16(), resp.text().unwrap()) + }).await.unwrap(); + + assert_eq!(status, 200); + assert_eq!(body, "via-env-proxy"); + assert_eq!(proxy.requests_forwarded(), 1); +} + +#[tokio::test] +async fn explicit_proxy_flag_overrides_env() { + let _env_lock = ENV_MUTEX.lock().unwrap(); + + let mut origin = mockito::Server::new_async().await; + let _mock = origin + .mock("GET", "/override.txt") + .with_status(200) + .with_body("flag-proxy-wins") + .create_async() + .await; + + let decoy = TestProxy::start().await; + let real = TestProxy::start().await; + let decoy_url = decoy.proxy_url(); + let real_url = real.proxy_url(); + let origin_url = origin.url(); + + let (status, body) = tokio::task::spawn_blocking(move || { + std::env::set_var("HTTPS_PROXY", &decoy_url); + let config = ProxyConfig::resolve(Some(&real_url), false).unwrap(); + std::env::remove_var("HTTPS_PROXY"); + let client = config.client().unwrap(); + let resp = client.get(format!("{}/override.txt", origin_url)).send().unwrap(); + (resp.status().as_u16(), resp.text().unwrap()) + }).await.unwrap(); + + assert_eq!(status, 200); + assert_eq!(body, "flag-proxy-wins"); + assert_eq!(real.requests_forwarded(), 1); + assert_eq!(decoy.requests_forwarded(), 0); +} + +#[tokio::test] +async fn proxy_returns_502_when_origin_is_unreachable() { + let proxy = TestProxy::start().await; + let proxy_url = proxy.proxy_url(); + + let status = tokio::task::spawn_blocking(move || { + let config = ProxyConfig::resolve(Some(&proxy_url), false).unwrap(); + let client = config.client().unwrap(); + let resp = client.get("http://127.0.0.1:1/unreachable").send().unwrap(); + resp.status().as_u16() + }).await.unwrap(); + + assert_eq!(status, 502); + assert_eq!(proxy.requests_forwarded(), 1); +} + +#[tokio::test] +async fn multiple_sequential_requests_all_forwarded() { + let mut origin = mockito::Server::new_async().await; + let _m1 = origin.mock("GET", "/a").with_status(200).with_body("aaa").create_async().await; + let _m2 = origin.mock("GET", "/b").with_status(200).with_body("bbb").create_async().await; + let _m3 = origin.mock("GET", "/c").with_status(200).with_body("ccc").create_async().await; + + let proxy = TestProxy::start().await; + let proxy_url = proxy.proxy_url(); + let origin_url = origin.url(); + + let (a, b, c) = tokio::task::spawn_blocking(move || { + let config = ProxyConfig::resolve(Some(&proxy_url), false).unwrap(); + let client = config.client().unwrap(); + let a = client.get(format!("{}/a", origin_url)).send().unwrap().text().unwrap(); + let b = client.get(format!("{}/b", origin_url)).send().unwrap().text().unwrap(); + let c = client.get(format!("{}/c", origin_url)).send().unwrap().text().unwrap(); + (a, b, c) + }).await.unwrap(); + + assert_eq!(a, "aaa"); + assert_eq!(b, "bbb"); + assert_eq!(c, "ccc"); + assert_eq!(proxy.requests_forwarded(), 3); +} + +// --------------------------------------------------------------------------- +// CLI-level tests +// --------------------------------------------------------------------------- + +#[allow(deprecated)] // cargo_bin_cmd! requires assert_cmd ≥ 2.2; we're on 2.1.2 +#[test] +fn cli_help_shows_proxy_flags() { + let mut cmd = assert_cmd::Command::cargo_bin("weevil").unwrap(); + let assert = cmd.arg("--help").assert(); + assert + .success() + .stdout(predicates::prelude::predicate::str::contains("--proxy")) + .stdout(predicates::prelude::predicate::str::contains("--no-proxy")); +} + +#[allow(deprecated)] +#[test] +fn cli_rejects_garbage_proxy_url() { + let mut cmd = assert_cmd::Command::cargo_bin("weevil").unwrap(); + let assert = cmd + .arg("--proxy") + .arg("not-a-url-at-all") + .arg("sdk") + .arg("install") + .assert(); + assert + .failure() + .stderr(predicates::prelude::predicate::str::contains("Invalid --proxy URL")); +} + +#[allow(deprecated)] +#[test] +fn cli_proxy_and_no_proxy_are_mutually_exclusive_in_effect() { + let mut cmd = assert_cmd::Command::cargo_bin("weevil").unwrap(); + let out = cmd + .arg("--proxy") + .arg("http://127.0.0.1:9999") + .arg("--no-proxy") + .arg("sdk") + .arg("install") + .output() + .expect("weevil binary failed to execute"); + + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(!stderr.contains("panic"), "binary panicked: {}", stderr); +} \ No newline at end of file