From 87c1b90503e5db92642bcbbcb392f200dd3fdfd5 Mon Sep 17 00:00:00 2001 From: Ryan Fowler Date: Mon, 25 May 2026 16:30:27 -0700 Subject: [PATCH] Refresh DNS on cross-host redirects --- AGENTS.md | 1 + src/http/mod.rs | 157 ++++++++++++++++++++++++++++++++----------- tests/integration.rs | 55 ++++++++++++++- 3 files changed, 172 insertions(+), 41 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b8bab19..424fc9d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -97,6 +97,7 @@ metadata/update/DNS/TLS inspection modes, and executes requests via `src/http`. - Rust `--timing` enables DNS pre-resolution timing and wraps reqwest's connector service so the waterfall includes DNS, TCP, TTFB, and Body phases. reqwest does not currently expose a separate TLS handshake duration, so Rust reports the combined TCP/TLS connector phase as TCP timing. - Rust response body paging is controlled by `--pager auto|on|off` or `pager = ...`; `auto` routes terminal stdout through `less -FIRX`, `on` forces the pager, and `off` disables it. Image responses and output-file writes bypass the pager. - Custom/pre-resolved DNS observes timeout budgets before the reqwest client is built: `--connect-timeout` bounds DNS resolution when set, otherwise DNS uses the remaining `--timeout` budget, and DoH lookup clients receive the same budget. +- Custom/pre-resolved DNS is scoped to the request URL; manual redirects that change scheme, host, or port rebuild the reqwest client and resolve the redirect target so `--dns-server`, `-vvv`, and `--timing` stay aligned with the actual target. - Custom UDP DNS uses random query IDs and applies a 5s per-query receive timeout when no request/connect timeout is available, so unresponsive UDP resolvers cannot hang indefinitely. - GitHub Actions run Cargo fmt, clippy, unit tests, and the Rust integration suite. Release builds Cargo archives named for the self-updater, Linux GNU binaries are built with a prebuilt `cargo-zigbuild` against an explicit glibc 2.28 floor, Windows release binaries use the static MSVC CRT, and each archive is uploaded with a SHA-256 sidecar. The release workflow also composes target-specific `RUSTFLAGS` with `--cfg reqwest_unstable`, which is required while reqwest HTTP/3 support is enabled. The release workflow supports manual dry runs via `workflow_dispatch`, uploading archives as workflow artifacts unless explicitly told to upload to an existing GitHub Release. Release builds set `FETCH_VERSION` from the release tag/manual version so the compiled binary reports the published or test version; `Cargo.toml` intentionally remains `0.0.0` unless crate publishing becomes a goal. Local builds derive `FETCH_VERSION` from a matching `v*` git tag, then `git describe`, then `v0.0.0-dev`. - Rust default config discovery on Windows honors `XDG_CONFIG_HOME/fetch/config` and `HOME/.config/fetch/config` before falling back to `AppData/fetch/config`; Windows mTLS integration fixtures use RSA test certificates to stay compatible with reqwest/rustls platform verification. diff --git a/src/http/mod.rs b/src/http/mod.rs index 7d4b9c2..ca15445 100644 --- a/src/http/mod.rs +++ b/src/http/mod.rs @@ -119,42 +119,21 @@ pub async fn execute(cli: &Cli) -> Result { .connect_timeout .map(|seconds| duration_from_seconds("connect-timeout", seconds)) .transpose()?; - let dns_timeout = dns_resolution_timeout(request_timeout, connect_timeout, request_start)?; - let dns_resolution = resolve_dns_for_client(cli, &url, http_version, dns_timeout).await?; - crate::tls::install_default_crypto_provider(); - let mut builder = Client::builder() - .use_rustls_tls() - .no_brotli() - .no_gzip() - .no_zstd(); let connect_timing = ConnectionTiming::default(); - builder = configure_http_version(builder, http_version); - builder = configure_unix_socket(builder, cli.unix.as_deref())?; - builder = configure_http3_local_address(builder, http_version, &url, dns_resolution.as_ref()); - builder = configure_dns_resolution(builder, url.host_str(), dns_resolution.as_ref()); - if cli.timing || (cli.verbose >= 3 && !cli.silent) { - builder = builder.connector_layer(ConnectionTimingLayer::new(connect_timing.clone())); - } - builder = configure_tls(builder, cli)?; - builder = configure_proxy(builder, cli.proxy.as_deref())?; - if cli.insecure { - builder = builder.danger_accept_invalid_certs(true); - } - if let Some(timeout) = remaining_request_timeout(request_timeout, request_start)? { - builder = builder.timeout(timeout); - } - if let Some(timeout) = connect_timeout { - builder = builder.connect_timeout(timeout); - } - if let Some(session) = &session { - builder = builder.cookie_provider(session.cookie_provider()); - } - builder = builder.redirect(redirect::Policy::none()); - let client = builder.build()?; + let client_build = ClientBuildContext { + http_version, + request_timeout, + connect_timeout, + request_start, + session: session.as_ref(), + connect_timing: &connect_timing, + }; + let initial_client = build_client_for_url(cli, &url, &client_build).await?; if cli.grpc && grpc_method.is_none() && grpc_request_requires_schema(cli) { - let schema = crate::grpc::reflection::schema_for_call(cli, &url, &client).await?; + let schema = + crate::grpc::reflection::schema_for_call(cli, &url, &initial_client.client).await?; grpc_method = Some(proto::method_for_url(&schema, &url)?); } let method_name = effective_method(cli); @@ -217,6 +196,7 @@ pub async fn execute(cli: &Cli) -> Result { let mut request_method = method.clone(); let mut request_url = url.clone(); let mut request_body = body.clone(); + let mut request_client = initial_client.clone(); let mut redirect_statuses = Vec::new(); let mut redirect_count = 0_usize; let mut timing = AttemptTiming::start(); @@ -234,7 +214,8 @@ pub async fn execute(cli: &Cli) -> Result { } if cli.verbose >= 3 && !cli.silent - && let Some(dns) = dns_resolution + && let Some(dns) = request_client + .dns_resolution .as_ref() .and_then(|resolution| resolution.timing.as_ref()) { @@ -252,7 +233,7 @@ pub async fn execute(cli: &Cli) -> Result { } let req = build_request( - &client, + &request_client.client, request_method.clone(), request_url.clone(), attempt_headers, @@ -261,7 +242,8 @@ pub async fn execute(cli: &Cli) -> Result { None, )?; timing.set_dns( - dns_resolution + request_client + .dns_resolution .as_ref() .and_then(|resolution| resolution.timing.as_ref()) .map(|dns| dns.duration), @@ -276,10 +258,20 @@ pub async fn execute(cli: &Cli) -> Result { redirect_statuses.push(response.status()); let (next_method, next_body) = redirected_request(request_method, request_body, response.status())?; + let refresh_client = redirect_requires_client_refresh( + cli, + http_version, + &request_url, + &redirect, + ); drain_response_body_bounded(response).await; request_method = next_method; request_url = redirect; request_body = next_body; + if refresh_client { + request_client = + build_client_for_url(cli, &request_url, &client_build).await?; + } redirect_count += 1; continue; } @@ -293,14 +285,17 @@ pub async fn execute(cli: &Cli) -> Result { timing.mark_response_headers(); timing.set_connect(connect_timing.duration()); if cli.verbose >= 3 && !cli.silent { - let connect_target = - connect_debug_target(&response, &request_url, dns_resolution.as_ref()); + let connect_target = connect_debug_target( + &response, + &request_url, + request_client.dns_resolution.as_ref(), + ); timing::print_debug_lines(&timing, &connect_target, cli.color.as_deref()); } let response = apply_digest_challenge( response, DigestRetryContext { - client: &client, + client: &request_client.client, method: request_method, headers: headers.clone(), body: request_body.clone(), @@ -438,6 +433,92 @@ struct DnsResolution { timing: Option, } +#[derive(Clone)] +struct UrlClient { + client: Client, + dns_resolution: Option, +} + +struct ClientBuildContext<'a> { + http_version: Option, + request_timeout: Option, + connect_timeout: Option, + request_start: Instant, + session: Option<&'a crate::session::Session>, + connect_timing: &'a ConnectionTiming, +} + +async fn build_client_for_url( + cli: &Cli, + url: &Url, + context: &ClientBuildContext<'_>, +) -> Result { + let dns_timeout = dns_resolution_timeout( + context.request_timeout, + context.connect_timeout, + context.request_start, + )?; + let dns_resolution = + resolve_dns_for_client(cli, url, context.http_version, dns_timeout).await?; + let mut builder = Client::builder() + .use_rustls_tls() + .no_brotli() + .no_gzip() + .no_zstd(); + builder = configure_http_version(builder, context.http_version); + builder = configure_unix_socket(builder, cli.unix.as_deref())?; + builder = + configure_http3_local_address(builder, context.http_version, url, dns_resolution.as_ref()); + builder = configure_dns_resolution(builder, url.host_str(), dns_resolution.as_ref()); + if cli.timing || (cli.verbose >= 3 && !cli.silent) { + builder = + builder.connector_layer(ConnectionTimingLayer::new(context.connect_timing.clone())); + } + builder = configure_tls(builder, cli)?; + builder = configure_proxy(builder, cli.proxy.as_deref())?; + if cli.insecure { + builder = builder.danger_accept_invalid_certs(true); + } + if let Some(timeout) = + remaining_request_timeout(context.request_timeout, context.request_start)? + { + builder = builder.timeout(timeout); + } + if let Some(timeout) = context.connect_timeout { + builder = builder.connect_timeout(timeout); + } + if let Some(session) = context.session { + builder = builder.cookie_provider(session.cookie_provider()); + } + builder = builder.redirect(redirect::Policy::none()); + Ok(UrlClient { + client: builder.build()?, + dns_resolution, + }) +} + +fn redirect_requires_client_refresh( + cli: &Cli, + http_version: Option, + current: &Url, + next: &Url, +) -> bool { + if url_client_endpoint(current) == url_client_endpoint(next) { + return false; + } + if cli.proxy.is_some() || cli.unix.is_some() { + return false; + } + cli.dns_server.is_some() + || matches!(http_version, Some(HttpVersion::Http3)) + || cli.timing + || (cli.verbose >= 3 && !cli.silent) +} + +fn url_client_endpoint(url: &Url) -> Option<(&str, &str, Option)> { + Some((url.scheme(), url.host_str()?, url.port_or_known_default())) +} + fn dns_resolution_timeout( request_timeout: Option, connect_timeout: Option, diff --git a/tests/integration.rs b/tests/integration.rs index 7b7e224..25955f4 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -912,6 +912,10 @@ fn fake_editor(dir: &Path, body: &str, code: i32) -> PathBuf { } fn start_udp_dns_server(host: &'static str, ip: Ipv4Addr) -> String { + start_udp_dns_server_with_hosts(vec![(host, ip)]) +} + +fn start_udp_dns_server_with_hosts(records: Vec<(&'static str, Ipv4Addr)>) -> String { let socket = UdpSocket::bind("127.0.0.1:0").expect("bind udp dns server"); let addr = socket.local_addr().unwrap().to_string(); thread::spawn(move || { @@ -924,12 +928,14 @@ fn start_udp_dns_server(host: &'static str, ip: Ipv4Addr) -> String { resp.extend_from_slice(&buf[..2]); resp.extend_from_slice(&[0x81, 0x80]); resp.extend_from_slice(&1_u16.to_be_bytes()); - let answer = name == host && qtype == 1; - resp.extend_from_slice(&(if answer { 1_u16 } else { 0_u16 }).to_be_bytes()); + let answer = records + .iter() + .find_map(|(host, ip)| (name == *host && qtype == 1).then_some(*ip)); + resp.extend_from_slice(&(if answer.is_some() { 1_u16 } else { 0_u16 }).to_be_bytes()); resp.extend_from_slice(&0_u16.to_be_bytes()); resp.extend_from_slice(&0_u16.to_be_bytes()); resp.extend_from_slice(&buf[12..question_end]); - if answer { + if let Some(ip) = answer { resp.extend_from_slice(&[0xc0, 0x0c]); resp.extend_from_slice(&1_u16.to_be_bytes()); resp.extend_from_slice(&1_u16.to_be_bytes()); @@ -4332,6 +4338,49 @@ fn dns_over_https_udp_and_inspect_dns_cases() { assert!(res.stderr.contains("TCP")); assert!(res.stderr.contains("TTFB")); + let redirect_location = Arc::new(Mutex::new(String::new())); + let redirect_location_for_handler = Arc::clone(&redirect_location); + let redirect_target = TestServer::start(move |req| { + if req.path == "/start" && req.header("host").starts_with("fetch-redirect-a.test:") { + return TestResponse::status(302, "Found", "") + .header("Location", &redirect_location_for_handler.lock().unwrap()) + .header("Connection", "close"); + } + if req.path == "/final" && req.header("host").starts_with("fetch-redirect-b.test:") { + return TestResponse::ok("redirect custom dns ok"); + } + TestResponse::status(400, "Bad Request", req.header("host")) + }); + let redirect_port = host_port(&redirect_target.url).split(':').nth(1).unwrap(); + *redirect_location.lock().unwrap() = + format!("http://fetch-redirect-b.test:{redirect_port}/final"); + let redirect_dns_addr = start_udp_dns_server_with_hosts(vec![ + ("fetch-redirect-a.test.", Ipv4Addr::new(127, 0, 0, 1)), + ("fetch-redirect-b.test.", Ipv4Addr::new(127, 0, 0, 1)), + ]); + let res = run_fetch(&[ + "--dns-server", + &redirect_dns_addr, + "-vvv", + &format!("http://fetch-redirect-a.test:{redirect_port}/start"), + ]); + assert_exit(&res, 0); + assert_eq!(res.stdout, "redirect custom dns ok"); + assert!(res.stderr.contains("* DNS: fetch-redirect-a.test")); + assert!(res.stderr.contains("* DNS: fetch-redirect-b.test")); + let requests = redirect_target.requests(); + assert_eq!(requests.len(), 2); + assert!( + requests[0] + .header("host") + .starts_with("fetch-redirect-a.test:") + ); + assert!( + requests[1] + .header("host") + .starts_with("fetch-redirect-b.test:") + ); + let unresponsive_dns_addr = start_unresponsive_udp_dns_server(); let res = run_fetch(&[ "--dns-server",