From 8edc6ba1b28fda31a8c52104ee519499654f8251 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Fri, 11 Jul 2025 10:42:51 +0200 Subject: [PATCH] Return correct source position for element CSP violations (#37970) The scripted_caller only has information if the context is coming from a script. If an element fetch listener processes CSP violations, then this information doesn't exist. Instead, we should use the global URL and the line number. WPT tests don't appear to expect a column number, as they are all zero. Not all elements are updated, as I am not actually sure all of them need it. The source position remains an Option, since there are also code paths that don't correspond to element or script sources. Maybe in the future we can always determine the source position, but let's take small steps towards that. Part of #4577 Signed-off-by: Tim van der Lippe Co-authored-by: Josh Matthews --- components/script/dom/create.rs | 2 +- components/script/dom/csp.rs | 57 ++++++++++++++----- components/script/dom/element.rs | 10 +++- components/script/dom/eventsource.rs | 2 +- components/script/dom/htmlimageelement.rs | 15 ++++- components/script/dom/htmllinkelement.rs | 15 ++++- components/script/dom/htmlmediaelement.rs | 2 +- components/script/dom/htmlscriptelement.rs | 5 +- components/script/dom/htmlvideoelement.rs | 2 +- components/script/dom/notification.rs | 2 +- components/script/dom/servoparser/mod.rs | 11 +++- components/script/dom/websocket.rs | 2 +- components/script/dom/xmlhttprequest.rs | 2 +- components/script/fetch.rs | 2 +- components/script/layout_image.rs | 2 +- components/script/script_module.rs | 2 +- components/script/script_thread.rs | 2 +- components/script/security_manager.rs | 2 +- components/script/stylesheet_loader.rs | 2 +- ...rective-allowed-in-meta.https.sub.html.ini | 3 - ...ds-reports-on-violation.https.sub.html.ini | 3 - 21 files changed, 103 insertions(+), 42 deletions(-) delete mode 100644 tests/wpt/meta/content-security-policy/reporting-api/report-to-directive-allowed-in-meta.https.sub.html.ini delete mode 100644 tests/wpt/meta/content-security-policy/reporting-api/reporting-api-sends-reports-on-violation.https.sub.html.ini diff --git a/components/script/dom/create.rs b/components/script/dom/create.rs index 2e7c4cf8def..6edf4a67c34 100644 --- a/components/script/dom/create.rs +++ b/components/script/dom/create.rs @@ -344,7 +344,7 @@ pub(crate) fn create_native_html_element( local_name!("html") => make!(HTMLHtmlElement), local_name!("i") => make!(HTMLElement), local_name!("iframe") => make!(HTMLIFrameElement), - local_name!("img") => make!(HTMLImageElement), + local_name!("img") => make!(HTMLImageElement, creator), local_name!("input") => make!(HTMLInputElement), local_name!("ins") => make!(HTMLModElement), // https://html.spec.whatwg.org/multipage/#other-elements,-attributes-and-apis:isindex-2 diff --git a/components/script/dom/csp.rs b/components/script/dom/csp.rs index ddef9498ee4..f9e552cf1b5 100644 --- a/components/script/dom/csp.rs +++ b/components/script/dom/csp.rs @@ -72,7 +72,7 @@ impl CspReporting for Option { let (is_js_evaluation_allowed, violations) = csp_list.is_js_evaluation_allowed(source); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); is_js_evaluation_allowed == CheckResult::Allowed } @@ -85,7 +85,7 @@ impl CspReporting for Option { let (is_wasm_evaluation_allowed, violations) = csp_list.is_wasm_evaluation_allowed(); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); is_wasm_evaluation_allowed == CheckResult::Allowed } @@ -118,7 +118,7 @@ impl CspReporting for Option { let (result, violations) = csp_list.should_navigation_request_be_blocked(&request, NavigationCheckType::Other); - global.report_csp_violations(violations, element); + global.report_csp_violations(violations, element, None); result == CheckResult::Blocked } @@ -140,7 +140,7 @@ impl CspReporting for Option { let (result, violations) = csp_list.should_elements_inline_type_behavior_be_blocked(&element, type_, source); - global.report_csp_violations(violations, Some(el)); + global.report_csp_violations(violations, Some(el), None); result == CheckResult::Blocked } @@ -159,7 +159,7 @@ impl CspReporting for Option { let (allowed_by_csp, violations) = csp_list.is_trusted_type_policy_creation_allowed(policy_name, created_policy_names); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); allowed_by_csp == CheckResult::Allowed } @@ -192,26 +192,53 @@ impl CspReporting for Option { let (allowed_by_csp, violations) = csp_list .should_sink_type_mismatch_violation_be_blocked_by_csp(sink, sink_group, source); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); allowed_by_csp == CheckResult::Blocked } } +pub(crate) struct SourcePosition { + pub(crate) source_file: String, + pub(crate) line_number: u32, + pub(crate) column_number: u32, +} + pub(crate) trait GlobalCspReporting { - fn report_csp_violations(&self, violations: Vec, element: Option<&Element>); + fn report_csp_violations( + &self, + violations: Vec, + element: Option<&Element>, + source_position: Option, + ); +} + +#[allow(unsafe_code)] +fn compute_scripted_caller_source_position() -> SourcePosition { + let scripted_caller = + unsafe { describe_scripted_caller(*GlobalScope::get_cx()) }.unwrap_or_default(); + + SourcePosition { + source_file: scripted_caller.filename, + line_number: scripted_caller.line, + column_number: scripted_caller.col + 1, + } } impl GlobalCspReporting for GlobalScope { /// - #[allow(unsafe_code)] - fn report_csp_violations(&self, violations: Vec, element: Option<&Element>) { - let scripted_caller = - unsafe { describe_scripted_caller(*GlobalScope::get_cx()) }.unwrap_or_default(); + fn report_csp_violations( + &self, + violations: Vec, + element: Option<&Element>, + source_position: Option, + ) { + let source_position = + source_position.unwrap_or_else(compute_scripted_caller_source_position); for violation in violations { let (sample, resource) = match violation.resource { ViolationResource::Inline { sample } => (sample, "inline".to_owned()), - ViolationResource::Url(url) => (None, url.into()), + ViolationResource::Url(url) => (Some(String::new()), url.into()), ViolationResource::TrustedTypePolicy { sample } => { (Some(sample), "trusted-types-policy".to_owned()) }, @@ -227,9 +254,9 @@ impl GlobalCspReporting for GlobalScope { .effective_directive(violation.directive.name) .original_policy(violation.policy.to_string()) .report_only(violation.policy.disposition == PolicyDisposition::Report) - .source_file(scripted_caller.filename.clone()) - .line_number(scripted_caller.line) - .column_number(scripted_caller.col + 1) + .source_file(source_position.source_file.clone()) + .line_number(source_position.line_number) + .column_number(source_position.column_number) .build(self); // Step 1: Let global be violation’s global object. // We use `self` as `global`; diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 9f001003ed9..374ea1a7197 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -100,7 +100,7 @@ use crate::dom::bindings::str::{DOMString, USVString}; use crate::dom::bindings::xmlname::matches_name_production; use crate::dom::characterdata::CharacterData; use crate::dom::create::create_element; -use crate::dom::csp::{CspReporting, InlineCheckType}; +use crate::dom::csp::{CspReporting, InlineCheckType, SourcePosition}; use crate::dom::customelementregistry::{ CallbackReaction, CustomElementDefinition, CustomElementReaction, CustomElementState, is_valid_custom_element_name, @@ -2663,6 +2663,14 @@ impl Element { TrustedHTMLOrNullIsEmptyString::TrustedHTML(_) => unreachable!(), } } + + pub(crate) fn compute_source_position(&self, line_number: u32) -> SourcePosition { + SourcePosition { + source_file: self.owner_global().get_url().to_string(), + line_number: line_number + 2, + column_number: 0, + } + } } #[allow(non_snake_case)] diff --git a/components/script/dom/eventsource.rs b/components/script/dom/eventsource.rs index 465b80c17b7..d0c84084b72 100644 --- a/components/script/dom/eventsource.rs +++ b/components/script/dom/eventsource.rs @@ -476,7 +476,7 @@ impl FetchResponseListener for EventSourceContext { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); } } diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs index a9190096c6f..427c9ed9401 100644 --- a/components/script/dom/htmlimageelement.rs +++ b/components/script/dom/htmlimageelement.rs @@ -165,6 +165,8 @@ pub(crate) struct HTMLImageElement { last_selected_source: DomRefCell>, #[ignore_malloc_size_of = "promises are hard"] image_decode_promises: DomRefCell>>, + /// Line number this element was created on + line_number: u64, } impl HTMLImageElement { @@ -237,6 +239,7 @@ struct ImageContext { /// timing data for this resource resource_timing: ResourceFetchTiming, url: ServoUrl, + element: Trusted, } impl FetchResponseListener for ImageContext { @@ -324,7 +327,11 @@ impl FetchResponseListener for ImageContext { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); - global.report_csp_violations(violations, None); + let elem = self.element.root(); + let source_position = elem + .upcast::() + .compute_source_position(elem.line_number as u32); + global.report_csp_violations(violations, None, Some(source_position)); } } @@ -444,6 +451,7 @@ impl HTMLImageElement { id, aborted: false, doc: Trusted::new(&document), + element: Trusted::new(self), resource_timing: ResourceFetchTiming::new(ResourceTimingType::Resource), url: img_url.clone(), }; @@ -1312,6 +1320,7 @@ impl HTMLImageElement { local_name: LocalName, prefix: Option, document: &Document, + creator: ElementCreator, ) -> HTMLImageElement { HTMLImageElement { htmlelement: HTMLElement::new_inherited(local_name, prefix, document), @@ -1341,6 +1350,7 @@ impl HTMLImageElement { source_set: DomRefCell::new(SourceSet::new()), last_selected_source: DomRefCell::new(None), image_decode_promises: DomRefCell::new(vec![]), + line_number: creator.return_line_number(), } } @@ -1350,11 +1360,12 @@ impl HTMLImageElement { prefix: Option, document: &Document, proto: Option, + creator: ElementCreator, can_gc: CanGc, ) -> DomRoot { Node::reflect_node_with_proto( Box::new(HTMLImageElement::new_inherited( - local_name, prefix, document, + local_name, prefix, document, creator, )), document, proto, diff --git a/components/script/dom/htmllinkelement.rs b/components/script/dom/htmllinkelement.rs index fa22372460d..2a2f94b80e7 100644 --- a/components/script/dom/htmllinkelement.rs +++ b/components/script/dom/htmllinkelement.rs @@ -121,6 +121,8 @@ pub(crate) struct HTMLLinkElement { previous_type_matched: Cell, /// Whether the previous media environment matched with the media query previous_media_environment_matched: Cell, + /// Line number this element was created on + line_number: u64, } impl HTMLLinkElement { @@ -143,6 +145,7 @@ impl HTMLLinkElement { is_explicitly_enabled: Cell::new(false), previous_type_matched: Cell::new(true), previous_media_environment_matched: Cell::new(true), + line_number: creator.return_line_number(), } } @@ -1018,7 +1021,11 @@ impl FetchResponseListener for PrefetchContext { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); - global.report_csp_violations(violations, None); + let link = self.link.root(); + let source_position = link + .upcast::() + .compute_source_position(link.line_number as u32); + global.report_csp_violations(violations, None, Some(source_position)); } } @@ -1092,7 +1099,11 @@ impl FetchResponseListener for PreloadContext { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); - global.report_csp_violations(violations, None); + let link = self.link.root(); + let source_position = link + .upcast::() + .compute_source_position(link.line_number as u32); + global.report_csp_violations(violations, None, Some(source_position)); } } diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index e8dfabec367..446e1b001d8 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -3159,7 +3159,7 @@ impl FetchResponseListener for HTMLMediaElementFetchListener { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); } } diff --git a/components/script/dom/htmlscriptelement.rs b/components/script/dom/htmlscriptelement.rs index 9a1dc3535e8..b35f35c4628 100644 --- a/components/script/dom/htmlscriptelement.rs +++ b/components/script/dom/htmlscriptelement.rs @@ -560,7 +560,10 @@ impl FetchResponseListener for ClassicContext { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); let elem = self.elem.root(); - global.report_csp_violations(violations, Some(elem.upcast::())); + let source_position = elem + .upcast::() + .compute_source_position(elem.line_number as u32); + global.report_csp_violations(violations, Some(elem.upcast()), Some(source_position)); } } diff --git a/components/script/dom/htmlvideoelement.rs b/components/script/dom/htmlvideoelement.rs index 52d62a841f6..2d34432cb8f 100644 --- a/components/script/dom/htmlvideoelement.rs +++ b/components/script/dom/htmlvideoelement.rs @@ -465,7 +465,7 @@ impl FetchResponseListener for PosterFrameFetchContext { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); } } diff --git a/components/script/dom/notification.rs b/components/script/dom/notification.rs index f60f74d3a53..18722c72898 100644 --- a/components/script/dom/notification.rs +++ b/components/script/dom/notification.rs @@ -794,7 +794,7 @@ impl FetchResponseListener for ResourceFetchListener { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); } } diff --git a/components/script/dom/servoparser/mod.rs b/components/script/dom/servoparser/mod.rs index 0cca83442c8..23aa9c06512 100644 --- a/components/script/dom/servoparser/mod.rs +++ b/components/script/dom/servoparser/mod.rs @@ -961,7 +961,14 @@ impl FetchResponseListener for ParserContext { let doc = &parser.document; let doc_body = DomRoot::upcast::(doc.GetBody().unwrap()); - let img = HTMLImageElement::new(local_name!("img"), None, doc, None, CanGc::note()); + let img = HTMLImageElement::new( + local_name!("img"), + None, + doc, + None, + ElementCreator::ParserCreated(1), + CanGc::note(), + ); img.SetSrc(USVString(self.url.to_string())); doc_body .AppendChild(&DomRoot::upcast::(img), CanGc::note()) @@ -1128,7 +1135,7 @@ impl FetchResponseListener for ParserContext { let document = &parser.document; let global = &document.global(); // TODO(https://github.com/w3c/webappsec-csp/issues/687): Update after spec is resolved - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); } } diff --git a/components/script/dom/websocket.rs b/components/script/dom/websocket.rs index ee5569fd6d7..9466b756fea 100644 --- a/components/script/dom/websocket.rs +++ b/components/script/dom/websocket.rs @@ -471,7 +471,7 @@ struct ReportCSPViolationTask { impl TaskOnce for ReportCSPViolationTask { fn run_once(self) { let global = self.websocket.root().global(); - global.report_csp_violations(self.violations, None); + global.report_csp_violations(self.violations, None, None); } } diff --git a/components/script/dom/xmlhttprequest.rs b/components/script/dom/xmlhttprequest.rs index c52967f4717..05b839c5cea 100644 --- a/components/script/dom/xmlhttprequest.rs +++ b/components/script/dom/xmlhttprequest.rs @@ -149,7 +149,7 @@ impl FetchResponseListener for XHRContext { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); } } diff --git a/components/script/fetch.rs b/components/script/fetch.rs index f3a69d3021c..bf546494084 100644 --- a/components/script/fetch.rs +++ b/components/script/fetch.rs @@ -313,7 +313,7 @@ impl FetchResponseListener for FetchContext { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); } } diff --git a/components/script/layout_image.rs b/components/script/layout_image.rs index b33c9f890df..a16d730eb4a 100644 --- a/components/script/layout_image.rs +++ b/components/script/layout_image.rs @@ -81,7 +81,7 @@ impl FetchResponseListener for LayoutImageContext { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); } } diff --git a/components/script/script_module.rs b/components/script/script_module.rs index 2de40a8084c..0c83195a2f9 100644 --- a/components/script/script_module.rs +++ b/components/script/script_module.rs @@ -1369,7 +1369,7 @@ impl FetchResponseListener for ModuleContext { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); } } diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 49b135c5eb9..dae86b924af 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -3789,7 +3789,7 @@ impl ScriptThread { fn handle_csp_violations(&self, id: PipelineId, _: RequestId, violations: Vec) { if let Some(global) = self.documents.borrow().find_global(id) { // TODO(https://github.com/w3c/webappsec-csp/issues/687): Update after spec is resolved - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); } } diff --git a/components/script/security_manager.rs b/components/script/security_manager.rs index 57805aa5d34..62ed054d33b 100644 --- a/components/script/security_manager.rs +++ b/components/script/security_manager.rs @@ -226,7 +226,7 @@ impl FetchResponseListener for CSPReportUriFetchListener { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); } } diff --git a/components/script/stylesheet_loader.rs b/components/script/stylesheet_loader.rs index f73677401e4..9d49d9a0247 100644 --- a/components/script/stylesheet_loader.rs +++ b/components/script/stylesheet_loader.rs @@ -298,7 +298,7 @@ impl FetchResponseListener for StylesheetContext { fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec) { let global = &self.resource_timing_global(); - global.report_csp_violations(violations, None); + global.report_csp_violations(violations, None, None); } } diff --git a/tests/wpt/meta/content-security-policy/reporting-api/report-to-directive-allowed-in-meta.https.sub.html.ini b/tests/wpt/meta/content-security-policy/reporting-api/report-to-directive-allowed-in-meta.https.sub.html.ini deleted file mode 100644 index b6beb950673..00000000000 --- a/tests/wpt/meta/content-security-policy/reporting-api/report-to-directive-allowed-in-meta.https.sub.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[report-to-directive-allowed-in-meta.https.sub.html] - [Report is observable to ReportingObserver] - expected: FAIL diff --git a/tests/wpt/meta/content-security-policy/reporting-api/reporting-api-sends-reports-on-violation.https.sub.html.ini b/tests/wpt/meta/content-security-policy/reporting-api/reporting-api-sends-reports-on-violation.https.sub.html.ini deleted file mode 100644 index 7ccf93969ba..00000000000 --- a/tests/wpt/meta/content-security-policy/reporting-api/reporting-api-sends-reports-on-violation.https.sub.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[reporting-api-sends-reports-on-violation.https.sub.html] - [Report is observable to ReportingObserver] - expected: FAIL