From 2a2aed4c58fa4d72c474aeec8d7366844fe0d776 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Fri, 11 Sep 2026 05:37:47 +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. --- 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 aca2606e66..58037c4465 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 @@ -2542,7 +2542,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; }}; @@ -2565,7 +2575,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 9cca895a91..6b4f7c739e 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 @@ -829,6 +829,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/");