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/");