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 <tvanderlippe@gmail.com>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>

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

components/script/dom/csp.rs

@@ -72,7 +72,7 @@ impl CspReporting for Option<CspList> {
 
         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<CspList> {
 
         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<CspList> {
         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<CspList> {
         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<CspList> {
         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<CspList> {
         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<Violation>, element: Option<&Element>);
+    fn report_csp_violations(
+        &self,
+        violations: Vec<Violation>,
+        element: Option<&Element>,
+        source_position: Option<SourcePosition>,
+    );
+}
+
+#[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 {
     /// <https://www.w3.org/TR/CSP/#report-violation>
-    #[allow(unsafe_code)]
+    fn report_csp_violations(
-    fn report_csp_violations(&self, violations: Vec<Violation>, element: Option<&Element>) {
+        &self,
-        let scripted_caller =
+        violations: Vec<Violation>,
-            unsafe { describe_scripted_caller(*GlobalScope::get_cx()) }.unwrap_or_default();
+        element: Option<&Element>,
+        source_position: Option<SourcePosition>,
+    ) {
+        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())
+                .source_file(source_position.source_file.clone())
-                .line_number(scripted_caller.line)
+                .line_number(source_position.line_number)
-                .column_number(scripted_caller.col + 1)
+                .column_number(source_position.column_number)
                 .build(self);
             // Step 1: Let global be violation’s global object.
             // We use `self` as `global`;

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)]

components/script/dom/eventsource.rs

@@ -476,7 +476,7 @@ impl FetchResponseListener for EventSourceContext {
 
     fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec<Violation>) {
         let global = &self.resource_timing_global();
-        global.report_csp_violations(violations, None);
+        global.report_csp_violations(violations, None, None);
     }
 }
 

components/script/dom/htmlimageelement.rs

@@ -165,6 +165,8 @@ pub(crate) struct HTMLImageElement {
     last_selected_source: DomRefCell<Option<USVString>>,
     #[ignore_malloc_size_of = "promises are hard"]
     image_decode_promises: DomRefCell<Vec<Rc<Promise>>>,
+    /// 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<HTMLImageElement>,
 }
 
 impl FetchResponseListener for ImageContext {
@@ -324,7 +327,11 @@ impl FetchResponseListener for ImageContext {
 
     fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec<Violation>) {
         let global = &self.resource_timing_global();
-        global.report_csp_violations(violations, None);
+        let elem = self.element.root();
+        let source_position = elem
+            .upcast::<Element>()
+            .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<Prefix>,
         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<Prefix>,
         document: &Document,
         proto: Option<HandleObject>,
+        creator: ElementCreator,
         can_gc: CanGc,
     ) -> DomRoot<HTMLImageElement> {
         Node::reflect_node_with_proto(
             Box::new(HTMLImageElement::new_inherited(
-                local_name, prefix, document,
+                local_name, prefix, document, creator,
             )),
             document,
             proto,

components/script/dom/htmllinkelement.rs

@@ -121,6 +121,8 @@ pub(crate) struct HTMLLinkElement {
     previous_type_matched: Cell<bool>,
     /// Whether the previous media environment matched with the media query
     previous_media_environment_matched: Cell<bool>,
+    /// 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<Violation>) {
         let global = &self.resource_timing_global();
-        global.report_csp_violations(violations, None);
+        let link = self.link.root();
+        let source_position = link
+            .upcast::<Element>()
+            .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<Violation>) {
         let global = &self.resource_timing_global();
-        global.report_csp_violations(violations, None);
+        let link = self.link.root();
+        let source_position = link
+            .upcast::<Element>()
+            .compute_source_position(link.line_number as u32);
+        global.report_csp_violations(violations, None, Some(source_position));
     }
 }
 

components/script/dom/htmlmediaelement.rs

@@ -3159,7 +3159,7 @@ impl FetchResponseListener for HTMLMediaElementFetchListener {
 
     fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec<Violation>) {
         let global = &self.resource_timing_global();
-        global.report_csp_violations(violations, None);
+        global.report_csp_violations(violations, None, None);
     }
 }
 

components/script/dom/htmlscriptelement.rs

@@ -560,7 +560,10 @@ impl FetchResponseListener for ClassicContext {
     fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec<Violation>) {
         let global = &self.resource_timing_global();
         let elem = self.elem.root();
-        global.report_csp_violations(violations, Some(elem.upcast::<Element>()));
+        let source_position = elem
+            .upcast::<Element>()
+            .compute_source_position(elem.line_number as u32);
+        global.report_csp_violations(violations, Some(elem.upcast()), Some(source_position));
     }
 }
 

components/script/dom/htmlvideoelement.rs

@@ -465,7 +465,7 @@ impl FetchResponseListener for PosterFrameFetchContext {
 
     fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec<Violation>) {
         let global = &self.resource_timing_global();
-        global.report_csp_violations(violations, None);
+        global.report_csp_violations(violations, None, None);
     }
 }
 

components/script/dom/notification.rs

@@ -794,7 +794,7 @@ impl FetchResponseListener for ResourceFetchListener {
 
     fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec<Violation>) {
         let global = &self.resource_timing_global();
-        global.report_csp_violations(violations, None);
+        global.report_csp_violations(violations, None, None);
     }
 }
 

components/script/dom/servoparser/mod.rs

@@ -961,7 +961,14 @@ impl FetchResponseListener for ParserContext {
 
                 let doc = &parser.document;
                 let doc_body = DomRoot::upcast::<Node>(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::<Node>(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);
     }
 }
 

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);
     }
 }
 

components/script/dom/xmlhttprequest.rs

@@ -149,7 +149,7 @@ impl FetchResponseListener for XHRContext {
 
     fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec<Violation>) {
         let global = &self.resource_timing_global();
-        global.report_csp_violations(violations, None);
+        global.report_csp_violations(violations, None, None);
     }
 }
 

components/script/fetch.rs

@@ -313,7 +313,7 @@ impl FetchResponseListener for FetchContext {
 
     fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec<Violation>) {
         let global = &self.resource_timing_global();
-        global.report_csp_violations(violations, None);
+        global.report_csp_violations(violations, None, None);
     }
 }
 

components/script/layout_image.rs

@@ -81,7 +81,7 @@ impl FetchResponseListener for LayoutImageContext {
 
     fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec<Violation>) {
         let global = &self.resource_timing_global();
-        global.report_csp_violations(violations, None);
+        global.report_csp_violations(violations, None, None);
     }
 }
 

components/script/script_module.rs

@@ -1369,7 +1369,7 @@ impl FetchResponseListener for ModuleContext {
 
     fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec<Violation>) {
         let global = &self.resource_timing_global();
-        global.report_csp_violations(violations, None);
+        global.report_csp_violations(violations, None, None);
     }
 }
 

components/script/script_thread.rs

@@ -3789,7 +3789,7 @@ impl ScriptThread {
     fn handle_csp_violations(&self, id: PipelineId, _: RequestId, violations: Vec<Violation>) {
         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);
         }
     }
 

components/script/security_manager.rs

@@ -226,7 +226,7 @@ impl FetchResponseListener for CSPReportUriFetchListener {
 
     fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec<Violation>) {
         let global = &self.resource_timing_global();
-        global.report_csp_violations(violations, None);
+        global.report_csp_violations(violations, None, None);
     }
 }
 

components/script/stylesheet_loader.rs

@@ -298,7 +298,7 @@ impl FetchResponseListener for StylesheetContext {
 
     fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec<Violation>) {
         let global = &self.resource_timing_global();
-        global.report_csp_violations(violations, None);
+        global.report_csp_violations(violations, None, None);
     }
 }
 

tests/wpt/meta/content-security-policy/reporting-api/report-to-directive-allowed-in-meta.https.sub.html.ini

@@ -1,3 +0,0 @@
-[report-to-directive-allowed-in-meta.https.sub.html]
-  [Report is observable to ReportingObserver]
-    expected: FAIL

tests/wpt/meta/content-security-policy/reporting-api/reporting-api-sends-reports-on-violation.https.sub.html.ini

@@ -1,3 +0,0 @@
-[reporting-api-sends-reports-on-violation.https.sub.html]
-  [Report is observable to ReportingObserver]
-    expected: FAIL