From ed7e56eb421e19b266da8322dcc2dfce6aac0fa5 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Tue, 22 Sep 2026 15:42:15 +0800 Subject: [PATCH] fix(fetch): preserve URLs on opaque redirect responses --- .../src/network_host/response/materialize.rs | 18 ++- .../src/runtime/page_vm/tests/fetch_xhr.rs | 5 +- .../src/script_vm/tests/browser_api/misc.rs | 6 +- .../src/script_vm/tests/webidl_fetch.rs | 117 ++++++++++-------- .../src/worker/thread/tests/network.rs | 5 +- 5 files changed, 89 insertions(+), 62 deletions(-) diff --git a/moli-renderer-v8/src/network_host/response/materialize.rs b/moli-renderer-v8/src/network_host/response/materialize.rs index 0c13c90aa..ffd669226 100644 --- a/moli-renderer-v8/src/network_host/response/materialize.rs +++ b/moli-renderer-v8/src/network_host/response/materialize.rs @@ -133,10 +133,13 @@ fn filtered_response_status(head: &moli_fetch::ResponseHead, filter: FetchRespon fn filtered_response_url(head: &moli_fetch::ResponseHead, filter: FetchResponseFilter) -> &str { match filter { - FetchResponseFilter::Opaque | FetchResponseFilter::OpaqueRedirect => "", - FetchResponseFilter::None | FetchResponseFilter::Basic | FetchResponseFilter::Cors => { - head.final_url.as_str() - } + FetchResponseFilter::Opaque => "", + // Manual redirects preserve the URL list; only their status, headers, + // and body are filtered. No redirect target was fetched. + FetchResponseFilter::None + | FetchResponseFilter::Basic + | FetchResponseFilter::Cors + | FetchResponseFilter::OpaqueRedirect => head.final_url.as_str(), } } @@ -427,7 +430,12 @@ pub(crate) fn build_filtered_cached_response_object<'s>( "opaqueredirect" => "opaqueredirect", _ => return None, }; - let obj = FetchResponseHeadDeclaration::new(0.0, false, String::new(), false, response_type) + let visible_url = if response_type == "opaqueredirect" { + internal_url.to_owned() + } else { + String::new() + }; + let obj = FetchResponseHeadDeclaration::new(0.0, false, visible_url, false, response_type) .bind(scope) .ok()?; if !internal_url.is_empty() { 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 96ba21955..01d0e70cf 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 @@ -4573,7 +4573,7 @@ async fn window_fetch_manual_redirect_returns_opaqueredirect_filtered_response() ok: response.ok, statusText: response.statusText, redirected: response.redirected, - urlIsEmpty: response.url === "", + urlMatchesRequest: response.url === {fetch_url_literal}, bodyIsNull: response.body === null, headers: Array.from(response.headers), bodyUsedBefore, @@ -4581,6 +4581,7 @@ async fn window_fetch_manual_redirect_returns_opaqueredirect_filtered_response() text, cloneType: clone.type, cloneStatus: clone.status, + cloneUrlMatchesRequest: clone.url === {fetch_url_literal}, cloneBodyIsNull: clone.body === null, cloneText, }}); @@ -4607,7 +4608,7 @@ async fn window_fetch_manual_redirect_returns_opaqueredirect_filtered_response() server.await.expect("manual redirect fetch server should finish"); assert_eq!( observed, - r#"{"type":"opaqueredirect","status":0,"ok":false,"statusText":"","redirected":false,"urlIsEmpty":true,"bodyIsNull":true,"headers":[],"bodyUsedBefore":false,"bodyUsedAfter":true,"text":"","cloneType":"opaqueredirect","cloneStatus":0,"cloneBodyIsNull":true,"cloneText":""}"# + r#"{"type":"opaqueredirect","status":0,"ok":false,"statusText":"","redirected":false,"urlMatchesRequest":true,"bodyIsNull":true,"headers":[],"bodyUsedBefore":false,"bodyUsedAfter":true,"text":"","cloneType":"opaqueredirect","cloneStatus":0,"cloneUrlMatchesRequest":true,"cloneBodyIsNull":true,"cloneText":""}"# ); let (records, _, _) = split_network_output_items(network_output); assert_eq!(records.len(), 1); diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/misc.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/misc.rs index d8e0a53a2..f5053692e 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/misc.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/misc.rs @@ -13156,12 +13156,12 @@ async fn navigator_service_worker_fetch_follows_and_filters_synthetic_redirect_r "String(globalThis.__serviceWorkerSyntheticRedirectProbe)", &format!( "200|true|{base_url}/app/api/redirect-final.txt|synthetic-redirect-final;\ - manual=0|opaqueredirect|false||;\ + manual=0|opaqueredirect|false|{base_url}/app/api/manual-start.txt|;\ relative-follow=rejected:TypeError;\ - relative-manual=0|opaqueredirect|false||;\ + relative-manual=0|opaqueredirect|false|{base_url}/app/api/generated-relative-redirect.txt|;\ opaqueredirect-follow=rejected:TypeError;\ opaqueredirect-error=rejected:TypeError;\ - opaqueredirect-manual=0|opaqueredirect|false||" + opaqueredirect-manual=0|opaqueredirect|false|{redirect_url}|" ), ) .await; 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 823b25701..659d60c45 100644 --- a/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs +++ b/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs @@ -9740,15 +9740,19 @@ fn materialize_response_object_preserves_redirected_slot() { } #[test] -fn filtered_response_materialization_preserves_internal_url_without_exposing_url() { - 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"); - let final_url = Url::parse("https://cross-response-materialize-filtered.test/redirect-start") +fn filtered_response_materialization_preserves_urls_across_clone_and_cache() { + use crate::types::AsyncSubresourceFetchResponseFilter::{Opaque, OpaqueRedirect}; + for (response_type, filter) in [("opaque", Opaque), ("opaqueredirect", OpaqueRedirect)] { + 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"); + let final_url = Url::parse( + "https://cross-response-materialize-filtered.test/redirect-start?x=%23#hidden", + ) .expect("final URL should parse"); - let context_ptr: *const v8::Global = &vm.page_default_context as *const _; + let context_ptr: *const v8::Global = &vm.page_default_context as *const _; - vm.renderer_document_isolate + vm.renderer_document_isolate .with_entered_renderer_document_isolate({ let final_url = final_url.clone(); move |isolate| { @@ -9772,7 +9776,7 @@ fn filtered_response_materialization_preserves_internal_url_without_exposing_url negotiated_http_version: None, }, moli_fetch::ResponseBody::materialized_bytes(Vec::new()), - Some(crate::types::AsyncSubresourceFetchResponseFilter::OpaqueRedirect), + Some(filter), ); let global = context.global(scope); let _ = global.set( @@ -9788,20 +9792,25 @@ fn filtered_response_materialization_preserves_internal_url_without_exposing_url ) .expect("filtered response head should materialize with internal URL"); assert_eq!(head.final_url.as_ref(), Some(&final_url)); - assert_eq!(head.response_type, "opaqueredirect"); + assert_eq!(head.response_type, response_type); assert_eq!(head.status, 0); Ok(()) } }) .expect("filtered response should install"); - let visible_url = vm - .eval("globalThis.__filteredResponse.url") - .expect("filtered response visible URL should evaluate"); - assert_eq!(visible_url, ""); + let visible_url = vm + .eval("globalThis.__filteredResponse.url") + .expect("filtered response visible URL should evaluate"); + let expected_url = if response_type == "opaque" { + "" + } else { + "https://cross-response-materialize-filtered.test/redirect-start?x=%23" + }; + assert_eq!(visible_url, expected_url); - vm.exec( - r#" + vm.exec( + r#" globalThis.__filteredResponseClone = globalThis.__filteredResponse.clone(); globalThis.__filteredResponseCacheClone = globalThis.__filteredResponse.clone(); globalThis.__filteredResponseCacheProbe = "pending"; @@ -9814,7 +9823,7 @@ fn filtered_response_materialization_preserves_internal_url_without_exposing_url globalThis.__filteredResponseCacheProbe = [ globalThis.__filteredResponseCached.type, globalThis.__filteredResponseCached.status, - globalThis.__filteredResponseCached.url === "", + globalThis.__filteredResponseCached.url, globalThis.__filteredResponseCached.body === null ].join("|"); })().catch(error => { @@ -9822,43 +9831,51 @@ fn filtered_response_materialization_preserves_internal_url_without_exposing_url "error:" + String(error && error.name) + ":" + String(error && error.message); }); "#, - None, - ) - .expect("filtered response cache roundtrip should schedule"); + None, + ) + .expect("filtered response cache roundtrip should schedule"); - let cache_probe = vm - .eval("String(globalThis.__filteredResponseCacheProbe)") - .expect("filtered response cache roundtrip should settle"); - assert_eq!(cache_probe, "opaqueredirect|0|true|true"); + let cache_probe = vm + .eval("String(globalThis.__filteredResponseCacheProbe)") + .expect("filtered response cache roundtrip should settle"); + assert_eq!( + cache_probe, + format!("{response_type}|0|{expected_url}|true") + ); + assert_eq!( + vm.eval("__filteredResponseClone.url").unwrap(), + expected_url + ); - let context_ptr: *const v8::Global = &vm.page_default_context as *const _; - 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); - let global = context.global(scope); - let clone = global - .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"); - assert_eq!(materialized_clone.final_url.as_ref(), Some(&final_url)); - assert_eq!(materialized_clone.response_type, "opaqueredirect"); + let context_ptr: *const v8::Global = &vm.page_default_context as *const _; + 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); + let global = context.global(scope); + let clone = global + .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"); + assert_eq!(materialized_clone.final_url.as_ref(), Some(&final_url)); + assert_eq!(materialized_clone.response_type, response_type); - 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"); - assert_eq!(materialized_cached.final_url.as_ref(), Some(&final_url)); - assert_eq!(materialized_cached.response_type, "opaqueredirect"); - Ok(()) - }) - .expect("filtered response clone/cache should materialize"); + 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"); + assert_eq!(materialized_cached.final_url.as_ref(), Some(&final_url)); + assert_eq!(materialized_cached.response_type, response_type); + Ok(()) + }) + .expect("filtered response clone/cache should materialize"); + } } #[test] diff --git a/moli-renderer-v8/src/worker/thread/tests/network.rs b/moli-renderer-v8/src/worker/thread/tests/network.rs index bebf48c01..5eec4ff85 100644 --- a/moli-renderer-v8/src/worker/thread/tests/network.rs +++ b/moli-renderer-v8/src/worker/thread/tests/network.rs @@ -5080,7 +5080,7 @@ async fn worker_fetch_manual_redirect_returns_opaqueredirect_filtered_response() ok: response.ok, statusText: response.statusText, redirected: response.redirected, - urlIsEmpty: response.url === "", + urlMatchesRequest: response.url === {url_literal}, bodyIsNull: response.body === null, headers: Array.from(response.headers), bodyUsedBefore, @@ -5088,6 +5088,7 @@ async fn worker_fetch_manual_redirect_returns_opaqueredirect_filtered_response() text, cloneType: clone.type, cloneStatus: clone.status, + cloneUrlMatchesRequest: clone.url === {url_literal}, cloneBodyIsNull: clone.body === null, cloneText, }}); @@ -5108,7 +5109,7 @@ async fn worker_fetch_manual_redirect_returns_opaqueredirect_filtered_response() .expect("worker fetch manual-redirect server should finish"); assert_eq!( post, - r#"{"type":"opaqueredirect","status":0,"ok":false,"statusText":"","redirected":false,"urlIsEmpty":true,"bodyIsNull":true,"headers":[],"bodyUsedBefore":false,"bodyUsedAfter":true,"text":"","cloneType":"opaqueredirect","cloneStatus":0,"cloneBodyIsNull":true,"cloneText":""}"# + r#"{"type":"opaqueredirect","status":0,"ok":false,"statusText":"","redirected":false,"urlMatchesRequest":true,"bodyIsNull":true,"headers":[],"bodyUsedBefore":false,"bodyUsedAfter":true,"text":"","cloneType":"opaqueredirect","cloneStatus":0,"cloneUrlMatchesRequest":true,"cloneBodyIsNull":true,"cloneText":""}"# ); }