diff --git a/crates/torad/src/source/mod.rs b/crates/torad/src/source/mod.rs index 7e3c8cc..620ddb0 100644 --- a/crates/torad/src/source/mod.rs +++ b/crates/torad/src/source/mod.rs @@ -48,15 +48,29 @@ pub(crate) struct SourceResolver { handlers: Vec>, } +/// Builds the HTTP client shared by the Jackett and generic-`.torrent` handlers. +/// +/// Two non-default settings, both load-bearing: +/// +/// - **Manual redirect policy.** Jackett 302-redirects to `magnet:` URIs that +/// reqwest refuses to follow; we inspect `Location` ourselves so the magnet +/// handler can take over. +/// - **No system proxy.** reqwest otherwise honours `HTTP_PROXY`/`ALL_PROXY` +/// from the environment. A self-hosted Jackett is typically reached over +/// loopback, and routing loopback through an inherited proxy breaks it. Note +/// this client also serves arbitrary remote `.torrent` URLs via +/// [`http_torrent`], so the setting applies to those too. +fn build_http_client() -> Result { + reqwest::Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .no_proxy() + .build() + .context("failed to build HTTP client") +} + impl SourceResolver { pub(crate) fn new() -> Result { - // Manual redirect policy: Jackett 302-redirects to `magnet:` URIs that - // reqwest would refuse to follow; we want to inspect `Location` - // ourselves so the magnet handler can take over. - let http_client = reqwest::Client::builder() - .redirect(reqwest::redirect::Policy::none()) - .build() - .context("failed to build HTTP client")?; + let http_client = build_http_client()?; Ok(Self { handlers: vec![ Box::new(magnet::MagnetHandler), @@ -76,3 +90,59 @@ impl SourceResolver { anyhow::bail!("no handler matched source: {source}") } } + +#[cfg(test)] +mod tests { + use super::*; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; + + /// Serves exactly one minimal HTTP/1.1 response, then stops. Returns the + /// bound address so the test can build a URL for it. + async fn spawn_oneshot_server() -> std::net::SocketAddr { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + if let Ok((mut sock, _)) = listener.accept().await { + let mut buf = [0u8; 1024]; + let _ = sock.read(&mut buf).await; + let _ = sock + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nhi") + .await; + let _ = sock.flush().await; + } + }); + addr + } + + /// Regression for [`build_http_client`]'s `.no_proxy()`: reqwest reads + /// `ALL_PROXY` at build time, so without it this request would be sent to a + /// dead proxy instead of the loopback server and fail. + /// + /// The proxy URL is deliberately `http://`, not `socks5://` — reqwest is + /// built here without its `socks` feature, so a SOCKS proxy would simply be + /// ignored and the test would pass with or without `.no_proxy()`. + /// + /// Env vars are process-global and `set_var` is `unsafe` under the 2024 + /// edition. This is the only test in the crate that touches the + /// environment, and it restores it before yielding, so nothing else here + /// can observe the change — but it is not safe to add a second such test + /// without serialising them. + #[tokio::test] + async fn http_client_ignores_system_proxy() { + let addr = spawn_oneshot_server().await; + + unsafe { std::env::set_var("ALL_PROXY", "http://127.0.0.1:1") }; + let client = build_http_client(); + unsafe { std::env::remove_var("ALL_PROXY") }; + + let response = client + .expect("client should build") + .get(format!("http://{addr}/")) + .send() + .await + .expect("request must reach the loopback server, not the dead proxy"); + assert_eq!(response.status(), reqwest::StatusCode::OK); + assert_eq!(response.text().await.unwrap(), "hi"); + } +}