Propagate destination through load_data (#37020)

This way, we don't always set the destination to Document (which is as
the spec is written today). Instead, we set it it in the load_data,
depending on which context we load it from.

Doing so allows us to set the `Destination::IFrame` for navigations in
iframes, enabling all frame-related CSP checks.

While we currently block iframes when `frame-src` or `child-src` is set,
their respective tests don't pass yet. That's because we don't yet
handle the cases
where we fire the correct `load` event.

Also update one WPT test to correctly fail, rather than erroring. That's
because it was using the wrong JS test variable.

Part of #4577

Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
This commit is contained in:
Tim van der Lippe 2025-05-17 10:22:11 +02:00 committed by GitHub
parent a028291466
commit ed469fe72f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 17 additions and 14 deletions

View file

@ -16,6 +16,7 @@ use embedder_traits::ViewportDetails;
use html5ever::{LocalName, Prefix, local_name, ns}; use html5ever::{LocalName, Prefix, local_name, ns};
use js::rust::HandleObject; use js::rust::HandleObject;
use net_traits::ReferrerPolicy; use net_traits::ReferrerPolicy;
use net_traits::request::Destination;
use profile_traits::ipc as ProfiledIpc; use profile_traits::ipc as ProfiledIpc;
use script_traits::{NewLayoutInfo, UpdatePipelineIdReason}; use script_traits::{NewLayoutInfo, UpdatePipelineIdReason};
use servo_url::ServoUrl; use servo_url::ServoUrl;
@ -282,6 +283,7 @@ impl HTMLIFrameElement {
Some(document.insecure_requests_policy()), Some(document.insecure_requests_policy()),
document.has_trustworthy_ancestor_or_current_origin(), document.has_trustworthy_ancestor_or_current_origin(),
); );
load_data.destination = Destination::IFrame;
load_data.policy_container = Some(window.as_global_scope().policy_container()); load_data.policy_container = Some(window.as_global_scope().policy_container());
let element = self.upcast::<Element>(); let element = self.upcast::<Element>();
load_data.srcdoc = String::from(element.get_string_attribute(&local_name!("srcdoc"))); load_data.srcdoc = String::from(element.get_string_attribute(&local_name!("srcdoc")));
@ -375,6 +377,8 @@ impl HTMLIFrameElement {
Some(document.insecure_requests_policy()), Some(document.insecure_requests_policy()),
document.has_trustworthy_ancestor_or_current_origin(), document.has_trustworthy_ancestor_or_current_origin(),
); );
load_data.destination = Destination::IFrame;
load_data.policy_container = Some(window.as_global_scope().policy_container());
let pipeline_id = self.pipeline_id(); let pipeline_id = self.pipeline_id();
// If the initial `about:blank` page is the current page, load with replacement enabled, // If the initial `about:blank` page is the current page, load with replacement enabled,
@ -382,10 +386,6 @@ impl HTMLIFrameElement {
let is_about_blank = let is_about_blank =
pipeline_id.is_some() && pipeline_id == self.about_blank_pipeline_id.get(); pipeline_id.is_some() && pipeline_id == self.about_blank_pipeline_id.get();
if is_about_blank {
load_data.policy_container = Some(window.as_global_scope().policy_container());
}
let history_handling = if is_about_blank { let history_handling = if is_about_blank {
NavigationHistoryBehavior::Replace NavigationHistoryBehavior::Replace
} else { } else {
@ -425,6 +425,7 @@ impl HTMLIFrameElement {
Some(document.insecure_requests_policy()), Some(document.insecure_requests_policy()),
document.has_trustworthy_ancestor_or_current_origin(), document.has_trustworthy_ancestor_or_current_origin(),
); );
load_data.destination = Destination::IFrame;
load_data.policy_container = Some(window.as_global_scope().policy_container()); load_data.policy_container = Some(window.as_global_scope().policy_container());
let browsing_context_id = BrowsingContextId::new(); let browsing_context_id = BrowsingContextId::new();
let webview_id = window.window_proxy().webview_id(); let webview_id = window.window_proxy().webview_id();

View file

@ -11,7 +11,6 @@ use std::cell::Cell;
use base::cross_process_instant::CrossProcessInstant; use base::cross_process_instant::CrossProcessInstant;
use base::id::{BrowsingContextId, PipelineId, WebViewId}; use base::id::{BrowsingContextId, PipelineId, WebViewId};
use constellation_traits::LoadData; use constellation_traits::LoadData;
use content_security_policy::Destination;
use crossbeam_channel::Sender; use crossbeam_channel::Sender;
use embedder_traits::ViewportDetails; use embedder_traits::ViewportDetails;
use http::header; use http::header;
@ -202,12 +201,13 @@ impl InProgressLoad {
self.load_data.referrer.clone(), self.load_data.referrer.clone(),
) )
.method(self.load_data.method.clone()) .method(self.load_data.method.clone())
.destination(Destination::Document) .destination(self.load_data.destination)
.mode(RequestMode::Navigate) .mode(RequestMode::Navigate)
.credentials_mode(CredentialsMode::Include) .credentials_mode(CredentialsMode::Include)
.use_url_credentials(true) .use_url_credentials(true)
.pipeline_id(Some(id)) .pipeline_id(Some(id))
.referrer_policy(self.load_data.referrer_policy) .referrer_policy(self.load_data.referrer_policy)
.policy_container(self.load_data.policy_container.clone().unwrap_or_default())
.insecure_requests_policy( .insecure_requests_policy(
self.load_data self.load_data
.inherited_insecure_requests_policy .inherited_insecure_requests_policy

View file

@ -23,7 +23,7 @@ use http::{HeaderMap, Method};
use ipc_channel::Error as IpcError; use ipc_channel::Error as IpcError;
use ipc_channel::ipc::{IpcReceiver, IpcSender}; use ipc_channel::ipc::{IpcReceiver, IpcSender};
use net_traits::policy_container::PolicyContainer; use net_traits::policy_container::PolicyContainer;
use net_traits::request::{InsecureRequestsPolicy, Referrer, RequestBody}; use net_traits::request::{Destination, InsecureRequestsPolicy, Referrer, RequestBody};
use net_traits::storage_thread::StorageType; use net_traits::storage_thread::StorageType;
use net_traits::{CoreResourceMsg, ReferrerPolicy, ResourceThreads}; use net_traits::{CoreResourceMsg, ReferrerPolicy, ResourceThreads};
use profile_traits::mem::MemoryReportResult; use profile_traits::mem::MemoryReportResult;
@ -111,6 +111,8 @@ pub struct LoadData {
pub has_trustworthy_ancestor_origin: bool, pub has_trustworthy_ancestor_origin: bool,
/// Servo internal: if crash details are present, trigger a crash error page with these details. /// Servo internal: if crash details are present, trigger a crash error page with these details.
pub crash: Option<String>, pub crash: Option<String>,
/// Destination, used for CSP checks
pub destination: Destination,
} }
/// The result of evaluating a javascript scheme url. /// The result of evaluating a javascript scheme url.
@ -152,6 +154,7 @@ impl LoadData {
crash: None, crash: None,
inherited_insecure_requests_policy, inherited_insecure_requests_policy,
has_trustworthy_ancestor_origin, has_trustworthy_ancestor_origin,
destination: Destination::Document,
} }
} }
} }

View file

@ -567585,7 +567585,7 @@
] ]
], ],
"frame-src-blocked.sub.html": [ "frame-src-blocked.sub.html": [
"a4957f8715c4bdc0db9473caee3fc9f2e767fd71", "76fcc2cbb536ba9ec0c8741ad9fe9af470165d32",
[ [
null, null,
{} {}

View file

@ -1,4 +1,3 @@
[frame-src-blocked.sub.html] [frame-src-blocked.sub.html]
expected: ERROR
[Expecting logs: ["PASS IFrame #1 generated a load event.","violated-directive=frame-src"\]] [Expecting logs: ["PASS IFrame #1 generated a load event.","violated-directive=frame-src"\]]
expected: FAIL expected: FAIL

View file

@ -1,4 +1,4 @@
[frame-src-cross-origin-same-document-navigation.window.html] [frame-src-cross-origin-same-document-navigation.window.html]
expected: OK expected: TIMEOUT
[frame-src-cross-origin-same-document-navigation] [frame-src-cross-origin-same-document-navigation]
expected: FAIL expected: TIMEOUT

View file

@ -18,17 +18,17 @@
}, false); }, false);
function alert_assert(msg) { function alert_assert(msg) {
t_alert.step(function() { t_log.step(function() {
if (msg.match(/^FAIL/i)) { if (msg.match(/^FAIL/i)) {
assert_unreached(msg); assert_unreached(msg);
t_alert.done(); t_log.done();
} }
for (var i = 0; i < expected_alerts.length; i++) { for (var i = 0; i < expected_alerts.length; i++) {
if (expected_alerts[i] == msg) { if (expected_alerts[i] == msg) {
assert_equals(expected_alerts[i], msg); assert_equals(expected_alerts[i], msg);
expected_alerts.splice(i, 1); expected_alerts.splice(i, 1);
if (expected_alerts.length == 0) { if (expected_alerts.length == 0) {
t_alert.done(); t_log.done();
} }
return; return;
} }