From 89c3475a2d52ce1cff1ecb3dbe846e00404ef3ce Mon Sep 17 00:00:00 2001 From: ldm0 Date: Sun, 13 Sep 2026 02:46:44 +0800 Subject: [PATCH] fix(fetch): preserve opaque response metadata through service workers Keep internal response status, status text and headers through Window and Worker fetch, Response.clone(), Cache storage and FetchEvent.respondWith(). This preserves CORP checks when a require-corp client receives an opaque response from a service worker. Keep public opaque headers empty and immutable, and use the public header view when matching Vary in Cache. Cover streamed and materialized responses, opaque redirects, filtered HTTP 206 responses and Vary:* cache roundtrips. Reject invalid filtered respondWith responses before following their restored internal Location headers. --- .../window_runtime/navigator.rs | 22 ++- moli-renderer-v8/src/network_host.rs | 7 +- moli-renderer-v8/src/network_host/response.rs | 8 +- .../src/network_host/response/materialize.rs | 79 ++++++--- .../src/script_vm/subresource_fetch.rs | 46 +++--- .../src/script_vm/tests/dom_xhr/mod.rs | 1 + .../tests/dom_xhr/opaque_response.rs | 153 ++++++++++++++++++ .../src/script_vm/tests/webidl_fetch.rs | 45 ++++-- .../service/fetch_settlement.rs | 18 ++- .../src/worker/global_scope/fetch.rs | 26 +-- .../src/worker/thread/dispatch.rs | 7 +- .../src/worker/thread/tests/lifecycle.rs | 31 +++- moli-storage-service/src/buckets.rs | 2 + 13 files changed, 345 insertions(+), 100 deletions(-) create mode 100644 moli-renderer-v8/src/script_vm/tests/dom_xhr/opaque_response.rs diff --git a/moli-renderer-v8/src/context_bootstrap/window_runtime/navigator.rs b/moli-renderer-v8/src/context_bootstrap/window_runtime/navigator.rs index b2f6d5627a..e34e35c8cd 100644 --- a/moli-renderer-v8/src/context_bootstrap/window_runtime/navigator.rs +++ b/moli-renderer-v8/src/context_bootstrap/window_runtime/navigator.rs @@ -2784,14 +2784,17 @@ fn storage_bucket_cached_response_from_value<'s>( value: v8::Local<'s, v8::Value>, resolver: v8::Local<'s, v8::PromiseResolver>, ) -> Option> { - let (head, response) = - match crate::network_host::materialize_response_object_head(scope, value, "Cache.put") { - Ok(result) => result, - Err(error) => { - reject_type_error(scope, resolver, &error); - return None; - } - }; + let (head, response) = match crate::network_host::materialize_response_object_internal_head( + scope, + value, + "Cache.put", + ) { + Ok(result) => result, + Err(error) => { + reject_type_error(scope, resolver, &error); + return None; + } + }; let body = match crate::network_host::materialize_response_object_body(scope, response, "Cache.put") { crate::network_host::MaterializedResponseBody::Ready(body) => body, @@ -2933,6 +2936,9 @@ fn build_storage_bucket_cached_response_object<'s>( scope, &response.response_type, &response.url, + response.status, + &response.status_text, + &response.headers, response.body, ); } diff --git a/moli-renderer-v8/src/network_host.rs b/moli-renderer-v8/src/network_host.rs index c7d79228aa..5e09dc6d0a 100644 --- a/moli-renderer-v8/src/network_host.rs +++ b/moli-renderer-v8/src/network_host.rs @@ -143,8 +143,6 @@ pub(in crate::network_host) use self::request_scope::{ observe_subresource_request_cookie_report_for_origin, subresource_request_origin_for_owner, subresource_request_scope_for_owner, }; -#[cfg(test)] -pub(crate) use self::response::materialize_response_object; pub(crate) use self::response::{ FetchResponseRequest, FetchResponseSecurityViolation, @@ -167,8 +165,6 @@ pub(crate) use self::response::{ is_cors_policy_failure_message, materialize_response_object_body, materialize_response_object_body_with_chunk_callback, - materialize_response_object_head, - materialize_response_object_head_for_service_worker_respond_with, materialized_body_bytes_from_value, network_response_filter, response_constructor_callback, @@ -177,6 +173,7 @@ pub(crate) use self::response::{ validate_cors_response_chain, validate_cors_response_chain_for_origin, response_has_null_body, + materialize_response_object_internal_head, validate_cors_response_for_origin, validate_cross_origin_embedder_and_document_isolation_policy, validate_cross_origin_resource_policy, @@ -189,6 +186,8 @@ pub(crate) use self::response::{ validate_opaque_response_blocking_with_body, validated_opaque_response_body, }; +#[cfg(test)] +pub(crate) use self::response::{materialize_response_object, materialize_response_object_head}; pub(crate) use self::stylesheet_subresource::{ StylesheetSubresourceFetchStart, start_stylesheet_subresource_fetch, }; diff --git a/moli-renderer-v8/src/network_host/response.rs b/moli-renderer-v8/src/network_host/response.rs index 05f62e7e35..489d8890d0 100644 --- a/moli-renderer-v8/src/network_host/response.rs +++ b/moli-renderer-v8/src/network_host/response.rs @@ -43,8 +43,6 @@ pub(crate) use self::cors::{ validate_opaque_response_blocking_with_body, validated_opaque_response_body, }; -#[cfg(test)] -pub(crate) use self::materialize::materialize_response_object; pub(crate) use self::materialize::{ FetchResponseRequest, MaterializedResponseBody, MaterializedResponseHead, build_fetch_response_object_for_request_mode, @@ -56,7 +54,7 @@ pub(crate) use self::materialize::{ build_filtered_cached_response_object, build_navigation_preload_response_object_from_stream_for_request_mode, materialize_response_object_body, materialize_response_object_body_with_chunk_callback, - materialize_response_object_head, - materialize_response_object_head_for_service_worker_respond_with, - materialized_body_bytes_from_value, network_response_filter, + materialize_response_object_internal_head, materialized_body_bytes_from_value, network_response_filter, }; +#[cfg(test)] +pub(crate) use self::materialize::{materialize_response_object, materialize_response_object_head}; diff --git a/moli-renderer-v8/src/network_host/response/materialize.rs b/moli-renderer-v8/src/network_host/response/materialize.rs index ef10aa2a33..5dd811116d 100644 --- a/moli-renderer-v8/src/network_host/response/materialize.rs +++ b/moli-renderer-v8/src/network_host/response/materialize.rs @@ -16,6 +16,29 @@ pub(crate) struct FetchResponseRequest<'a> { pub(crate) redirect_mode: RequestRedirectMode, } +impl FetchResponseRequest<'_> { + pub(crate) fn filter_response_headers( + self, + request_origin: &moli_url::WebOrigin, + head: &moli_fetch::ResponseHead, + credentials_mode: moli_fetch::RequestCredentialsMode, + ) -> Vec<(String, String)> { + // Opaque responses need their internal headers for Cache and respondWith. + // Their public header list is made empty when the Response is built. + if self.mode == RequestMode::NoCors + || self.redirect_mode == RequestRedirectMode::Manual && is_redirect_status(head.status) + { + head.headers.clone() + } else { + filter_cors_exposed_response_headers_for_origin( + request_origin, + head, + credentials_mode, + ) + } + } +} + fn is_redirect_status(status: u16) -> bool { matches!(status, 301 | 302 | 303 | 307 | 308) } @@ -367,34 +390,44 @@ fn build_fetch_response_object_head<'s>( FetchResponseInternalUrlDeclaration::new(head.final_url.to_string()) .initialize(scope, obj) .expect("Fetch Response internal URL declaration should initialize"); - if !filter.is_readable() { - set_response_slot_value( + if filter != FetchResponseFilter::None { + set_filtered_response_internal_head( scope, obj, - RESPONSE_INTERNAL_STATUS_SLOT, - v8::Number::new(scope, head.status as f64).into(), - ); - set_response_slot_string( - scope, - obj, - RESPONSE_INTERNAL_STATUS_TEXT_SLOT, + head.status, head.status_text(), - ); - let internal_headers = filter_headers_for_guard(&head.headers, HeadersGuard::Response); - let internal_headers_obj = - build_headers_object_with_state(scope, &internal_headers, HeadersGuard::Response, true); - install_headers_object_methods(scope, internal_headers_obj); - set_response_slot_value( - scope, - obj, - RESPONSE_INTERNAL_HEADERS_SLOT, - internal_headers_obj.into(), + &head.headers, ); } mark_response_object(scope, obj); obj } +fn set_filtered_response_internal_head( + scope: &mut v8::PinScope<'_, '_>, + obj: v8::Local<'_, v8::Object>, + status: u16, + status_text: &str, + headers: &[(String, String)], +) { + set_response_slot_value( + scope, + obj, + RESPONSE_INTERNAL_STATUS_SLOT, + v8::Number::new(scope, status as f64).into(), + ); + set_response_slot_string(scope, obj, RESPONSE_INTERNAL_STATUS_TEXT_SLOT, status_text); + let internal_headers = + build_headers_object_with_state(scope, headers, HeadersGuard::None, true); + install_headers_object_methods(scope, internal_headers); + set_response_slot_value( + scope, + obj, + RESPONSE_INTERNAL_HEADERS_SLOT, + internal_headers.into(), + ); +} + fn finish_fetch_response_object_with_body_stream<'s>( scope: &mut v8::PinScope<'s, '_>, obj: v8::Local<'s, v8::Object>, @@ -439,6 +472,9 @@ pub(crate) fn build_filtered_cached_response_object<'s>( scope: &mut v8::PinScope<'s, '_>, response_type: &str, internal_url: &str, + status: u16, + status_text: &str, + internal_headers: &[(String, String)], body: Vec, ) -> Option> { let response_type = match response_type { @@ -459,6 +495,7 @@ pub(crate) fn build_filtered_cached_response_object<'s>( .initialize(scope, obj) .ok()?; } + set_filtered_response_internal_head(scope, obj, status, status_text, internal_headers); mark_response_object(scope, obj); let headers = filter_headers_for_guard(&[], HeadersGuard::Response); @@ -609,13 +646,13 @@ pub(crate) fn materialize_response_object_head<'s>( )) } -pub(crate) fn materialize_response_object_head_for_service_worker_respond_with<'s>( +pub(crate) fn materialize_response_object_internal_head<'s>( scope: &mut v8::PinScope<'s, '_>, value: v8::Local<'s, v8::Value>, context: &str, ) -> Result<(MaterializedResponseHead, v8::Local<'s, v8::Object>), String> { let (mut head, response) = materialize_response_object_head(scope, value, context)?; - if head.response_type == "opaqueredirect" + if matches!(head.response_type.as_str(), "opaque" | "opaqueredirect") && let Some(internal_status) = response_slot_number(scope, response, RESPONSE_INTERNAL_STATUS_SLOT) { diff --git a/moli-renderer-v8/src/script_vm/subresource_fetch.rs b/moli-renderer-v8/src/script_vm/subresource_fetch.rs index df79414674..c8f31fa5f3 100644 --- a/moli-renderer-v8/src/script_vm/subresource_fetch.rs +++ b/moli-renderer-v8/src/script_vm/subresource_fetch.rs @@ -3722,10 +3722,7 @@ impl ScriptVm { record_started, ); let mut observable_response = response; - if matches!( - pending.info.resource_type, - SubresourceResourceType::Fetch | SubresourceResourceType::Xhr - ) && !response_filter.is_some_and(|filter| filter.is_readable()) { + if pending.info.resource_type == SubresourceResourceType::Xhr && !response_filter.is_some_and(|filter| filter.is_readable()) { observable_response.headers = crate::network_host::filter_cors_exposed_response_headers( &pending.request_origin, @@ -3743,6 +3740,16 @@ impl ScriptVm { .expect("detached keepalive completion is handled before V8 entry"); let resolver = v8::Local::new(scope, &resolver); let (mut head, body) = observable_response.into_body(); + let response_request = crate::network_host::FetchResponseRequest { + method: &response_request_method, + mode: pending.request_mode, + redirect_mode, + }; + head.headers = response_request.filter_response_headers( + &pending.request_origin(), + &head, + pending.credentials_mode, + ); if let Some(status_text) = response_status_text { head.status_text = Some(status_text); } @@ -3758,11 +3765,7 @@ impl ScriptVm { crate::network_host::build_fetch_response_object_from_body_source_for_request_mode_with_filter( scope, &pending.request_origin, - crate::network_host::FetchResponseRequest { - method: &response_request_method, - mode: pending.request_mode, - redirect_mode, - }, + response_request, head, body, response_filter, @@ -4879,12 +4882,9 @@ impl ScriptVm { } let mut observable_head = started.head.clone(); - if matches!( - pending.info.resource_type, - SubresourceResourceType::Fetch | SubresourceResourceType::Xhr - ) && !started.response_filter.is_some_and(|filter| filter.is_readable()) { - observable_head.headers = crate::network_host::filter_cors_exposed_response_headers( - &pending.request_origin, + if pending.info.resource_type == SubresourceResourceType::Xhr && !started.response_filter.is_some_and(|filter| filter.is_readable()) { + observable_head.headers = crate::network_host::filter_cors_exposed_response_headers_for_origin( + &pending.request_origin(), &observable_head, pending.credentials_mode, ); @@ -4972,14 +4972,20 @@ impl ScriptVm { .resolver() .expect("detached keepalive stream is handled before V8 entry"); let resolver = v8::Local::new(scope, resolver); + let response_request = crate::network_host::FetchResponseRequest { + method: &started.request_method, + mode: pending.request_mode, + redirect_mode: fetch.redirect_mode(), + }; + observable_head.headers = response_request.filter_response_headers( + &pending.request_origin(), + &observable_head, + pending.credentials_mode, + ); let response_obj = crate::network_host::build_fetch_response_object_from_stream_for_request_mode_with_filter( scope, &pending.request_origin, - crate::network_host::FetchResponseRequest { - method: &started.request_method, - mode: pending.request_mode, - redirect_mode: fetch.redirect_mode(), - }, + response_request, observable_head, started.body_source_id, started.response_filter, diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/mod.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/mod.rs index 537bb3484a..c461aad805 100644 --- a/moli-renderer-v8/src/script_vm/tests/dom_xhr/mod.rs +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/mod.rs @@ -11,6 +11,7 @@ mod file_input; mod forms; mod misc; mod null_body; +mod opaque_response; mod open_validation; mod query_realms; mod redirect_filter; diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/opaque_response.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/opaque_response.rs new file mode 100644 index 0000000000..a7b25657a9 --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/opaque_response.rs @@ -0,0 +1,153 @@ +use super::*; +use crate::util::v8str; + +#[test] +fn window_filtered_fetch_preserves_internal_head_through_clone_and_cache() { + for streaming in [false, true] { + for (mode, redirect, status, response_type) in [ + ("no-cors", "follow", 200, "opaque"), + ("cors", "manual", 302, "opaqueredirect"), + ] { + let mut vm = new_storage_test_vm("https://opaque-response.test/"); + vm.set_fetch_subresource_interception( + true, + Some(crate::types::SubresourceResourceType::Fetch), + ); + vm.eval(&format!(r#" + globalThis.opaqueResult = 'pending'; + fetch('https://remote-opaque-response.test/response', {{mode: '{mode}', redirect: '{redirect}'}}) + .then(async response => {{ + globalThis.original = response; + globalThis.cloned = response.clone(); + const bucket = await navigator.storageBuckets.open('opaque-response'); + const cache = await bucket.caches.open('responses'); + await cache.put('/key', cloned.clone()); + globalThis.cached = await cache.match('/key'); + await navigator.storageBuckets.delete('opaque-response'); + for (const entry of [original, cloned, cached]) {{ + if (!(entry instanceof Response) || entry.type !== '{response_type}' || + entry.status !== 0 || entry.statusText !== '' || entry.body !== null || + entry.bodyUsed || [...entry.headers].length !== 0) throw new Error('public response surface'); + let error; + try {{ entry.headers.set('x-author', 'changed'); }} catch (value) {{ error = value; }} + if (!(error instanceof TypeError)) throw new Error('immutable public headers'); + }} + opaqueResult = 'ok'; + }}).catch(error => opaqueResult = String(error.stack || error)); + "#)).unwrap(); + let requests = vm.take_pending_subresource_fetch_infos(); + assert_eq!(requests.len(), 1); + let request = &requests[0]; + let headers = vec![ + ( + "Content-Type".to_owned(), + "application/octet-stream".to_owned(), + ), + ("Content-Length".to_owned(), "0".to_owned()), + ("Access-Control-Allow-Origin".to_owned(), "*".to_owned()), + ( + "Cross-Origin-Resource-Policy".to_owned(), + "cross-origin".to_owned(), + ), + ("Vary".to_owned(), "*".to_owned()), + ("Set-Cookie".to_owned(), "hidden=secret".to_owned()), + ]; + let head = moli_fetch::ResponseHead { + final_url: request.url.clone(), + status, + status_text: Some("Internal Status".to_owned()), + headers: headers.clone(), + request_cookie_report: None, + cookie_set_reports: Vec::new(), + redirected: false, + redirect_chain: Vec::new(), + from_cache: false, + negotiated_http_version: None, + }; + if streaming { + let body_source_id = crate::network_host::new_network_body_source_id(); + vm.start_streaming_async_subresource_fetch( + crate::types::AsyncSubresourceStreamingStarted { + internal_id: request.internal_id, + request_url: request.url.clone(), + request_method: "GET".to_owned(), + request_headers: Vec::new(), + request_body: None, + body_source_id, + head, + network_request_headers: None, + }, + ) + .unwrap(); + vm.finish_streaming_async_subresource_fetch( + request.internal_id, + body_source_id, + Ok(()), + ) + .unwrap(); + } else { + vm.complete_async_subresource_fetch( + crate::types::AsyncSubresourceFetchCompletion { + internal_id: request.internal_id, + request_url: request.url.clone(), + request_method: "GET".to_owned(), + request_headers: Vec::new(), + request_body: None, + response_status_text: None, + skip_fetch_security_validation: false, + response_filter: None, + network_error_text: None, + result: Ok( + crate::protocol_types::NavigationResponse::from_head_and_body( + head, + String::new(), + Vec::new(), + ), + ), + }, + ) + .unwrap(); + } + vm.exec("0", None).unwrap(); + assert_eq!( + vm.eval("opaqueResult").unwrap(), + "ok", + "{response_type}/streaming={streaming}" + ); + let context_ptr: *const v8::Global = &vm.page_default_context; + vm.renderer_document_isolate + .with_entered_renderer_document_isolate(move |isolate| { + let scope = std::pin::pin!(v8::HandleScope::new(isolate)); + let scope = &mut scope.init(); + let context = unsafe { v8::Local::new(scope, &*context_ptr) }; + let scope = &mut v8::ContextScope::new(scope, context); + for name in ["original", "cloned", "cached"] { + let value = context + .global(scope) + .get(scope, v8str(scope, name).into()) + .unwrap(); + let (head, _) = + crate::network_host::materialize_response_object_internal_head( + scope, value, "test", + ) + .unwrap(); + assert_eq!(head.response_type, response_type, "{name}"); + assert_eq!(head.status, status, "{name}"); + assert_eq!(head.status_text, "Internal Status", "{name}"); + for (name, value) in &headers { + assert!( + head.headers + .iter() + .any(|(key, entry)| key.eq_ignore_ascii_case(name) + && entry == value), + "missing internal {name}: {:?}", + head.headers + ); + } + } + Ok(()) + }) + .unwrap(); + } + } +} diff --git a/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs b/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs index 1c04693a5a..409842dff2 100644 --- a/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs +++ b/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs @@ -9559,9 +9559,19 @@ fn materialize_response_object_preserves_redirected_slot() { } #[test] -fn filtered_response_materialization_preserves_urls_across_clone_and_cache() { +fn filtered_response_materialization_preserves_internal_head_across_clone_and_cache() { use crate::types::AsyncSubresourceFetchResponseFilter::{Opaque, OpaqueRedirect}; for (response_type, filter) in [("opaque", Opaque), ("opaqueredirect", OpaqueRedirect)] { + let internal_status = if response_type == "opaque" { 206 } else { 302 }; + let internal_headers = vec![ + ( + "cross-origin-resource-policy".to_owned(), + "cross-origin".to_owned(), + ), + ("location".to_owned(), "target.html".to_owned()), + ("set-cookie".to_owned(), "hidden=secret".to_owned()), + ("vary".to_owned(), "*".to_owned()), + ]; let mut vm = new_storage_test_vm("https://response-materialize-filtered.test/"); let document_url = Url::parse("https://response-materialize-filtered.test/") .expect("document URL should parse"); @@ -9574,6 +9584,7 @@ fn filtered_response_materialization_preserves_urls_across_clone_and_cache() { vm.renderer_document_isolate .with_entered_renderer_document_isolate({ let final_url = final_url.clone(); + let internal_headers = internal_headers.clone(); move |isolate| { let scope = std::pin::pin!(v8::HandleScope::new(isolate)); let scope = &mut scope.init(); @@ -9585,10 +9596,10 @@ fn filtered_response_materialization_preserves_urls_across_clone_and_cache() { crate::network_host::FetchResponseRequest { redirect_mode: moli_fetch::RequestRedirectMode::Follow, method: "GET", mode: moli_fetch::RequestMode::Cors }, moli_fetch::ResponseHead { - status_text: None, + status_text: Some("Internal Status".to_owned()), final_url: final_url.clone(), - status: 302, - headers: vec![("location".to_owned(), "target.html".to_owned())], + status: internal_status, + headers: internal_headers, request_cookie_report: None, cookie_set_reports: Vec::new(), redirected: false, @@ -9645,7 +9656,9 @@ redirect_mode: moli_fetch::RequestRedirectMode::Follow, method: "GET", mode: mol globalThis.__filteredResponseCached.type, globalThis.__filteredResponseCached.status, globalThis.__filteredResponseCached.url, - globalThis.__filteredResponseCached.body === null + globalThis.__filteredResponseCached.body === null, + [...globalThis.__filteredResponseCached.headers].length, + globalThis.__filteredResponseCached.statusText ].join("|"); })().catch(error => { globalThis.__filteredResponseCacheProbe = @@ -9661,7 +9674,7 @@ redirect_mode: moli_fetch::RequestRedirectMode::Follow, method: "GET", mode: mol .expect("filtered response cache roundtrip should settle"); assert_eq!( cache_probe, - format!("{response_type}|0|{expected_url}|true") + format!("{response_type}|0|{expected_url}|true|0|") ); assert_eq!( vm.eval("__filteredResponseClone.url").unwrap(), @@ -9680,19 +9693,31 @@ redirect_mode: moli_fetch::RequestRedirectMode::Follow, method: "GET", mode: mol .get(scope, v8str(scope, "__filteredResponseClone").into()) .expect("filtered response clone should exist"); let materialized_clone = - crate::network_host::materialize_response_object(scope, clone, "clone") - .expect("filtered response clone should preserve internal URL"); + crate::network_host::materialize_response_object_internal_head( + scope, clone, "clone", + ) + .expect("filtered response clone should preserve the internal head") + .0; assert_eq!(materialized_clone.final_url.as_ref(), Some(&final_url)); assert_eq!(materialized_clone.response_type, response_type); + assert_eq!(materialized_clone.status, internal_status); + assert_eq!(materialized_clone.status_text, "Internal Status"); + assert_eq!(materialized_clone.headers, internal_headers); let cached = global .get(scope, v8str(scope, "__filteredResponseCached").into()) .expect("cached filtered response should exist"); let materialized_cached = - crate::network_host::materialize_response_object(scope, cached, "cache") - .expect("cached filtered response should preserve internal URL"); + crate::network_host::materialize_response_object_internal_head( + scope, cached, "cache", + ) + .expect("cached filtered response should preserve the internal head") + .0; assert_eq!(materialized_cached.final_url.as_ref(), Some(&final_url)); assert_eq!(materialized_cached.response_type, response_type); + assert_eq!(materialized_cached.status, internal_status); + assert_eq!(materialized_cached.status_text, "Internal Status"); + assert_eq!(materialized_cached.headers, internal_headers); Ok(()) }) .expect("filtered response clone/cache should materialize"); diff --git a/moli-renderer-v8/src/service_worker_runtime/service/fetch_settlement.rs b/moli-renderer-v8/src/service_worker_runtime/service/fetch_settlement.rs index 5a4b277fea..a548223ac2 100644 --- a/moli-renderer-v8/src/service_worker_runtime/service/fetch_settlement.rs +++ b/moli-renderer-v8/src/service_worker_runtime/service/fetch_settlement.rs @@ -514,6 +514,16 @@ impl ServiceWorkerRuntimeService { response: ServiceWorkerFetchResponse, ) { job.cancel_pending_navigation_preload(); + // Validate the filtered response before inspecting its internal redirect. + // A restored Location must not turn an invalid respondWith into a fetch. + if let Some(message) = service_worker_fetch_response_rejection(&job, &response) { + self.complete_fetch_with_network_failure( + job, + message, + crate::network_host::FAILED_ERROR_TEXT.to_owned(), + ); + return; + } if is_redirect_status(response.status) && response.response_type == "opaqueredirect" && service_worker_fetch_is_navigation_request(&job) @@ -581,14 +591,6 @@ impl ServiceWorkerRuntimeService { } } } - if let Some(message) = service_worker_fetch_response_rejection(&job, &response) { - self.complete_fetch_with_network_failure( - job, - message, - crate::network_host::FAILED_ERROR_TEXT.to_owned(), - ); - return; - } let final_url = response .final_url .clone() diff --git a/moli-renderer-v8/src/worker/global_scope/fetch.rs b/moli-renderer-v8/src/worker/global_scope/fetch.rs index e80814fb62..8c649fa398 100644 --- a/moli-renderer-v8/src/worker/global_scope/fetch.rs +++ b/moli-renderer-v8/src/worker/global_scope/fetch.rs @@ -2627,7 +2627,12 @@ pub(in crate::worker) fn start_worker_streaming_fetch( pending.request_mode, ); let mut observable_head = started.head.clone(); - observable_head.headers = filter_cors_exposed_response_headers_for_origin( + observable_head.headers = crate::network_host::FetchResponseRequest { + method: &pending.request_method, + mode: pending.request_mode, + redirect_mode: pending.redirect_mode, + } + .filter_response_headers( &request_origin, &observable_head, pending.credentials_mode, @@ -3043,7 +3048,12 @@ pub(in crate::worker) fn drain_worker_fetch_completion_result( }, )); } - let filtered_headers = filter_cors_exposed_response_headers_for_origin( + let response_request = crate::network_host::FetchResponseRequest { + method: &pending.request_method, + mode: pending.request_mode, + redirect_mode: pending.redirect_mode, + }; + let filtered_headers = response_request.filter_response_headers( &request_origin, &response_head, pending.credentials_mode, @@ -3059,11 +3069,7 @@ pub(in crate::worker) fn drain_worker_fetch_completion_result( build_fetch_response_object_from_body_source_for_request_mode( scope, &pending.document_url, - crate::network_host::FetchResponseRequest { - method: &pending.request_method, - mode: pending.request_mode, - redirect_mode: pending.redirect_mode, - }, + response_request, head, body, ) @@ -3078,11 +3084,7 @@ pub(in crate::worker) fn drain_worker_fetch_completion_result( build_fetch_response_object_from_subresource_body_for_request_mode( scope, &pending.document_url, - crate::network_host::FetchResponseRequest { - method: &pending.request_method, - mode: pending.request_mode, - redirect_mode: pending.redirect_mode, - }, + response_request, head, body, ) diff --git a/moli-renderer-v8/src/worker/thread/dispatch.rs b/moli-renderer-v8/src/worker/thread/dispatch.rs index 63eee97f7e..b49a785f4d 100644 --- a/moli-renderer-v8/src/worker/thread/dispatch.rs +++ b/moli-renderer-v8/src/worker/thread/dispatch.rs @@ -29,9 +29,8 @@ use crate::network_host::{ close_pending_network_body_stream, enqueue_pending_network_body_chunk, error_pending_network_body_stream_with_reason, materialize_response_object_body, materialize_response_object_body_with_chunk_callback, - materialize_response_object_head_for_service_worker_respond_with, - materialized_body_bytes_from_value, new_network_body_source_id, - set_request_destination_for_service_worker_fetch_event, + materialize_response_object_internal_head, materialized_body_bytes_from_value, + new_network_body_source_id, set_request_destination_for_service_worker_fetch_event, set_request_mode_for_service_worker_fetch_event, set_request_reload_navigation_for_service_worker_fetch_event, }; @@ -3160,7 +3159,7 @@ fn service_worker_respond_with_settled<'s>( return; }; let result = if fulfilled { - match materialize_response_object_head_for_service_worker_respond_with( + match materialize_response_object_internal_head( scope, args.get(0), "FetchEvent.respondWith", diff --git a/moli-renderer-v8/src/worker/thread/tests/lifecycle.rs b/moli-renderer-v8/src/worker/thread/tests/lifecycle.rs index 878ded8703..877153e2c4 100644 --- a/moli-renderer-v8/src/worker/thread/tests/lifecycle.rs +++ b/moli-renderer-v8/src/worker/thread/tests/lifecycle.rs @@ -2827,7 +2827,8 @@ async fn service_worker_opaque_headers_precede_orb_validation_and_cache_preserve } #[tokio::test] -async fn service_worker_fetch_respond_with_body_accessed_opaque_response_keeps_internal_body() { +async fn service_worker_fetch_respond_with_body_accessed_opaque_response_keeps_internal_head_and_body() + { ensure_v8(); let listener = TcpListener::bind("127.0.0.1:0") .await @@ -2854,7 +2855,7 @@ async fn service_worker_fetch_respond_with_body_accessed_opaque_response_keeps_i assert!(request.contains("Sec-Fetch-Mode: no-cors\r\n")); stream .write_all( - b"HTTP/1.1 200 OK\r\nContent-Type: application/javascript\r\nContent-Length: 15\r\nConnection: close\r\n\r\ncallback('OK');", + b"HTTP/1.1 200 OK\r\nContent-Type: application/javascript\r\nCross-Origin-Resource-Policy: cross-origin\r\nVary: *\r\nSet-Cookie: hidden=secret\r\nContent-Length: 15\r\nConnection: close\r\n\r\ncallback('OK');", ) .await .expect("write service worker opaque body response"); @@ -2869,7 +2870,9 @@ async fn service_worker_fetch_respond_with_body_accessed_opaque_response_keeps_i function assertOpaqueResponse(response, label) {{ response.body; if (response.type !== "opaque" || response.status !== 0 || - response.body !== null || response.bodyUsed) {{ + response.body !== null || response.bodyUsed || + response.statusText !== "" || response.url !== "" || + [...response.headers].length !== 0) {{ throw new Error(label + ":" + [ response.type, response.status, @@ -2974,7 +2977,7 @@ async fn service_worker_fetch_respond_with_body_accessed_opaque_response_keeps_i "clone/cache mode {clone_mode}/{cache_mode}" ); assert_eq!( - response.status, 0, + response.status, 200, "clone/cache mode {clone_mode}/{cache_mode}" ); assert_eq!( @@ -2982,10 +2985,22 @@ async fn service_worker_fetch_respond_with_body_accessed_opaque_response_keeps_i Some(fetch_url.as_str()), "clone/cache mode {clone_mode}/{cache_mode}" ); - assert!( - response.headers.is_empty(), - "clone/cache mode {clone_mode}/{cache_mode}" - ); + assert_eq!(response.status_text, "OK"); + for (name, value) in [ + ("content-type", "application/javascript"), + ("cross-origin-resource-policy", "cross-origin"), + ("vary", "*"), + ("set-cookie", "hidden=secret"), + ] { + assert!( + response + .headers + .iter() + .any(|(key, entry)| key.eq_ignore_ascii_case(name) && entry == value), + "missing internal {name} for {clone_mode}/{cache_mode}: {:?}", + response.headers, + ); + } assert_eq!( response.body, b"callback('OK');".to_vec(), diff --git a/moli-storage-service/src/buckets.rs b/moli-storage-service/src/buckets.rs index 6b1019c912..d1aa97cb37 100644 --- a/moli-storage-service/src/buckets.rs +++ b/moli-storage-service/src/buckets.rs @@ -1771,6 +1771,8 @@ fn cache_entry_matches_query( return false; } query.ignore_vary + // Opaque public headers are empty even though Cache retains the internal head. + || matches!(entry.response.response_type.as_str(), "opaque" | "opaqueredirect") || cached_response_vary_matches_request( &entry.response.headers, &entry.request.headers,