From 8ceadf46df09548c5b1a0ca19c7531c206fdff7c Mon Sep 17 00:00:00 2001 From: ldm0 Date: Sun, 30 Aug 2026 06:29:11 +0800 Subject: [PATCH] fix(blob): revoke object URLs with window realms --- .../wpt-cross-current/failed-cases.txt | 1 - .../wpt-cross-current/passed-cases.txt | 1 + moli-file-api/src/blob_store.rs | 92 +++++++++++++++++-- moli-renderer-v8/src/blob.rs | 10 +- .../context_host/runtime_observable.rs | 11 ++- .../src/script_vm/tests/browser_api/misc.rs | 51 ++++++++++ 6 files changed, 156 insertions(+), 10 deletions(-) diff --git a/moli-benchmark/wpt-cross-current/failed-cases.txt b/moli-benchmark/wpt-cross-current/failed-cases.txt index 0639fbc5c6..615d01fb48 100644 --- a/moli-benchmark/wpt-cross-current/failed-cases.txt +++ b/moli-benchmark/wpt-cross-current/failed-cases.txt @@ -1,6 +1,5 @@ FileAPI/idlharness.html FileAPI/url/sandboxed-iframe.html -FileAPI/url/url-lifetime.html acid/acid3/numbered-tests.html client-hints/service-workers/intercept-request.https.html client-hints/service-workers/new-request.https.html diff --git a/moli-benchmark/wpt-cross-current/passed-cases.txt b/moli-benchmark/wpt-cross-current/passed-cases.txt index 65d2439a2e..dc95af11fc 100644 --- a/moli-benchmark/wpt-cross-current/passed-cases.txt +++ b/moli-benchmark/wpt-cross-current/passed-cases.txt @@ -5,6 +5,7 @@ FileAPI/FileReader/workers.html FileAPI/blob/Blob-constructor-endings.html FileAPI/file/File-constructor-endings.html FileAPI/filelist-section/filelist.html +FileAPI/url/url-lifetime.html IndexedDB/database-names-by-origin.html IndexedDB/idb_webworkers.htm IndexedDB/idbfactory-origin-isolation.html diff --git a/moli-file-api/src/blob_store.rs b/moli-file-api/src/blob_store.rs index f06df45d73..82b7749dae 100644 --- a/moli-file-api/src/blob_store.rs +++ b/moli-file-api/src/blob_store.rs @@ -43,6 +43,7 @@ impl Default for BlobEntries { #[derive(Clone, Copy, Debug)] struct ObjectUrlState { owner_id: Option, + lifetime_id: Option, blob_id: BlobId, } @@ -171,6 +172,17 @@ where owner_id: Option, blob_id: BlobId, origin: &str, + ) -> Option { + self.create_object_url_with_lifetime(owner_id, None, blob_id, origin) + } + + /// Create an object URL tied to a more specific execution-context lifetime. + pub fn create_object_url_with_lifetime( + &self, + owner_id: Option, + lifetime_id: Option, + blob_id: BlobId, + origin: &str, ) -> Option { self.retain_blob_object_url_ref(blob_id)?; let object_url_id = self @@ -178,9 +190,14 @@ where .fetch_add(1, Ordering::Relaxed) .max(1); let object_url = format!("blob:{origin}/{object_url_id}"); - self.object_urls - .lock() - .insert(object_url.clone(), ObjectUrlState { owner_id, blob_id }); + self.object_urls.lock().insert( + object_url.clone(), + ObjectUrlState { + owner_id, + lifetime_id, + blob_id, + }, + ); Some(object_url) } @@ -212,6 +229,28 @@ where Some((String::from_utf8_lossy(&bytes).into_owned(), mime_type)) } + /// Revoke every object URL created by one execution-context lifetime. + pub fn cleanup_object_url_lifetime(&self, lifetime_id: u64) -> usize { + let removed_blob_ids = { + let mut object_urls = self.object_urls.lock(); + let mut removed_blob_ids = Vec::new(); + object_urls.retain(|_, state| { + if state.lifetime_id == Some(lifetime_id) { + removed_blob_ids.push(state.blob_id); + false + } else { + true + } + }); + removed_blob_ids + }; + let removed_count = removed_blob_ids.len(); + for blob_id in removed_blob_ids { + self.release_blob_object_url_ref(blob_id); + } + removed_count + } + /// Remove Blob/object URL entries owned by a context. pub fn cleanup_owner_resources(&self, owner_id: OwnerId) { let removed_blob_ids = { @@ -229,9 +268,22 @@ where ids }; - self.object_urls.lock().retain(|_, state| { - state.owner_id != Some(owner_id) && !removed_blob_ids.contains(&state.blob_id) - }); + let released_blob_ids = { + let mut object_urls = self.object_urls.lock(); + let mut released_blob_ids = Vec::new(); + object_urls.retain(|_, state| { + let remove = + state.owner_id == Some(owner_id) || removed_blob_ids.contains(&state.blob_id); + if remove && !removed_blob_ids.contains(&state.blob_id) { + released_blob_ids.push(state.blob_id); + } + !remove + }); + released_blob_ids + }; + for blob_id in released_blob_ids { + self.release_blob_object_url_ref(blob_id); + } } /// Retain a reader reference for a Blob. @@ -441,4 +493,32 @@ mod tests { Some((b"other".to_vec(), "text/plain".to_owned())) ); } + + #[test] + fn cleanup_object_url_lifetime_revokes_only_matching_urls() { + let store = BlobStore::::default(); + let blob = store.create_blob( + Some(1), + Some(10), + b"shared".to_vec(), + "text/plain".to_owned(), + ); + let first_url = store + .create_object_url_with_lifetime(Some(1), Some(101), blob, "https://example.test") + .expect("first object URL"); + let second_url = store + .create_object_url_with_lifetime(Some(1), Some(202), blob, "https://example.test") + .expect("second object URL"); + + assert_eq!(store.cleanup_object_url_lifetime(101), 1); + assert!(store.object_url_bytes_and_type(&first_url).is_none()); + assert_eq!( + store.object_url_bytes_and_type(&second_url), + Some((b"shared".to_vec(), "text/plain".to_owned())) + ); + + store.release_blob_wrapper_ref(blob); + assert_eq!(store.cleanup_object_url_lifetime(202), 1); + assert!(store.blob_bytes(blob).is_none()); + } } diff --git a/moli-renderer-v8/src/blob.rs b/moli-renderer-v8/src/blob.rs index 0c68d1df42..f6b1ca3a48 100644 --- a/moli-renderer-v8/src/blob.rs +++ b/moli-renderer-v8/src/blob.rs @@ -366,7 +366,9 @@ pub(super) fn create_object_url_for_object<'s>( ) -> Option { let blob_id = blob_id_from_object(scope, object)?; let owner_id = current_resource_owner_id(scope); - blob_store().create_object_url(owner_id, blob_id, origin) + let lifetime_id = native_bridge::current_runtime_observable_context_token(scope) + .map(native_bridge::RuntimeObservableContextToken::as_u64); + blob_store().create_object_url_with_lifetime(owner_id, lifetime_id, blob_id, origin) } pub(super) fn revoke_object_url(url: &str) { @@ -486,6 +488,12 @@ pub(crate) fn cleanup_owner_resources(owner_id: ResourceOwnerId) { blob_store().cleanup_owner_resources(owner_id); } +pub(crate) fn cleanup_object_urls_for_context( + context_token: native_bridge::RuntimeObservableContextToken, +) -> usize { + blob_store().cleanup_object_url_lifetime(context_token.as_u64()) +} + fn release_blob_wrapper_ref(blob_id: BlobId) { blob_store().release_blob_wrapper_ref(blob_id); } diff --git a/moli-renderer-v8/src/native_bridge/context_host/runtime_observable.rs b/moli-renderer-v8/src/native_bridge/context_host/runtime_observable.rs index 477989ba45..884e407f56 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/runtime_observable.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/runtime_observable.rs @@ -422,6 +422,8 @@ impl JsContextHost { &mut self, context_token: RuntimeObservableContextToken, ) -> usize { + let revoked_blob_object_url_count = + crate::blob::cleanup_object_urls_for_context(context_token); crate::observer_runtime::retire_context_token(self, context_token); let indexed_db_retirement = self.retire_indexed_db_context(context_token); let retired_indexed_db_connections = indexed_db_retirement.retired_connections.len(); @@ -449,13 +451,15 @@ impl JsContextHost { ?context_token, retired_count, retired_indexed_db_connections, + revoked_blob_object_url_count, "retired LocalWindow bindings with destroyed V8 execution context" ); - } else if retired_indexed_db_connections > 0 { + } else if retired_indexed_db_connections > 0 || revoked_blob_object_url_count > 0 { tracing::debug!( ?context_token, retired_indexed_db_connections, - "retired IndexedDB state with destroyed V8 execution context" + revoked_blob_object_url_count, + "retired context-owned state with destroyed V8 execution context" ); } retired_count @@ -473,6 +477,8 @@ impl JsContextHost { &mut self, context_token: RuntimeObservableContextToken, ) -> usize { + let revoked_blob_object_url_count = + crate::blob::cleanup_object_urls_for_context(context_token); crate::observer_runtime::retire_context_token(self, context_token); let retired_realm_count = self .window_execution_context_realms @@ -480,6 +486,7 @@ impl JsContextHost { tracing::debug!( ?context_token, retired_realm_count, + revoked_blob_object_url_count, "retired isolated Window realm registration" ); retired_realm_count 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 5be7762e76..145a91216b 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 @@ -3340,6 +3340,57 @@ fn blob_slice_uses_receiver_realm_after_method_realm_is_detached() { ); } +#[test] +fn removing_child_frame_revokes_only_its_blob_object_urls() { + let mut vm = new_parsed_test_vm( + "https://blob-url-child-lifetime.test/", + "", + ); + + let urls = vm + .eval( + r#" +(() => { + const parentUrl = URL.createObjectURL(new Blob(["parent"])); + const frame = document.createElement("iframe"); + document.body.appendChild(frame); + const childUrl = frame.contentWindow.URL.createObjectURL( + new frame.contentWindow.Blob(["child"]) + ); + globalThis.__blobUrlLifetimeFrame = frame; + return `${parentUrl}|${childUrl}`; +})() +"#, + ) + .expect("child Blob object URL setup should evaluate"); + let (parent_url, child_url) = urls + .split_once('|') + .expect("setup should return both object URLs"); + + assert_eq!( + crate::blob::object_url_body_and_type(parent_url), + Some(("parent".to_owned(), String::new())) + ); + assert_eq!( + crate::blob::object_url_body_and_type(child_url), + Some(("child".to_owned(), String::new())) + ); + + vm.eval("globalThis.__blobUrlLifetimeFrame.remove()") + .expect("child frame removal should evaluate"); + vm.drain_pending_child_frame_work_for_test(); + + assert_eq!( + crate::blob::object_url_body_and_type(parent_url), + Some(("parent".to_owned(), String::new())), + "removing a child frame must preserve the parent realm's object URLs" + ); + assert!( + crate::blob::object_url_body_and_type(child_url).is_none(), + "removing a child frame must revoke object URLs created by its realm" + ); +} + #[test] fn blob_stream_is_native_readable_stream_and_response_consumes_bytes() { let mut vm = new_storage_test_vm("https://blob-stream-reader.test/");