From 4c0f79edefcd78e3b1b19d4cf032a7241cf93250 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Mon, 24 Aug 2026 04:43:23 +0800 Subject: [PATCH 1/4] feat(fetch): support request methods and bodies --- moli-test-support/src/routes_core.rs | 39 +++++++++++++ moli-test-support/src/server_routes.rs | 6 ++ moli/src/app.rs | 15 +++-- moli/src/cli.rs | 41 ++++++++++++++ moli/tests/cli.rs | 42 ++++++++++++++ moli/tests/fetch_cli.rs | 55 +++++++++++++++++++ skills/moli-webfetch/SKILL.md | 3 +- .../moli-webfetch/references/fetch-recipes.md | 3 + 8 files changed, 197 insertions(+), 7 deletions(-) diff --git a/moli-test-support/src/routes_core.rs b/moli-test-support/src/routes_core.rs index 02264ee16..900acf121 100644 --- a/moli-test-support/src/routes_core.rs +++ b/moli-test-support/src/routes_core.rs @@ -5551,6 +5551,45 @@ pub(super) async fn redirect_page() -> Redirect { Redirect::temporary("/static") } +pub(super) async fn initial_request_redirect_307() -> Response { + Response::builder() + .status(StatusCode::TEMPORARY_REDIRECT) + .header("location", "/request-redirect/final") + .body(Body::empty()) + .expect("307 redirect response should build") +} + +pub(super) async fn initial_request_redirect_308() -> Response { + Response::builder() + .status(StatusCode::PERMANENT_REDIRECT) + .header("location", "/request-redirect/final") + .body(Body::empty()) + .expect("308 redirect response should build") +} + +pub(super) async fn initial_request_redirect_final(request: AxumRequest) -> Html { + let method = request.method().as_str().to_owned(); + let content_type = request + .headers() + .get(CONTENT_TYPE) + .and_then(|value| value.to_str().ok()) + .unwrap_or_default() + .to_owned(); + let marker = request + .headers() + .get("x-moli-redirect-test") + .and_then(|value| value.to_str().ok()) + .unwrap_or_default() + .to_owned(); + let body = axum::body::to_bytes(request.into_body(), 1024 * 1024) + .await + .expect("redirect test request body should be readable"); + let body = String::from_utf8_lossy(&body); + Html(format!( + "
method={method};body={body};content-type={content_type};marker={marker}
" + )) +} + pub(super) async fn redirect_cookie_page() -> Response { ( [(SET_COOKIE, "session=fixture; Path=/; HttpOnly")], diff --git a/moli-test-support/src/server_routes.rs b/moli-test-support/src/server_routes.rs index 2a0dec382..ff50990d0 100644 --- a/moli-test-support/src/server_routes.rs +++ b/moli-test-support/src/server_routes.rs @@ -2466,6 +2466,12 @@ pub(super) fn build_router() -> Router { get(asset_parser_image_fetch_policy_css), ) .route("/redirect", get(redirect_page)) + .route("/request-redirect/307", any(initial_request_redirect_307)) + .route("/request-redirect/308", any(initial_request_redirect_308)) + .route( + "/request-redirect/final", + any(initial_request_redirect_final), + ) .route("/redirect-cookie", get(redirect_cookie_page)) .route("/cookie", get(cookie_page)) .route("/cookie-location-gate", get(cookie_location_gate_page)) diff --git a/moli/src/app.rs b/moli/src/app.rs index 9ba8841b2..4b8745bd0 100644 --- a/moli/src/app.rs +++ b/moli/src/app.rs @@ -42,7 +42,7 @@ pub async fn run_cli_with_config( ) -> Result<()> { match cli.command { Commands::Fetch(args) => { - let request = build_fetch_request(&args.url, &config)?; + let request = build_fetch_request(&args, &config)?; if config.browser.fetch().obey_robots() { // Checked before the browser starts so a refused fetch costs // nothing but the robots.txt request itself. @@ -145,11 +145,14 @@ pub async fn run_cli_with_config( Ok(()) } -fn build_fetch_request(url: &str, config: &AppConfig) -> Result { - let mut request = Request::get(url)?; - // Keep CLI-provided headers scoped to the initial document navigation. - request.request_headers = config.fetch.request_headers.clone(); - Ok(request) +fn build_fetch_request(args: &crate::cli::FetchArgs, config: &AppConfig) -> Result { + Request::new( + &args.method, + &args.url, + args.body.clone(), + config.fetch.request_headers.clone(), + ) + .map(Request::with_top_level_navigation_cookie_context) } struct CliFetchFailureContext { diff --git a/moli/src/cli.rs b/moli/src/cli.rs index 2bbcf72d1..4dd4392a6 100644 --- a/moli/src/cli.rs +++ b/moli/src/cli.rs @@ -47,6 +47,20 @@ pub struct FetchArgs { #[arg(short = 'H', long = "header", value_name = "HEADER", value_parser = parse_request_header_arg)] pub headers: Vec, + /// HTTP method for the initial document request. + #[arg( + short = 'X', + long, + value_name = "METHOD", + default_value = "GET", + value_parser = parse_request_method + )] + pub method: String, + + /// UTF-8 body for the initial document request. + #[arg(long, value_name = "TEXT")] + pub body: Option, + #[arg(long)] pub noscript: bool, @@ -314,6 +328,33 @@ fn parse_request_header_arg(raw: &str) -> Result { }) } +fn parse_request_method(raw: &str) -> Result { + let is_token_byte = |byte: u8| { + byte.is_ascii_alphanumeric() + || matches!( + byte, + b'!' | b'#' + | b'$' + | b'%' + | b'&' + | b'\'' + | b'*' + | b'+' + | b'-' + | b'.' + | b'^' + | b'_' + | b'`' + | b'|' + | b'~' + ) + }; + if raw.is_empty() || !raw.bytes().all(is_token_byte) { + return Err("HTTP method must be a non-empty RFC 9110 token".to_owned()); + } + Ok(raw.to_owned()) +} + #[derive(Debug, Clone, PartialEq, Eq, Args)] pub struct ServeArgs { #[arg(long, default_value = "127.0.0.1")] diff --git a/moli/tests/cli.rs b/moli/tests/cli.rs index f1be3ebe5..b1295a30d 100644 --- a/moli/tests/cli.rs +++ b/moli/tests/cli.rs @@ -74,6 +74,8 @@ fn parses_explicit_fetch_command_with_compatibility_flags() { cli.command, Commands::Fetch(Box::new(FetchArgs { dump: Some(DumpFormat::SemanticTree), + method: "GET".to_owned(), + body: None, headers: vec![ RequestHeaderArg { name: "X-Test".to_owned(), @@ -412,6 +414,8 @@ fn infers_fetch_mode_from_bare_url() { cli.command, Commands::Fetch(Box::new(FetchArgs { dump: None, + method: "GET".to_owned(), + body: None, headers: vec![], noscript: false, with_base: false, @@ -452,6 +456,8 @@ fn parses_bare_dump_with_explicit_fetch_command_and_defaults_to_html() { cli.command, Commands::Fetch(Box::new(FetchArgs { dump: Some(DumpFormat::Html), + method: "GET".to_owned(), + body: None, headers: vec![], noscript: false, with_base: false, @@ -493,6 +499,8 @@ fn parses_header_flag_with_explicit_fetch_command() { cli.command, Commands::Fetch(Box::new(FetchArgs { dump: None, + method: "GET".to_owned(), + body: None, headers: vec![RequestHeaderArg { name: "X-Test".to_owned(), value: "one".to_owned(), @@ -802,6 +810,40 @@ fn app_config_preserves_repeatable_request_headers() { assert!(config.browser.fetch().default_request_headers().is_empty()); } +#[test] +fn parses_initial_request_method_and_body() { + let cli = Cli::try_parse_from(normalize_args_for_compat([ + "moli", + "fetch", + "-X", + "POST", + "--body", + "hello=moli", + "https://example.com", + ])) + .unwrap(); + + let Commands::Fetch(args) = cli.command else { + panic!("expected fetch command"); + }; + assert_eq!(args.method, "POST"); + assert_eq!(args.body.as_deref(), Some("hello=moli")); +} + +#[test] +fn rejects_invalid_initial_request_method() { + let error = Cli::try_parse_from(normalize_args_for_compat([ + "moli", + "fetch", + "--method", + "NOT VALID", + "https://example.com", + ])) + .unwrap_err(); + + assert_eq!(error.kind(), clap::error::ErrorKind::ValueValidation); +} + struct TempWebBotAuthKeyFile { path: PathBuf, } diff --git a/moli/tests/fetch_cli.rs b/moli/tests/fetch_cli.rs index 08174815e..47f269343 100644 --- a/moli/tests/fetch_cli.rs +++ b/moli/tests/fetch_cli.rs @@ -2776,6 +2776,61 @@ fn cli_http_redirect_with_location_does_not_consume_redirect_wait() -> Result<() Ok(()) } +fn assert_cli_request_redirect_preserves_method_body_and_headers(status: u16) -> Result<()> { + let runtime = tokio::runtime::Runtime::new()?; + let server = runtime.block_on(FixtureServer::spawn())?; + let url = server.url(&format!("/request-redirect/{status}")); + let body = format!("payload-{status}"); + let content_type = format!("application/x-moli-{status}"); + let content_type_header = format!("Content-Type: {content_type}"); + let marker = format!("redirect-{status}"); + let marker_header = format!("X-Moli-Redirect-Test: {marker}"); + let output = run_fetch_cli_with_dump_and_args( + &url, + "html", + &[ + "--method", + "POST", + "--body", + &body, + "--header", + &content_type_header, + "--header", + &marker_header, + ], + )?; + runtime.block_on(server.shutdown()); + + assert!( + output.status.success(), + "moli fetch failed: stdout={}\nstderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + let stdout = clean_output(&output.stdout); + assert!(stdout.contains("method=POST"), "stdout={stdout}"); + assert!(stdout.contains(&format!("body={body}")), "stdout={stdout}"); + assert!( + stdout.contains(&format!("content-type={content_type}")), + "stdout={stdout}" + ); + assert!( + stdout.contains(&format!("marker={marker}")), + "stdout={stdout}" + ); + Ok(()) +} + +#[test] +fn cli_307_redirect_preserves_initial_request_method_body_and_headers() -> Result<()> { + assert_cli_request_redirect_preserves_method_body_and_headers(307) +} + +#[test] +fn cli_308_redirect_preserves_initial_request_method_body_and_headers() -> Result<()> { + assert_cli_request_redirect_preserves_method_body_and_headers(308) +} + #[test] fn cli_dump_json_includes_title_headers_and_redirect_chain() -> Result<()> { let runtime = tokio::runtime::Runtime::new()?; diff --git a/skills/moli-webfetch/SKILL.md b/skills/moli-webfetch/SKILL.md index 5896ac2ef..72b05134a 100644 --- a/skills/moli-webfetch/SKILL.md +++ b/skills/moli-webfetch/SKILL.md @@ -123,7 +123,8 @@ unrelated actions. - Use `--cookie-file` or `--profile-dir` only for state the user is authorized to use. Never expose headers, cookies, or tokens in the response. - Remember that `-H/--header` applies to the initial navigation, not every - subresource. + subresource. `-X/--method` and `--body` likewise configure only the initial + navigation. - Treat stdout as the requested artifact. Redirect screenshot, full-document screenshot, and PDF output to files, verify that they are non-empty and have the expected type, and never print their binary bytes into a text response. diff --git a/skills/moli-webfetch/references/fetch-recipes.md b/skills/moli-webfetch/references/fetch-recipes.md index 719bdcf2e..d9e327112 100644 --- a/skills/moli-webfetch/references/fetch-recipes.md +++ b/skills/moli-webfetch/references/fetch-recipes.md @@ -148,6 +148,9 @@ For a crawl rather than a single lookup: ## Request State and Policy - Add initial navigation headers with repeated `-H 'Name: Value'`. +- Set the initial navigation method with `-X/--method METHOD`; add a UTF-8 body + with `--body TEXT`. Normal HTTP redirect rules apply, including method/body + preservation across 307 and 308 responses. - Import cookie files with repeated `--cookie-file`. - Use `--profile-dir` when state must persist across invocations; it also provides the default HTTP cache location unless `--http-cache-dir` is set. From 861fd5e5a58705eb371d550ea2a511bddb8758d0 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Mon, 24 Aug 2026 21:19:18 +0800 Subject: [PATCH 2/4] feat(fetch): send request bodies with GET --- moli-fetch/src/blocking/mod.rs | 30 ++++++++++--- moli/src/app.rs | 5 ++- moli/src/cli.rs | 3 +- moli/tests/fetch_cli.rs | 82 +++++++++++++++++++++++++++++++--- 4 files changed, 107 insertions(+), 13 deletions(-) diff --git a/moli-fetch/src/blocking/mod.rs b/moli-fetch/src/blocking/mod.rs index 42a5748d7..d2308a458 100644 --- a/moli-fetch/src/blocking/mod.rs +++ b/moli-fetch/src/blocking/mod.rs @@ -661,8 +661,23 @@ pub(crate) fn configure_easy( easy.url(request_url.as_str()) .with_context(|| anyhow!("failed to set curl request url to {}", request_url))?; + if request.method.eq_ignore_ascii_case("HEAD") && request.body.is_some() { + bail!("HEAD request bodies are not supported"); + } + match request.method.as_str() { - "GET" => easy.get(true).context("failed to configure GET request")?, + "GET" if request.body.is_none() => { + easy.get(true).context("failed to configure GET request")? + } + "GET" => { + // CURLOPT_HTTPGET resets libcurl's upload state. Use a custom GET + // method when a body is present so CURLOPT_POSTFIELDS remains on + // the wire while response handling retains normal GET semantics. + easy.custom_request("GET") + .context("failed to configure GET request with body")?; + easy.post_fields_copy(request.body.as_deref().unwrap_or_default()) + .context("failed to set GET body")?; + } "HEAD" => easy .nobody(true) .context("failed to configure HEAD request")?, @@ -739,13 +754,16 @@ pub(crate) fn configure_easy( has_headers = true; } } - if request.method.eq_ignore_ascii_case("POST") && !has_content_type_header { - // libcurl otherwise synthesizes `Content-Type: application/x-www-form-urlencoded` - // for POST bodies. Browser fetch/sendBeacon only send Content-Type when - // BodyInit or caller headers produce one, so suppress curl's transport default. + if (request.method.eq_ignore_ascii_case("POST") + || request.method.eq_ignore_ascii_case("GET") && request.body.is_some()) + && !has_content_type_header + { + // CURLOPT_POSTFIELDS otherwise makes libcurl synthesize + // `Content-Type: application/x-www-form-urlencoded`, including for a + // custom GET. A generic Moli request body has no implied media type. headers .append("Content-Type:") - .context("failed to suppress curl default POST content-type")?; + .context("failed to suppress curl default request body content-type")?; has_headers = true; } diff --git a/moli/src/app.rs b/moli/src/app.rs index 4b8745bd0..05c1e9d9d 100644 --- a/moli/src/app.rs +++ b/moli/src/app.rs @@ -11,7 +11,7 @@ use crate::{ cookie_cache, fetch_dump, robots, }; use anyhow::Result; -use anyhow::{Context, anyhow}; +use anyhow::{Context, anyhow, bail}; use clap::Parser; use moli_core::runtime::{ Browser, FetchReadinessTimeout, FetchedDocument, NavigationRuntimeConfig, @@ -146,6 +146,9 @@ pub async fn run_cli_with_config( } fn build_fetch_request(args: &crate::cli::FetchArgs, config: &AppConfig) -> Result { + if args.method.eq_ignore_ascii_case("HEAD") && args.body.is_some() { + bail!("HEAD request bodies are not supported"); + } Request::new( &args.method, &args.url, diff --git a/moli/src/cli.rs b/moli/src/cli.rs index 4dd4392a6..e893e2014 100644 --- a/moli/src/cli.rs +++ b/moli/src/cli.rs @@ -57,7 +57,8 @@ pub struct FetchArgs { )] pub method: String, - /// UTF-8 body for the initial document request. + /// UTF-8 body for the initial document request. GET bodies are sent; + /// HEAD bodies are rejected. #[arg(long, value_name = "TEXT")] pub body: Option, diff --git a/moli/tests/fetch_cli.rs b/moli/tests/fetch_cli.rs index 47f269343..490eb5188 100644 --- a/moli/tests/fetch_cli.rs +++ b/moli/tests/fetch_cli.rs @@ -2776,7 +2776,10 @@ fn cli_http_redirect_with_location_does_not_consume_redirect_wait() -> Result<() Ok(()) } -fn assert_cli_request_redirect_preserves_method_body_and_headers(status: u16) -> Result<()> { +fn assert_cli_request_redirect_preserves_method_body_and_headers( + status: u16, + method: &str, +) -> Result<()> { let runtime = tokio::runtime::Runtime::new()?; let server = runtime.block_on(FixtureServer::spawn())?; let url = server.url(&format!("/request-redirect/{status}")); @@ -2790,7 +2793,7 @@ fn assert_cli_request_redirect_preserves_method_body_and_headers(status: u16) -> "html", &[ "--method", - "POST", + method, "--body", &body, "--header", @@ -2808,7 +2811,10 @@ fn assert_cli_request_redirect_preserves_method_body_and_headers(status: u16) -> String::from_utf8_lossy(&output.stderr) ); let stdout = clean_output(&output.stdout); - assert!(stdout.contains("method=POST"), "stdout={stdout}"); + assert!( + stdout.contains(&format!("method={method}")), + "stdout={stdout}" + ); assert!(stdout.contains(&format!("body={body}")), "stdout={stdout}"); assert!( stdout.contains(&format!("content-type={content_type}")), @@ -2823,12 +2829,78 @@ fn assert_cli_request_redirect_preserves_method_body_and_headers(status: u16) -> #[test] fn cli_307_redirect_preserves_initial_request_method_body_and_headers() -> Result<()> { - assert_cli_request_redirect_preserves_method_body_and_headers(307) + assert_cli_request_redirect_preserves_method_body_and_headers(307, "POST") } #[test] fn cli_308_redirect_preserves_initial_request_method_body_and_headers() -> Result<()> { - assert_cli_request_redirect_preserves_method_body_and_headers(308) + assert_cli_request_redirect_preserves_method_body_and_headers(308, "POST") +} + +#[test] +fn cli_get_sends_an_inline_utf8_body() -> Result<()> { + let runtime = tokio::runtime::Runtime::new()?; + let server = runtime.block_on(FixtureServer::spawn())?; + let url = server.url("/request-redirect/final"); + let output = run_fetch_cli_with_dump_and_args( + &url, + "html", + &[ + "--method", + "GET", + "--body", + "direct-get-世界", + "--header", + "Content-Type: text/plain; charset=utf-8", + ], + )?; + runtime.block_on(server.shutdown()); + + assert!( + output.status.success(), + "moli fetch failed: stdout={}\nstderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + let stdout = clean_output(&output.stdout); + assert!(stdout.contains("method=GET"), "stdout={stdout}"); + assert!(stdout.contains("body=direct-get-世界"), "stdout={stdout}"); + assert!( + stdout.contains("content-type=text/plain; charset=utf-8"), + "stdout={stdout}", + ); + Ok(()) +} + +#[test] +fn cli_307_redirect_preserves_get_body_and_headers() -> Result<()> { + assert_cli_request_redirect_preserves_method_body_and_headers(307, "GET") +} + +#[test] +fn cli_308_redirect_preserves_get_body_and_headers() -> Result<()> { + assert_cli_request_redirect_preserves_method_body_and_headers(308, "GET") +} + +#[test] +fn cli_head_body_fails_instead_of_silently_dropping_the_body() -> Result<()> { + let runtime = tokio::runtime::Runtime::new()?; + let server = runtime.block_on(FixtureServer::spawn())?; + let url = server.url("/request-redirect/final"); + let output = run_fetch_cli_with_dump_and_args( + &url, + "html", + &["--method", "HEAD", "--body", "must-not-disappear"], + )?; + runtime.block_on(server.shutdown()); + + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("HEAD request bodies are not supported"), + "stderr={stderr}", + ); + Ok(()) } #[test] From c720e172a079eff03f9637e8e57ddf45d421ea49 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Mon, 24 Aug 2026 21:19:32 +0800 Subject: [PATCH 3/4] fix(fetch): strip credentials on cross-origin redirects --- moli-fetch/src/blocking/mod.rs | 143 +++++++++++++++++++++++++++++---- 1 file changed, 128 insertions(+), 15 deletions(-) diff --git a/moli-fetch/src/blocking/mod.rs b/moli-fetch/src/blocking/mod.rs index d2308a458..b26476604 100644 --- a/moli-fetch/src/blocking/mod.rs +++ b/moli-fetch/src/blocking/mod.rs @@ -165,6 +165,7 @@ pub(crate) fn outgoing_request_headers_for_url( cookie_header: Option<&str>, ) -> Vec<(String, String)> { let mut outgoing = Vec::new(); + let request_origin_matches = same_origin(&request.url, request_url); if let Some(proxy_bearer_token) = config.proxy_bearer_token() { outgoing.push(( @@ -185,6 +186,9 @@ pub(crate) fn outgoing_request_headers_for_url( } for (name, value) in &request.request_headers { + if !request_origin_matches && is_origin_bound_request_header(name) { + continue; + } if cookie_header.is_some() && name.eq_ignore_ascii_case("cookie") { continue; } @@ -214,20 +218,6 @@ pub(crate) fn outgoing_request_headers_for_url( )); } - if let Some(auth) = request.auth() - && auth.target == RequestAuthTarget::Server - && auth.scheme == RequestAuthScheme::Basic - && !header_present(&outgoing, "authorization") - { - outgoing.push(( - "Authorization".to_owned(), - format!( - "Basic {}", - encode_basic_auth(&auth.username, &auth.password) - ), - )); - } - if let Some(auth) = request.auth() && auth.target == RequestAuthTarget::ProxyHeader && !header_present(&outgoing, "proxy-authorization") @@ -244,6 +234,10 @@ pub(crate) fn outgoing_request_headers_for_url( outgoing } +fn is_origin_bound_request_header(name: &str) -> bool { + name.eq_ignore_ascii_case("authorization") || name.eq_ignore_ascii_case("cookie") +} + pub(crate) fn network_request_extra_info_from_headers( config: &FetchConfig, outgoing_headers: &[(String, String)], @@ -774,6 +768,7 @@ pub(crate) fn configure_easy( if let Some(auth) = request.auth() && request.auth_requires_buffered_transport() + && (auth.target != RequestAuthTarget::Server || same_origin(&request.url, request_url)) { let mut methods = Auth::new(); match auth.scheme { @@ -961,7 +956,7 @@ fn effective_connect_timeout(config: &FetchConfig, request: &Request) -> Option< #[cfg(test)] mod tests { use super::*; - use crate::RequestMode; + use crate::{RequestAuth, RequestMode}; fn url(value: &str) -> Url { Url::parse(value).expect("valid URL") @@ -1041,6 +1036,124 @@ mod tests { ); } + #[test] + fn explicit_authorization_and_cookie_headers_follow_only_the_original_origin() { + let config = FetchConfig::default(); + let original_url = url("https://app.test/start"); + let cross_host_url = url("https://cdn.test/final"); + let cross_port_url = url("https://app.test:444/final"); + let downgrade_url = url("http://app.test/final"); + let returned_url = url("https://app.test/final"); + let explicit_default_port_url = url("https://app.test:443/final"); + let request = Request::new( + "GET", + original_url.as_str(), + None, + vec![ + ("aUtHoRiZaTiOn".to_owned(), "Bearer secret".to_owned()), + ("cOoKiE".to_owned(), "manual=secret".to_owned()), + ("X-Trace".to_owned(), "keep-me".to_owned()), + ], + ) + .unwrap(); + + let cases = [ + ("initial request", original_url.clone(), Vec::new(), true), + ( + "same-origin redirect", + returned_url.clone(), + vec![redirect(&original_url, &returned_url)], + true, + ), + ( + "same origin with an explicit default port", + explicit_default_port_url.clone(), + vec![redirect(&original_url, &explicit_default_port_url)], + true, + ), + ( + "cross-host redirect", + cross_host_url.clone(), + vec![redirect(&original_url, &cross_host_url)], + false, + ), + ( + "cross-port redirect", + cross_port_url.clone(), + vec![redirect(&original_url, &cross_port_url)], + false, + ), + ( + "scheme downgrade", + downgrade_url.clone(), + vec![redirect(&original_url, &downgrade_url)], + false, + ), + ( + "return to original origin", + returned_url.clone(), + vec![ + redirect(&original_url, &cross_host_url), + redirect(&cross_host_url, &returned_url), + ], + true, + ), + ]; + + for (name, request_url, redirect_chain, keeps_sensitive_headers) in cases { + let headers = outgoing_request_headers_for_url( + &config, + &request, + &request_url, + &redirect_chain, + None, + ); + assert_eq!( + header_value(&headers, "authorization").is_some(), + keeps_sensitive_headers, + "{name}", + ); + assert_eq!( + header_value(&headers, "cookie").is_some(), + keeps_sensitive_headers, + "{name}", + ); + assert_eq!( + header_value(&headers, "x-trace").as_deref(), + Some("keep-me"), + "ordinary explicit headers should survive {name}", + ); + } + } + + #[test] + fn request_basic_auth_is_not_recreated_for_a_cross_origin_redirect() { + let config = FetchConfig::default(); + let original_url = url("https://app.test/start"); + let cross_origin_url = url("https://api.test/final"); + let request = Request::new("GET", original_url.as_str(), None, Vec::new()) + .unwrap() + .with_auth(RequestAuth { + target: RequestAuthTarget::Server, + scheme: RequestAuthScheme::Basic, + username: "user".to_owned(), + password: "password".to_owned(), + }); + + let initial = + outgoing_request_headers_for_url(&config, &request, &original_url, &Vec::new(), None); + assert!(header_value(&initial, "authorization").is_some()); + + let redirected = outgoing_request_headers_for_url( + &config, + &request, + &cross_origin_url, + &[redirect(&original_url, &cross_origin_url)], + None, + ); + assert_eq!(header_value(&redirected, "authorization"), None); + } + #[test] fn browser_subresource_origin_header_is_method_mode_and_origin_aware() { struct Case { From 33e7b99a47ea6bea05da8803e87f154c3dfa1c3e Mon Sep 17 00:00:00 2001 From: ldm0 Date: Mon, 24 Aug 2026 21:19:45 +0800 Subject: [PATCH 4/4] test(fetch): cover GET bodies and redirect credentials --- moli-fetch/src/tests/mod.rs | 204 ++++++++++++++++++ moli-fetch/src/tests/support.rs | 46 +++- skills/moli-webfetch/SKILL.md | 4 +- .../moli-webfetch/references/fetch-recipes.md | 4 +- 4 files changed, 253 insertions(+), 5 deletions(-) diff --git a/moli-fetch/src/tests/mod.rs b/moli-fetch/src/tests/mod.rs index 35bc0a6f3..c337914c7 100644 --- a/moli-fetch/src/tests/mod.rs +++ b/moli-fetch/src/tests/mod.rs @@ -1440,6 +1440,210 @@ async fn fetch_raw_stream_finishes_null_body_status_without_connection_close() - Ok(()) } +#[tokio::test] +async fn fetch_get_sends_its_utf8_body_on_the_wire() -> Result<()> { + let server = ScriptedHttpServer::spawn(vec![ScriptedResponse::ok("get-body-ok")]); + let client = FetchClient::new(&FetchConfig::default(), new_shared_browser_cookie_store()); + let body = "payload=hello-世界"; + let response = client + .fetch(Request::new( + "GET", + &server.url_path("/get-body"), + Some(body.to_owned()), + vec![("X-Request-Marker".to_owned(), "get-body".to_owned())], + )?) + .await?; + + assert_eq!(response.body_text(), "get-body-ok"); + let requests = server.requests(); + assert_eq!(requests.len(), 1); + let request = &requests[0]; + assert!(request.starts_with("GET /get-body HTTP/1.1"), "{request}"); + assert!( + request.contains(&format!("Content-Length: {}", body.len())), + "{request}", + ); + assert!(request.contains("X-Request-Marker: get-body"), "{request}"); + assert_eq!( + request + .split_once("\r\n\r\n") + .map(|(_, request_body)| request_body), + Some(body), + ); + assert!( + !request.to_ascii_lowercase().contains("content-type:"), + "a generic GET body must not acquire libcurl's form content type: {request}", + ); + + server.shutdown(); + Ok(()) +} + +#[tokio::test] +async fn fetch_get_preserves_an_explicit_empty_body() -> Result<()> { + let server = ScriptedHttpServer::spawn(vec![ScriptedResponse::ok("empty-get-body-ok")]); + let client = FetchClient::new(&FetchConfig::default(), new_shared_browser_cookie_store()); + client + .fetch(Request::new( + "GET", + &server.url_path("/empty-get-body"), + Some(String::new()), + Vec::new(), + )?) + .await?; + + let requests = server.requests(); + assert_eq!(requests.len(), 1); + let request = &requests[0]; + assert!( + request.starts_with("GET /empty-get-body HTTP/1.1"), + "{request}" + ); + assert!(request.contains("Content-Length: 0"), "{request}"); + assert_eq!( + request + .split_once("\r\n\r\n") + .map(|(_, request_body)| request_body), + Some(""), + ); + + server.shutdown(); + Ok(()) +} + +#[tokio::test] +async fn fetch_head_rejects_a_body_instead_of_silently_dropping_it() -> Result<()> { + let server = ScriptedHttpServer::spawn(vec![ScriptedResponse::ok("unexpected")]); + let client = FetchClient::new(&FetchConfig::default(), new_shared_browser_cookie_store()); + let error = client + .fetch(Request::new( + "HEAD", + &server.url_path("/head-body"), + Some("payload".to_owned()), + Vec::new(), + )?) + .await + .expect_err("HEAD with a body must fail before transfer"); + + assert!( + format!("{error:#}").contains("HEAD request bodies are not supported"), + "{error:#}", + ); + assert_eq!(server.hits(), 0); + + server.shutdown(); + Ok(()) +} + +#[tokio::test] +async fn fetch_307_and_308_preserve_get_method_body_and_body_headers() -> Result<()> { + for (status, reason) in [(307, "Temporary Redirect"), (308, "Permanent Redirect")] { + let server = ScriptedHttpServer::spawn(vec![ + ScriptedResponse::status(status, reason).with_header("Location", "/final"), + ScriptedResponse::ok("final-body"), + ]); + let client = FetchClient::new(&FetchConfig::default(), new_shared_browser_cookie_store()); + let body = format!("get-payload-{status}"); + let response = client + .fetch(Request::new( + "GET", + &server.url_path("/redirect"), + Some(body.clone()), + vec![("Content-Type".to_owned(), "text/plain".to_owned())], + )?) + .await?; + + assert_eq!(response.body_text(), "final-body"); + let requests = server.requests(); + assert_eq!(requests.len(), 2); + for (index, request) in requests.iter().enumerate() { + let expected_path = if index == 0 { "/redirect" } else { "/final" }; + assert!( + request.starts_with(&format!("GET {expected_path} HTTP/1.1")), + "status={status} request={request}", + ); + assert!( + request + .to_ascii_lowercase() + .contains("content-type: text/plain"), + "status={status} request={request}", + ); + assert_eq!( + request + .split_once("\r\n\r\n") + .map(|(_, request_body)| request_body), + Some(body.as_str()), + "status={status} request={request}", + ); + } + + server.shutdown(); + } + Ok(()) +} + +#[tokio::test] +async fn cross_origin_redirect_strips_manual_credentials_but_preserves_get_body() -> Result<()> { + let destination = ScriptedHttpServer::spawn(vec![ScriptedResponse::ok("destination-body")]); + let origin = ScriptedHttpServer::spawn(vec![ + ScriptedResponse::status(307, "Temporary Redirect") + .with_header("Location", &destination.url_path("/final")), + ]); + let client = FetchClient::new(&FetchConfig::default(), new_shared_browser_cookie_store()); + let response = client + .fetch(Request::new( + "GET", + &origin.url_path("/start"), + Some("cross-origin-payload".to_owned()), + vec![ + ("Authorization".to_owned(), "Bearer secret".to_owned()), + ("Cookie".to_owned(), "manual=secret".to_owned()), + ("Content-Type".to_owned(), "text/plain".to_owned()), + ("X-Trace".to_owned(), "preserved".to_owned()), + ], + )?) + .await?; + + assert_eq!(response.body_text(), "destination-body"); + let origin_requests = origin.requests(); + let destination_requests = destination.requests(); + assert_eq!(origin_requests.len(), 1); + assert_eq!(destination_requests.len(), 1); + let initial = origin_requests[0].to_ascii_lowercase(); + assert!( + initial.contains("authorization: bearer secret"), + "{initial}" + ); + assert!(initial.contains("cookie: manual=secret"), "{initial}"); + + let redirected = &destination_requests[0]; + let redirected_lower = redirected.to_ascii_lowercase(); + assert!( + redirected.starts_with("GET /final HTTP/1.1"), + "{redirected}" + ); + assert!(!redirected_lower.contains("authorization:"), "{redirected}"); + assert!(!redirected_lower.contains("cookie:"), "{redirected}"); + assert!( + redirected_lower.contains("x-trace: preserved"), + "{redirected}" + ); + assert!( + redirected_lower.contains("content-type: text/plain"), + "{redirected}", + ); + assert_eq!( + redirected + .split_once("\r\n\r\n") + .map(|(_, request_body)| request_body), + Some("cross-origin-payload"), + ); + + origin.shutdown(); + destination.shutdown(); + Ok(()) +} + #[tokio::test] async fn fetch_redirect_303_rewrites_post_to_get_and_drops_body_headers() -> Result<()> { let server = ScriptedHttpServer::spawn(vec![ diff --git a/moli-fetch/src/tests/support.rs b/moli-fetch/src/tests/support.rs index 5d3b8f06b..96c918f9b 100644 --- a/moli-fetch/src/tests/support.rs +++ b/moli-fetch/src/tests/support.rs @@ -887,9 +887,8 @@ fn handle_scripted_connection( requests: Arc>>, responses: Arc>>, ) { - let mut request = [0; 1024]; - let bytes_read = stream.read(&mut request).unwrap_or(0); - let request_text = String::from_utf8_lossy(&request[..bytes_read]).into_owned(); + let request = read_scripted_http_request(&mut stream); + let request_text = String::from_utf8_lossy(&request).into_owned(); requests.lock().push(request_text); let _ = hits.fetch_add(1, Ordering::SeqCst) + 1; let response_spec = { @@ -910,6 +909,47 @@ fn handle_scripted_connection( } } +fn read_scripted_http_request(stream: &mut std::net::TcpStream) -> Vec { + let _ = stream.set_read_timeout(Some(Duration::from_secs(1))); + let mut request = Vec::new(); + let mut expected_len = None; + let mut buffer = [0; 1024]; + + loop { + let bytes_read = match stream.read(&mut buffer) { + Ok(0) | Err(_) => break, + Ok(bytes_read) => bytes_read, + }; + request.extend_from_slice(&buffer[..bytes_read]); + + if expected_len.is_none() + && let Some(head_end) = request + .windows(4) + .position(|window| window == b"\r\n\r\n") + .map(|offset| offset + 4) + { + let content_length = std::str::from_utf8(&request[..head_end]) + .ok() + .and_then(|head| { + head.lines().find_map(|line| { + let (name, value) = line.split_once(':')?; + name.eq_ignore_ascii_case("content-length") + .then(|| value.trim().parse::().ok()) + .flatten() + }) + }) + .unwrap_or(0); + expected_len = Some(head_end.saturating_add(content_length)); + } + + if expected_len.is_some_and(|expected_len| request.len() >= expected_len) { + break; + } + } + + request +} + fn scripted_http_response_bytes(response_spec: &ScriptedResponse) -> Vec { let body = response_spec.body.clone(); let mut extra_headers = String::new(); diff --git a/skills/moli-webfetch/SKILL.md b/skills/moli-webfetch/SKILL.md index 72b05134a..d491c0592 100644 --- a/skills/moli-webfetch/SKILL.md +++ b/skills/moli-webfetch/SKILL.md @@ -124,7 +124,9 @@ unrelated actions. to use. Never expose headers, cookies, or tokens in the response. - Remember that `-H/--header` applies to the initial navigation, not every subresource. `-X/--method` and `--body` likewise configure only the initial - navigation. + navigation. GET bodies are sent as provided; HEAD bodies are rejected rather + than silently discarded. Redirects do not forward explicit `Authorization` + or `Cookie` headers away from the initial request origin. - Treat stdout as the requested artifact. Redirect screenshot, full-document screenshot, and PDF output to files, verify that they are non-empty and have the expected type, and never print their binary bytes into a text response. diff --git a/skills/moli-webfetch/references/fetch-recipes.md b/skills/moli-webfetch/references/fetch-recipes.md index d9e327112..92e229662 100644 --- a/skills/moli-webfetch/references/fetch-recipes.md +++ b/skills/moli-webfetch/references/fetch-recipes.md @@ -150,7 +150,9 @@ For a crawl rather than a single lookup: - Add initial navigation headers with repeated `-H 'Name: Value'`. - Set the initial navigation method with `-X/--method METHOD`; add a UTF-8 body with `--body TEXT`. Normal HTTP redirect rules apply, including method/body - preservation across 307 and 308 responses. + preservation across 307 and 308 responses. GET bodies are supported; HEAD + bodies are rejected. Explicit `Authorization` and `Cookie` headers remain on + the initial origin when redirects cross an origin boundary. - Import cookie files with repeated `--cookie-file`. - Use `--profile-dir` when state must persist across invocations; it also provides the default HTTP cache location unless `--http-cache-dir` is set.