Skip to content

Commit fb00bd3

Browse files
committed
Propagate destination through load_data
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 servo#4577 Signed-off-by: Tim van der Lippe <[email protected]>
1 parent f7b1673 commit fb00bd3

File tree

6 files changed

+21
-12
lines changed

6 files changed

+21
-12
lines changed

components/script/dom/htmliframeelement.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use embedder_traits::ViewportDetails;
1616
use html5ever::{LocalName, Prefix, local_name, ns};
1717
use js::rust::HandleObject;
1818
use net_traits::ReferrerPolicy;
19+
use net_traits::request::Destination;
1920
use profile_traits::ipc as ProfiledIpc;
2021
use script_traits::{NewLayoutInfo, UpdatePipelineIdReason};
2122
use servo_url::ServoUrl;
@@ -282,6 +283,7 @@ impl HTMLIFrameElement {
282283
Some(document.insecure_requests_policy()),
283284
document.has_trustworthy_ancestor_or_current_origin(),
284285
);
286+
load_data.destination = Destination::IFrame;
285287
load_data.policy_container = Some(window.as_global_scope().policy_container());
286288
let element = self.upcast::<Element>();
287289
load_data.srcdoc = String::from(element.get_string_attribute(&local_name!("srcdoc")));
@@ -375,17 +377,15 @@ impl HTMLIFrameElement {
375377
Some(document.insecure_requests_policy()),
376378
document.has_trustworthy_ancestor_or_current_origin(),
377379
);
380+
load_data.destination = Destination::IFrame;
381+
load_data.policy_container = Some(window.as_global_scope().policy_container());
378382

379383
let pipeline_id = self.pipeline_id();
380384
// If the initial `about:blank` page is the current page, load with replacement enabled,
381385
// see https://html.spec.whatwg.org/multipage/#the-iframe-element:about:blank-3
382386
let is_about_blank =
383387
pipeline_id.is_some() && pipeline_id == self.about_blank_pipeline_id.get();
384388

385-
if is_about_blank {
386-
load_data.policy_container = Some(window.as_global_scope().policy_container());
387-
}
388-
389389
let history_handling = if is_about_blank {
390390
NavigationHistoryBehavior::Replace
391391
} else {
@@ -425,6 +425,7 @@ impl HTMLIFrameElement {
425425
Some(document.insecure_requests_policy()),
426426
document.has_trustworthy_ancestor_or_current_origin(),
427427
);
428+
load_data.destination = Destination::IFrame;
428429
load_data.policy_container = Some(window.as_global_scope().policy_container());
429430
let browsing_context_id = BrowsingContextId::new();
430431
let webview_id = window.window_proxy().webview_id();

components/script/navigation.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ use std::cell::Cell;
1111
use base::cross_process_instant::CrossProcessInstant;
1212
use base::id::{BrowsingContextId, PipelineId, WebViewId};
1313
use constellation_traits::LoadData;
14-
use content_security_policy::Destination;
1514
use crossbeam_channel::Sender;
1615
use embedder_traits::ViewportDetails;
1716
use http::header;
17+
use net_traits::policy_container::PolicyContainer;
1818
use net_traits::request::{
1919
CredentialsMode, InsecureRequestsPolicy, RedirectMode, RequestBuilder, RequestMode,
2020
};
@@ -202,12 +202,18 @@ impl InProgressLoad {
202202
self.load_data.referrer.clone(),
203203
)
204204
.method(self.load_data.method.clone())
205-
.destination(Destination::Document)
205+
.destination(self.load_data.destination.clone())
206206
.mode(RequestMode::Navigate)
207207
.credentials_mode(CredentialsMode::Include)
208208
.use_url_credentials(true)
209209
.pipeline_id(Some(id))
210210
.referrer_policy(self.load_data.referrer_policy)
211+
.policy_container(
212+
self.load_data
213+
.policy_container
214+
.clone()
215+
.unwrap_or(PolicyContainer::default()),
216+
)
211217
.insecure_requests_policy(
212218
self.load_data
213219
.inherited_insecure_requests_policy

components/shared/constellation/from_script_message.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use http::{HeaderMap, Method};
2323
use ipc_channel::Error as IpcError;
2424
use ipc_channel::ipc::{IpcReceiver, IpcSender};
2525
use net_traits::policy_container::PolicyContainer;
26-
use net_traits::request::{InsecureRequestsPolicy, Referrer, RequestBody};
26+
use net_traits::request::{Destination, InsecureRequestsPolicy, Referrer, RequestBody};
2727
use net_traits::storage_thread::StorageType;
2828
use net_traits::{CoreResourceMsg, ReferrerPolicy, ResourceThreads};
2929
use profile_traits::mem::MemoryReportResult;
@@ -111,6 +111,8 @@ pub struct LoadData {
111111
pub has_trustworthy_ancestor_origin: bool,
112112
/// Servo internal: if crash details are present, trigger a crash error page with these details.
113113
pub crash: Option<String>,
114+
/// Destination, used for CSP checks
115+
pub destination: Destination,
114116
}
115117

116118
/// The result of evaluating a javascript scheme url.
@@ -152,6 +154,7 @@ impl LoadData {
152154
crash: None,
153155
inherited_insecure_requests_policy,
154156
has_trustworthy_ancestor_origin,
157+
destination: Destination::Document,
155158
}
156159
}
157160
}

tests/wpt/meta/MANIFEST.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567572,7 +567572,7 @@
567572567572
]
567573567573
],
567574567574
"frame-src-blocked.sub.html": [
567575-
"a4957f8715c4bdc0db9473caee3fc9f2e767fd71",
567575+
"76fcc2cbb536ba9ec0c8741ad9fe9af470165d32",
567576567576
[
567577567577
null,
567578567578
{}
Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
11
[frame-src-blocked.sub.html]
2-
expected: ERROR
32
[Expecting logs: ["PASS IFrame #1 generated a load event.","violated-directive=frame-src"\]]
43
expected: FAIL

tests/wpt/tests/content-security-policy/frame-src/frame-src-blocked.sub.html

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,17 @@
1818
}, false);
1919

2020
function alert_assert(msg) {
21-
t_alert.step(function() {
21+
t_log.step(function() {
2222
if (msg.match(/^FAIL/i)) {
2323
assert_unreached(msg);
24-
t_alert.done();
24+
t_log.done();
2525
}
2626
for (var i = 0; i < expected_alerts.length; i++) {
2727
if (expected_alerts[i] == msg) {
2828
assert_equals(expected_alerts[i], msg);
2929
expected_alerts.splice(i, 1);
3030
if (expected_alerts.length == 0) {
31-
t_alert.done();
31+
t_log.done();
3232
}
3333
return;
3434
}

0 commit comments

Comments
 (0)