From 92532b9d7b76d07123409ffca374fd151f50a33d Mon Sep 17 00:00:00 2001 From: ldm0 Date: Mon, 14 Sep 2026 02:18:06 +0800 Subject: [PATCH] fix(xhr): validate and combine author request headers Normalize header values after WebIDL conversion and state checks, reject invalid names or values with SyntaxError, and ignore forbidden request headers. Combine repeated values with comma-space while retaining the first name and empty list members. Keep quoted strings intact in the shared method-override filter so quoted commas do not turn permitted values into forbidden methods. Cover error ordering, reentrant argument conversion, and actual outgoing headers. Validation: cargo fmt --all, workspace all-targets/all-features Clippy with -D warnings, and cargo nextest run --no-fail-fast pass (17973 passed, 13 skipped). The 37-case WPT check improves from 22 to 27 passing cases and 300 to 388 passing assertions, without regressions. All 20 supplemental Window/Worker checks pass across synchronous and asynchronous XHR. Source: 2a2aed4c58fa4d72c474aeec8d7366844fe0d776 --- moli-fetch/src/headers.rs | 70 ++++++++++++++++--- .../src/network_host/xhr/header_surface.rs | 28 ++++++-- .../src/runtime/page_vm/tests/fetch_xhr.rs | 19 ++++- .../src/script_vm/tests/dom_xhr/xhr.rs | 69 ++++++++++++++++++ 4 files changed, 171 insertions(+), 15 deletions(-) diff --git a/moli-fetch/src/headers.rs b/moli-fetch/src/headers.rs index 39f5f91a66..92e1f41aa8 100644 --- a/moli-fetch/src/headers.rs +++ b/moli-fetch/src/headers.rs @@ -42,15 +42,34 @@ pub fn is_forbidden_request_header_override_value(name: &str, value: &str) -> bo ) { return false; } - value.split(',').any(|method| { - matches!( - method - .trim_matches(is_http_whitespace) - .to_ascii_uppercase() - .as_str(), - "CONNECT" | "TRACE" | "TRACK" - ) - }) + // Fetch's "get, decode, and split" keeps quoted strings intact, including + // their quotes, so commas and method names inside them are ordinary data. + let mut quoted = false; + let mut escaped = false; + value + .split(|character| { + if escaped { + escaped = false; + false + } else if quoted && character == '\\' { + escaped = true; + false + } else if character == '"' { + quoted = !quoted; + false + } else { + character == ',' && !quoted + } + }) + .any(|method| { + matches!( + method + .trim_matches(is_http_whitespace) + .to_ascii_uppercase() + .as_str(), + "CONNECT" | "TRACE" | "TRACK" + ) + }) } pub fn is_no_cors_safelisted_request_header(name: &str, value: &str) -> bool { @@ -203,6 +222,39 @@ mod tests { )); } + #[test] + fn method_override_filter_respects_quoted_list_members() { + for name in [ + "X-HTTP-Method", + "X-HTTP-Method-Override", + "X-Method-Override", + ] { + for value in [ + r#""GET,TRACE,POST""#, + r#""GET\",TRACK,POST""#, + r#"prefix"one,CONNECT,two"suffix"#, + r#""unterminated,TRACE"#, + r#""TRACE""#, + ] { + assert!( + !is_forbidden_request_header_override_value(name, value), + "{name}: {value}" + ); + } + for value in [ + r#""GET,TRACE", TRACK"#, + r#""GET\",TRACE", CONNECT"#, + r#""GET\\", CONNECT"#, + r#"prefix"CONNECT", TRACE"#, + ] { + assert!( + is_forbidden_request_header_override_value(name, value), + "{name}: {value}" + ); + } + } + } + #[test] fn no_cors_safelist_keeps_fetch_header_subset() { assert!(is_no_cors_safelisted_request_header("accept", "text/html")); diff --git a/moli-renderer-v8/src/network_host/xhr/header_surface.rs b/moli-renderer-v8/src/network_host/xhr/header_surface.rs index 4aed82df56..e9e3b64402 100644 --- a/moli-renderer-v8/src/network_host/xhr/header_surface.rs +++ b/moli-renderer-v8/src/network_host/xhr/header_surface.rs @@ -44,17 +44,37 @@ pub(super) fn xhr_set_request_header_callback<'s>( return; } + let value = parsed.value.trim_matches(['\t', '\n', '\r', ' ']); + if HeaderName::from_bytes(parsed.name.as_bytes()).is_err() + || value + .bytes() + .any(|byte| matches!(byte, b'\0' | b'\n' | b'\r')) + { + throw_dom_exception( + scope, + "SyntaxError", + 12, + "Invalid XMLHttpRequest request header.", + ); + return; + } + if moli_fetch::is_forbidden_request_header_name(&parsed.name) + || moli_fetch::is_forbidden_request_header_override_value(&parsed.name, value) + { + return; + } + let existing_json = xhr_state_string_property(scope, xhr, XHR_REQUEST_HEADERS_SLOT) .unwrap_or_else(|| "[]".to_owned()); let mut pairs: Vec<[String; 2]> = serde_json::from_str(&existing_json).unwrap_or_default(); - let lower_name = parsed.name.to_ascii_lowercase(); if let Some(pair) = pairs .iter_mut() - .find(|pair| pair[0].to_ascii_lowercase() == lower_name) + .find(|pair| pair[0].eq_ignore_ascii_case(&parsed.name)) { - pair[1] = parsed.value; + pair[1].push_str(", "); + pair[1].push_str(value); } else { - pairs.push([parsed.name, parsed.value]); + pairs.push([parsed.name, value.to_owned()]); } let new_json = serde_json::to_string(&pairs).unwrap_or_else(|_| "[]".to_owned()); set_xhr_state_string(scope, xhr, XHR_REQUEST_HEADERS_SLOT, &new_json); diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/fetch_xhr.rs b/moli-renderer-v8/src/runtime/page_vm/tests/fetch_xhr.rs index 28ae6f987b..bdb93b01df 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/fetch_xhr.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/fetch_xhr.rs @@ -2444,7 +2444,17 @@ async fn xhr_emits_browser_style_subresource_headers_on_wire() { globalThis.__xhrDone = false; const xhr = new XMLHttpRequest(); xhr.open("GET", {xhr_url_literal}); - xhr.setRequestHeader("X-Test", "xhr"); + xhr.setRequestHeader("X-Test", " \txhr \r\n"); + xhr.setRequestHeader("x-test", "\nsecond\t"); + xhr.setRequestHeader("X-Empty", ""); + xhr.setRequestHeader("x-empty", "value"); + xhr.setRequestHeader("X-HTTP-Method", "GET"); + xhr.setRequestHeader("x-http-method", "TRACE"); + xhr.setRequestHeader("x-http-method", '"GET,TRACE,POST"'); + xhr.setRequestHeader("Sec-Fetch-Mode", "no-cors"); + xhr.setRequestHeader("Cookie", "forbidden=1"); + xhr.setRequestHeader("Proxy-Authorization", "blocked"); + xhr.setRequestHeader("Referer", "https://injected.test/"); xhr.onload = () => {{ globalThis.__xhrDone = true; }}; @@ -2467,7 +2477,12 @@ async fn xhr_emits_browser_style_subresource_headers_on_wire() { let request_lower = request.to_ascii_lowercase(); assert!(request.starts_with("GET /xhr HTTP/1.1\r\n")); - assert!(request_lower.contains("x-test: xhr\r\n")); + assert!(request.contains("\r\nX-Test: xhr, second\r\n")); + assert!(request.contains("\r\nX-Empty: , value\r\n")); + assert!(request.contains("\r\nX-HTTP-Method: GET, \"GET,TRACE,POST\"\r\n")); + assert!(!request_lower.contains("forbidden=1")); + assert!(!request_lower.contains("proxy-authorization:")); + assert!(!request_lower.contains("injected.test")); assert!(request_lower.contains("referer: ")); assert!(request_lower.contains("/page.html\r\n")); assert!(request_lower.contains("accept: */*\r\n")); diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/xhr.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/xhr.rs index b7f18404a1..eb933b8944 100644 --- a/moli-renderer-v8/src/script_vm/tests/dom_xhr/xhr.rs +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/xhr.rs @@ -709,6 +709,75 @@ fn xml_http_request_methods_apply_webidl_argument_conversion() { "undefined|throw:TypeError|throw:TypeError|throw:TypeError|undefined|throw:TypeError|throw:TypeError|throw:TypeError|throw:TypeError|throw:TypeError|throw:RangeError|undefined" ); } +#[test] +fn xml_http_request_set_request_header_preserves_validation_order() { + let mut vm = new_storage_test_vm("https://xhr-header-validation.test/"); + let result = vm + .eval( + r#" +(() => { + const probe = callback => { + try { + return callback() === undefined ? "undefined" : "unexpected return value"; + } catch (error) { + return error instanceof DOMException + ? "DOM:" + error.name + ":" + error.code : error.name; + } + }; + const unopened = new XMLHttpRequest(); + const xhr = new XMLHttpRequest(); + xhr.open("GET", "/headers"); + const conversions = []; + const entered = new XMLHttpRequest(); + return JSON.stringify({ + unopenedBadSyntax: probe(() => unopened.setRequestHeader("bad:name", "bad\nvalue")), + unopenedNonByteString: probe(() => unopened.setRequestHeader("X-Test", "\u0100")), + forbiddenUnopened: probe(() => unopened.setRequestHeader("Host", "example.test")), + invalidNames: ["", "x y", "x:y", "\u00ff", "\u007f"].map( + name => probe(() => xhr.setRequestHeader(name, "ok"))), + invalidValues: ["x\0x", "x\rx", "x\nx"].map( + value => probe(() => xhr.setRequestHeader("X-Test", value))), + forbiddenBadValue: probe(() => xhr.setRequestHeader("Host", "x\nx")), + forbiddenValidValue: probe(() => xhr.setRequestHeader("Host", "example.test")), + emptyValue: probe(() => xhr.setRequestHeader("X-Test", "")), + normalizedLineEnds: probe(() => xhr.setRequestHeader("X-Test", "\r\n \tvalue\r\n ")), + otherBytes: probe(() => xhr.setRequestHeader("X-Test", "\u000b\u000c\u0085\u00a0")), + reentrantConversion: probe(() => entered.setRequestHeader({ + toString() { conversions.push("name"); return "X-Test"; } + }, { + toString() { + conversions.push("value"); + entered.open("GET", "/headers"); + return " value "; + } + })), + conversions + }); +})() +"#, + ) + .expect("XHR request header validation probe should run"); + let observed: serde_json::Value = + serde_json::from_str(&result).expect("parse XHR header validation result"); + assert_eq!( + observed, + serde_json::json!({ + "unopenedBadSyntax": "DOM:InvalidStateError:11", + "unopenedNonByteString": "TypeError", + "forbiddenUnopened": "DOM:InvalidStateError:11", + "invalidNames": vec!["DOM:SyntaxError:12"; 5], + "invalidValues": vec!["DOM:SyntaxError:12"; 3], + "forbiddenBadValue": "DOM:SyntaxError:12", + "forbiddenValidValue": "undefined", + "emptyValue": "undefined", + "normalizedLineEnds": "undefined", + "otherBytes": "undefined", + "reentrantConversion": "undefined", + "conversions": ["name", "value"], + }) + ); +} + #[test] fn xml_http_request_override_mime_type_affects_response_mime() { let mut vm = new_storage_test_vm("https://xhr-override-mime.test/");