From 9e635bb90c73080b99c619409bda1c8d026dfd35 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 | 97 ++++++++++++++++++- moli-renderer-v8/src/blob.rs | 16 ++- .../context_host/runtime_observable.rs | 11 ++- .../misc/extracted/blob_file_reader.rs | 51 ++++++++++ 6 files changed, 164 insertions(+), 13 deletions(-) diff --git a/moli-benchmark/wpt-cross-current/failed-cases.txt b/moli-benchmark/wpt-cross-current/failed-cases.txt index e3a576d4e..4c700d565 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 525182c81..62dae397b 100644 --- a/moli-benchmark/wpt-cross-current/passed-cases.txt +++ b/moli-benchmark/wpt-cross-current/passed-cases.txt @@ -7,6 +7,7 @@ FileAPI/blob/Blob-stream-byob-crash.html FileAPI/blob/Blob-stream-sync-xhr-crash.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-databases-opaque-origin.html diff --git a/moli-file-api/src/blob_store.rs b/moli-file-api/src/blob_store.rs index e9163cc66..d149bcef6 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(Debug)] struct ObjectUrlState { owner_id: Option, + lifetime_id: Option, blob_id: BlobId, access_key: Option, } @@ -169,14 +170,36 @@ where self.create_object_url_with_access_key(owner_id, blob_id, origin, None) } - /// Create an object URL with its creator environment’s access key. - /// The key belongs to the URL, independently of the backing Blob. + /// 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.create_object_url_with_lifetime_and_access_key(owner_id, lifetime_id, blob_id, origin, None) + } + + /// Create an object URL with its creator environment's access key. pub fn create_object_url_with_access_key( &self, owner_id: Option, blob_id: BlobId, origin: &str, access_key: Option, + ) -> Option { + self.create_object_url_with_lifetime_and_access_key(owner_id, None, blob_id, origin, access_key) + } + + /// Associate the URL's independent creator key and execution-context lifetime. + pub fn create_object_url_with_lifetime_and_access_key( + &self, + owner_id: Option, + lifetime_id: Option, + blob_id: BlobId, + origin: &str, + access_key: Option, ) -> Option { self.retain_blob_object_url_ref(blob_id)?; let mut object_urls = self.object_urls.lock(); @@ -190,6 +213,7 @@ where object_url.clone(), ObjectUrlState { owner_id, + lifetime_id, blob_id, access_key, }, @@ -246,6 +270,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 = { @@ -263,9 +309,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. @@ -535,4 +594,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 a4c0c0d49..b785068fc 100644 --- a/moli-renderer-v8/src/blob.rs +++ b/moli-renderer-v8/src/blob.rs @@ -349,14 +349,14 @@ pub(super) fn create_object_url_for_object<'s>( let partition = current_blob_storage_partition_identity(scope)?; let owner_id = current_resource_owner_id(scope); let origin = storage_key.origin().to_owned(); - blob_store().create_object_url_with_access_key( + let lifetime_id = native_bridge::current_runtime_observable_context_token(scope) + .map(native_bridge::RuntimeObservableContextToken::as_u64); + blob_store().create_object_url_with_lifetime_and_access_key( owner_id, + lifetime_id, blob_id, &origin, - Some(ObjectUrlAccessKey { - partition, - storage_key, - }), + Some(ObjectUrlAccessKey { partition, storage_key }), ) } @@ -490,6 +490,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 477989ba4..884e407f5 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/extracted/blob_file_reader.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/blob_file_reader.rs index 3ce8f5f75..b25b0c4d9 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/blob_file_reader.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/blob_file_reader.rs @@ -1045,3 +1045,54 @@ fn blob_url_revocation_respects_browser_partitions_and_allows_same_origin_realms assert!(crate::blob::object_url_body_and_type(&url).is_none()); } } + +#[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" + ); +}