From 934b3341d7c5e1140aab35ed9b978d74759ac327 Mon Sep 17 00:00:00 2001 From: Jan Varga Date: Fri, 4 Jul 2025 11:15:12 +0200 Subject: [PATCH] storage: Isolate sessionStorage per top-level browsing context and copy sessionStorage when creating a new auxiliary browsing context (#37803) This pull request introduces changes to the storage subsystem to: - Isolate sessionStorage per top-level browsing context (WebViewId), in addition to origin. - Copy sessionStorage when creating a new auxiliary browsing context without noopener, as required by the corresponding spec These changes bring Servo closer to spec compliance, matching expected browser behavior. Testing: This work affects observable behavior. As a result, some previously failing WPT tests now pass. No new tests are added, since the behavior is already covered by existing web-platform-tests. Fixes: #21291 --------- Signed-off-by: Jan Varga --- components/net/storage_thread.rs | 247 +++++++++++------- components/script/dom/storage.rs | 37 ++- components/script/dom/windowproxy.rs | 24 +- components/shared/net/storage_thread.rs | 42 ++- ...rage_session_window_noopener.window.js.ini | 3 - .../storage_session_window_open.window.js.ini | 3 - 6 files changed, 234 insertions(+), 122 deletions(-) delete mode 100644 tests/wpt/meta/webstorage/storage_session_window_noopener.window.js.ini delete mode 100644 tests/wpt/meta/webstorage/storage_session_window_open.window.js.ini diff --git a/components/net/storage_thread.rs b/components/net/storage_thread.rs index 899214a450c..da8740f11be 100644 --- a/components/net/storage_thread.rs +++ b/components/net/storage_thread.rs @@ -7,6 +7,7 @@ use std::collections::{BTreeMap, HashMap}; use std::path::PathBuf; use std::thread; +use base::id::WebViewId; use ipc_channel::ipc::{self, IpcReceiver, IpcSender}; use malloc_size_of::MallocSizeOf; use net_traits::storage_thread::{StorageThreadMsg, StorageType}; @@ -47,10 +48,12 @@ impl StorageThreadFactory for IpcSender { } } +type OriginEntry = (usize, BTreeMap); + struct StorageManager { port: IpcReceiver, - session_data: HashMap)>, - local_data: HashMap)>, + session_data: HashMap>, + local_data: HashMap, config_dir: Option, } @@ -73,30 +76,38 @@ impl StorageManager { fn start(&mut self) { loop { match self.port.recv().unwrap() { - StorageThreadMsg::Length(sender, url, storage_type) => { - self.length(sender, url, storage_type) + StorageThreadMsg::Length(sender, storage_type, webview_id, url) => { + self.length(sender, storage_type, webview_id, url) }, - StorageThreadMsg::Key(sender, url, storage_type, index) => { - self.key(sender, url, storage_type, index) + StorageThreadMsg::Key(sender, storage_type, webview_id, url, index) => { + self.key(sender, storage_type, webview_id, url, index) }, - StorageThreadMsg::Keys(sender, url, storage_type) => { - self.keys(sender, url, storage_type) + StorageThreadMsg::Keys(sender, storage_type, webview_id, url) => { + self.keys(sender, storage_type, webview_id, url) }, - StorageThreadMsg::SetItem(sender, url, storage_type, name, value) => { - self.set_item(sender, url, storage_type, name, value); + StorageThreadMsg::SetItem(sender, storage_type, webview_id, url, name, value) => { + self.set_item(sender, storage_type, webview_id, url, name, value); self.save_state() }, - StorageThreadMsg::GetItem(sender, url, storage_type, name) => { - self.request_item(sender, url, storage_type, name) + StorageThreadMsg::GetItem(sender, storage_type, webview_id, url, name) => { + self.request_item(sender, storage_type, webview_id, url, name) }, - StorageThreadMsg::RemoveItem(sender, url, storage_type, name) => { - self.remove_item(sender, url, storage_type, name); + StorageThreadMsg::RemoveItem(sender, storage_type, webview_id, url, name) => { + self.remove_item(sender, storage_type, webview_id, url, name); self.save_state() }, - StorageThreadMsg::Clear(sender, url, storage_type) => { - self.clear(sender, url, storage_type); + StorageThreadMsg::Clear(sender, storage_type, webview_id, url) => { + self.clear(sender, storage_type, webview_id, url); self.save_state() }, + StorageThreadMsg::Clone { + sender, + src: src_webview_id, + dest: dest_webview_id, + } => { + self.clone(src_webview_id, dest_webview_id); + let _ = sender.send(()); + }, StorageThreadMsg::CollectMemoryReport(sender) => { let reports = self.collect_memory_reports(); sender.send(ProcessReports::new(reports)); @@ -137,53 +148,90 @@ impl StorageManager { fn select_data( &self, storage_type: StorageType, - ) -> &HashMap)> { + webview_id: WebViewId, + origin: &str, + ) -> Option<&OriginEntry> { match storage_type { - StorageType::Session => &self.session_data, - StorageType::Local => &self.local_data, + StorageType::Session => self + .session_data + .get(&webview_id) + .and_then(|origin_map| origin_map.get(origin)), + StorageType::Local => self.local_data.get(origin), } } fn select_data_mut( &mut self, storage_type: StorageType, - ) -> &mut HashMap)> { + webview_id: WebViewId, + origin: &str, + ) -> Option<&mut OriginEntry> { match storage_type { - StorageType::Session => &mut self.session_data, - StorageType::Local => &mut self.local_data, + StorageType::Session => self + .session_data + .get_mut(&webview_id) + .and_then(|origin_map| origin_map.get_mut(origin)), + StorageType::Local => self.local_data.get_mut(origin), } } - fn length(&self, sender: IpcSender, url: ServoUrl, storage_type: StorageType) { + fn ensure_data_mut( + &mut self, + storage_type: StorageType, + webview_id: WebViewId, + origin: &str, + ) -> &mut OriginEntry { + match storage_type { + StorageType::Session => self + .session_data + .entry(webview_id) + .or_default() + .entry(origin.to_string()) + .or_default(), + StorageType::Local => self.local_data.entry(origin.to_string()).or_default(), + } + } + + fn length( + &self, + sender: IpcSender, + storage_type: StorageType, + webview_id: WebViewId, + url: ServoUrl, + ) { let origin = self.origin_as_string(url); - let data = self.select_data(storage_type); + let data = self.select_data(storage_type, webview_id, &origin); sender - .send(data.get(&origin).map_or(0, |(_, entry)| entry.len())) + .send(data.map_or(0, |(_, entry)| entry.len())) .unwrap(); } fn key( &self, sender: IpcSender>, - url: ServoUrl, storage_type: StorageType, + webview_id: WebViewId, + url: ServoUrl, index: u32, ) { let origin = self.origin_as_string(url); - let data = self.select_data(storage_type); + let data = self.select_data(storage_type, webview_id, &origin); let key = data - .get(&origin) .and_then(|(_, entry)| entry.keys().nth(index as usize)) .cloned(); sender.send(key).unwrap(); } - fn keys(&self, sender: IpcSender>, url: ServoUrl, storage_type: StorageType) { + fn keys( + &self, + sender: IpcSender>, + storage_type: StorageType, + webview_id: WebViewId, + url: ServoUrl, + ) { let origin = self.origin_as_string(url); - let data = self.select_data(storage_type); - let keys = data - .get(&origin) - .map_or(vec![], |(_, entry)| entry.keys().cloned().collect()); + let data = self.select_data(storage_type, webview_id, &origin); + let keys = data.map_or(vec![], |(_, entry)| entry.keys().cloned().collect()); sender.send(keys).unwrap(); } @@ -195,75 +243,64 @@ impl StorageManager { fn set_item( &mut self, sender: IpcSender), ()>>, - url: ServoUrl, storage_type: StorageType, + webview_id: WebViewId, + url: ServoUrl, name: String, value: String, ) { let origin = self.origin_as_string(url); let (this_storage_size, other_storage_size) = { - let local_data = self.select_data(StorageType::Local); - let session_data = self.select_data(StorageType::Session); - let local_data_size = local_data.get(&origin).map_or(0, |&(total, _)| total); - let session_data_size = session_data.get(&origin).map_or(0, |&(total, _)| total); + let local_data = self.select_data(StorageType::Local, webview_id, &origin); + let session_data = self.select_data(StorageType::Session, webview_id, &origin); + let local_data_size = local_data.map_or(0, |&(total, _)| total); + let session_data_size = session_data.map_or(0, |&(total, _)| total); match storage_type { StorageType::Local => (local_data_size, session_data_size), StorageType::Session => (session_data_size, local_data_size), } }; - let data = self.select_data_mut(storage_type); - if !data.contains_key(&origin) { - data.insert(origin.clone(), (0, BTreeMap::new())); + let &mut (ref mut total, ref mut entry) = + self.ensure_data_mut(storage_type, webview_id, &origin); + + let mut new_total_size = this_storage_size + value.len(); + if let Some(old_value) = entry.get(&name) { + new_total_size -= old_value.len(); + } else { + new_total_size += name.len(); } - let message = data - .get_mut(&origin) - .map(|&mut (ref mut total, ref mut entry)| { - let mut new_total_size = this_storage_size + value.len(); - if let Some(old_value) = entry.get(&name) { - new_total_size -= old_value.len(); - } else { - new_total_size += name.len(); - } - - if (new_total_size + other_storage_size) > QUOTA_SIZE_LIMIT { - return Err(()); - } - - let message = - entry - .insert(name.clone(), value.clone()) - .map_or(Ok((true, None)), |old| { - if old == value { - Ok((false, None)) - } else { - Ok((true, Some(old))) - } - }); - *total = new_total_size; - message - }) - .unwrap(); + let message = if (new_total_size + other_storage_size) > QUOTA_SIZE_LIMIT { + Err(()) + } else { + *total = new_total_size; + entry + .insert(name.clone(), value.clone()) + .map_or(Ok((true, None)), |old| { + if old == value { + Ok((false, None)) + } else { + Ok((true, Some(old))) + } + }) + }; sender.send(message).unwrap(); } fn request_item( &self, sender: IpcSender>, - url: ServoUrl, storage_type: StorageType, + webview_id: WebViewId, + url: ServoUrl, name: String, ) { let origin = self.origin_as_string(url); - let data = self.select_data(storage_type); + let data = self.select_data(storage_type, webview_id, &origin); sender - .send( - data.get(&origin) - .and_then(|(_, entry)| entry.get(&name)) - .cloned(), - ) + .send(data.and_then(|(_, entry)| entry.get(&name)).cloned()) .unwrap(); } @@ -271,41 +308,53 @@ impl StorageManager { fn remove_item( &mut self, sender: IpcSender>, - url: ServoUrl, storage_type: StorageType, + webview_id: WebViewId, + url: ServoUrl, name: String, ) { let origin = self.origin_as_string(url); - let data = self.select_data_mut(storage_type); - let old_value = data - .get_mut(&origin) - .and_then(|&mut (ref mut total, ref mut entry)| { - entry.remove(&name).inspect(|old| { - *total -= name.len() + old.len(); - }) - }); + let data = self.select_data_mut(storage_type, webview_id, &origin); + let old_value = data.and_then(|&mut (ref mut total, ref mut entry)| { + entry.remove(&name).inspect(|old| { + *total -= name.len() + old.len(); + }) + }); sender.send(old_value).unwrap(); } - fn clear(&mut self, sender: IpcSender, url: ServoUrl, storage_type: StorageType) { + fn clear( + &mut self, + sender: IpcSender, + storage_type: StorageType, + webview_id: WebViewId, + url: ServoUrl, + ) { let origin = self.origin_as_string(url); - let data = self.select_data_mut(storage_type); + let data = self.select_data_mut(storage_type, webview_id, &origin); sender - .send( - data.get_mut(&origin) - .is_some_and(|&mut (ref mut total, ref mut entry)| { - if !entry.is_empty() { - entry.clear(); - *total = 0; - true - } else { - false - } - }), - ) + .send(data.is_some_and(|&mut (ref mut total, ref mut entry)| { + if !entry.is_empty() { + entry.clear(); + *total = 0; + true + } else { + false + } + })) .unwrap(); } + fn clone(&mut self, src_webview_id: WebViewId, dest_webview_id: WebViewId) { + let Some(src_origin_entries) = self.session_data.get(&src_webview_id) else { + return; + }; + + let dest_origin_entries = src_origin_entries.clone(); + self.session_data + .insert(dest_webview_id, dest_origin_entries); + } + fn origin_as_string(&self, url: ServoUrl) -> String { url.origin().ascii_serialization() } diff --git a/components/script/dom/storage.rs b/components/script/dom/storage.rs index 054e4ab5ac3..126394bb351 100644 --- a/components/script/dom/storage.rs +++ b/components/script/dom/storage.rs @@ -2,6 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use base::id::WebViewId; use constellation_traits::ScriptToConstellationMessage; use dom_struct::dom_struct; use ipc_channel::ipc::IpcSender; @@ -49,6 +50,10 @@ impl Storage { ) } + fn webview_id(&self) -> WebViewId { + self.global().as_window().window_proxy().webview_id() + } + fn get_url(&self) -> ServoUrl { self.global().get_url() } @@ -66,8 +71,9 @@ impl StorageMethods for Storage { self.get_storage_thread() .send(StorageThreadMsg::Length( sender, - self.get_url(), self.storage_type, + self.webview_id(), + self.get_url(), )) .unwrap(); receiver.recv().unwrap() as u32 @@ -80,8 +86,9 @@ impl StorageMethods for Storage { self.get_storage_thread() .send(StorageThreadMsg::Key( sender, - self.get_url(), self.storage_type, + self.webview_id(), + self.get_url(), index, )) .unwrap(); @@ -93,7 +100,13 @@ impl StorageMethods for Storage { let (sender, receiver) = ipc::channel(self.global().time_profiler_chan().clone()).unwrap(); let name = String::from(name); - let msg = StorageThreadMsg::GetItem(sender, self.get_url(), self.storage_type, name); + let msg = StorageThreadMsg::GetItem( + sender, + self.storage_type, + self.webview_id(), + self.get_url(), + name, + ); self.get_storage_thread().send(msg).unwrap(); receiver.recv().unwrap().map(DOMString::from) } @@ -106,8 +119,9 @@ impl StorageMethods for Storage { let msg = StorageThreadMsg::SetItem( sender, - self.get_url(), self.storage_type, + self.webview_id(), + self.get_url(), name.clone(), value.clone(), ); @@ -128,8 +142,13 @@ impl StorageMethods for Storage { let (sender, receiver) = ipc::channel(self.global().time_profiler_chan().clone()).unwrap(); let name = String::from(name); - let msg = - StorageThreadMsg::RemoveItem(sender, self.get_url(), self.storage_type, name.clone()); + let msg = StorageThreadMsg::RemoveItem( + sender, + self.storage_type, + self.webview_id(), + self.get_url(), + name.clone(), + ); self.get_storage_thread().send(msg).unwrap(); if let Some(old_value) = receiver.recv().unwrap() { self.broadcast_change_notification(Some(name), Some(old_value), None); @@ -143,8 +162,9 @@ impl StorageMethods for Storage { self.get_storage_thread() .send(StorageThreadMsg::Clear( sender, - self.get_url(), self.storage_type, + self.webview_id(), + self.get_url(), )) .unwrap(); if receiver.recv().unwrap() { @@ -159,8 +179,9 @@ impl StorageMethods for Storage { self.get_storage_thread() .send(StorageThreadMsg::Keys( sender, - self.get_url(), self.storage_type, + self.webview_id(), + self.get_url(), )) .unwrap(); receiver diff --git a/components/script/dom/windowproxy.rs b/components/script/dom/windowproxy.rs index 648146ac2e9..022a940adc1 100644 --- a/components/script/dom/windowproxy.rs +++ b/components/script/dom/windowproxy.rs @@ -32,7 +32,9 @@ use js::jsval::{NullValue, PrivateValue, UndefinedValue}; use js::rust::wrappers::{JS_TransplantObject, NewWindowProxy, SetWindowProxy}; use js::rust::{Handle, MutableHandle, MutableHandleValue, get_object_class}; use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; +use net_traits::IpcSend; use net_traits::request::Referrer; +use net_traits::storage_thread::StorageThreadMsg; use script_traits::NewLayoutInfo; use serde::{Deserialize, Serialize}; use servo_url::{ImmutableOrigin, ServoUrl}; @@ -334,8 +336,6 @@ impl WindowProxy { theme: window.theme(), }; ScriptThread::process_attach_layout(new_layout_info, document.origin().clone()); - // TODO: if noopener is false, copy the sessionStorage storage area of the creator origin. - // See step 14 of https://html.spec.whatwg.org/multipage/#creating-a-new-browsing-context let new_window_proxy = ScriptThread::find_document(response.new_pipeline_id) .and_then(|doc| doc.browsing_context())?; if name.to_lowercase() != "_blank" { @@ -343,6 +343,26 @@ impl WindowProxy { } if noopener { new_window_proxy.disown(); + } else { + // After creating a new auxiliary browsing context and document, + // the session storage is copied over. + // See https://html.spec.whatwg.org/multipage/#the-sessionstorage-attribute + + let (sender, receiver) = ipc::channel().unwrap(); + + let msg = StorageThreadMsg::Clone { + sender, + src: window.window_proxy().webview_id(), + dest: response.new_webview_id, + }; + + document + .global() + .resource_threads() + .sender() + .send(msg) + .unwrap(); + receiver.recv().unwrap(); } Some(new_window_proxy) } diff --git a/components/shared/net/storage_thread.rs b/components/shared/net/storage_thread.rs index 2ba0aa12445..8b877678dce 100644 --- a/components/shared/net/storage_thread.rs +++ b/components/shared/net/storage_thread.rs @@ -2,6 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use base::id::WebViewId; use ipc_channel::ipc::IpcSender; use malloc_size_of_derive::MallocSizeOf; use profile_traits::mem::ReportsChan; @@ -18,31 +19,58 @@ pub enum StorageType { #[derive(Debug, Deserialize, Serialize)] pub enum StorageThreadMsg { /// gets the number of key/value pairs present in the associated storage data - Length(IpcSender, ServoUrl, StorageType), + Length(IpcSender, StorageType, WebViewId, ServoUrl), /// gets the name of the key at the specified index in the associated storage data - Key(IpcSender>, ServoUrl, StorageType, u32), + Key( + IpcSender>, + StorageType, + WebViewId, + ServoUrl, + u32, + ), /// Gets the available keys in the associated storage data - Keys(IpcSender>, ServoUrl, StorageType), + Keys(IpcSender>, StorageType, WebViewId, ServoUrl), /// gets the value associated with the given key in the associated storage data - GetItem(IpcSender>, ServoUrl, StorageType, String), + GetItem( + IpcSender>, + StorageType, + WebViewId, + ServoUrl, + String, + ), /// sets the value of the given key in the associated storage data SetItem( IpcSender), ()>>, - ServoUrl, StorageType, + WebViewId, + ServoUrl, String, String, ), /// removes the key/value pair for the given key in the associated storage data - RemoveItem(IpcSender>, ServoUrl, StorageType, String), + RemoveItem( + IpcSender>, + StorageType, + WebViewId, + ServoUrl, + String, + ), /// clears the associated storage data by removing all the key/value pairs - Clear(IpcSender, ServoUrl, StorageType), + Clear(IpcSender, StorageType, WebViewId, ServoUrl), + + /// clones all storage data of the given top-level browsing context for a new browsing context. + /// should only be used for sessionStorage. + Clone { + sender: IpcSender<()>, + src: WebViewId, + dest: WebViewId, + }, /// send a reply when done cleaning up thread resources and then shut it down Exit(IpcSender<()>), diff --git a/tests/wpt/meta/webstorage/storage_session_window_noopener.window.js.ini b/tests/wpt/meta/webstorage/storage_session_window_noopener.window.js.ini deleted file mode 100644 index a48d30bdf39..00000000000 --- a/tests/wpt/meta/webstorage/storage_session_window_noopener.window.js.ini +++ /dev/null @@ -1,3 +0,0 @@ -[storage_session_window_noopener.window.html] - [A new noopener window to make sure there is a not copy of the previous window's sessionStorage] - expected: FAIL diff --git a/tests/wpt/meta/webstorage/storage_session_window_open.window.js.ini b/tests/wpt/meta/webstorage/storage_session_window_open.window.js.ini deleted file mode 100644 index c2a5c444327..00000000000 --- a/tests/wpt/meta/webstorage/storage_session_window_open.window.js.ini +++ /dev/null @@ -1,3 +0,0 @@ -[storage_session_window_open.window.html] - [A new window to make sure there is a copy of the previous window's sessionStorage, and that they diverge after a change] - expected: FAIL