Fix reporting when only the report-only CSP header is present (#38002)

This was a bit confusing at first, but the report-only only
had an effect if it was used in conjunction with the regular
CSP header. This is incorrect, as the report-only header
can be present on its own.

Additionally, there was double-logic for parsing the CSP list
values, since we can only concatenate CSP lists if we have
an initial value, which requires a concrete policy value.

Therefore, abstract that way by looping over both headers and
handling the case where initially it is `None` and, if the
CSP header is not present, still `None` when we parse
the `report-only` header.

Additionally, update a WPT test. It was expecting the image
to load, yet was showing the fail image.

Part of #4577

Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>

components/script/dom/csp.rs

@@ -13,7 +13,7 @@ use content_security_policy::{
     CheckResult, CspList, Destination, Element as CspElement, Initiator, NavigationCheckType,
     Origin, ParserMetadata, PolicyDisposition, PolicySource, Request, ViolationResource,
 };
-use http::HeaderMap;
+use http::header::{HeaderMap, HeaderValue, ValueIter};
 use hyper_serde::Serde;
 use js::rust::describe_scripted_caller;
 
@@ -233,6 +233,9 @@ impl GlobalCspReporting for GlobalScope {
         element: Option<&Element>,
         source_position: Option<SourcePosition>,
     ) {
+        if violations.is_empty() {
+            return;
+        }
         let source_position =
             source_position.unwrap_or_else(compute_scripted_caller_source_position);
         for violation in violations {
@@ -297,35 +300,43 @@ impl GlobalCspReporting for GlobalScope {
     }
 }
 
-/// <https://www.w3.org/TR/CSP/#initialize-document-csp>
+fn parse_and_potentially_append_to_csp_list(
+    old_csp_list: Option<CspList>,
+    csp_header_iter: ValueIter<HeaderValue>,
+    disposition: PolicyDisposition,
+) -> Option<CspList> {
+    let mut csp_list = old_csp_list;
+    for header in csp_header_iter {
+        // This silently ignores the CSP if it contains invalid Unicode.
+        // We should probably report an error somewhere.
+        let new_csp_list = header
+            .to_str()
+            .ok()
+            .map(|value| CspList::parse(value, PolicySource::Header, disposition));
+        if let Some(new_csp_list_value) = new_csp_list {
+            match csp_list {
+                None => csp_list = Some(new_csp_list_value),
+                Some(ref mut csp_list) => csp_list.append(new_csp_list_value),
+            };
+        }
+    }
+    csp_list
+}
+
+/// <https://www.w3.org/TR/CSP/#parse-response-csp>
 pub(crate) fn parse_csp_list_from_metadata(headers: &Option<Serde<HeaderMap>>) -> Option<CspList> {
-    // TODO: Implement step 1 (local scheme special case)
     let headers = headers.as_ref()?;
-    let mut csp = headers.get_all("content-security-policy").iter();
+    let csp_enforce_list = parse_and_potentially_append_to_csp_list(
-    // This silently ignores the CSP if it contains invalid Unicode.
+        None,
-    // We should probably report an error somewhere.
+        headers.get_all("content-security-policy").iter(),
-    let c = csp.next().and_then(|c| c.to_str().ok())?;
-    let mut csp_list = CspList::parse(c, PolicySource::Header, PolicyDisposition::Enforce);
-    for c in csp {
-        let c = c.to_str().ok()?;
-        csp_list.append(CspList::parse(
-            c,
-            PolicySource::Header,
         PolicyDisposition::Enforce,
-        ));
+    );
-    }
+
-    let csp_report = headers
+    parse_and_potentially_append_to_csp_list(
+        csp_enforce_list,
+        headers
             .get_all("content-security-policy-report-only")
-        .iter();
+            .iter(),
-    // This silently ignores the CSP if it contains invalid Unicode.
-    // We should probably report an error somewhere.
-    for c in csp_report {
-        let c = c.to_str().ok()?;
-        csp_list.append(CspList::parse(
-            c,
-            PolicySource::Header,
         PolicyDisposition::Report,
-        ));
+    )
-    }
-    Some(csp_list)
 }

tests/wpt/meta/MANIFEST.json

@@ -577733,7 +577733,7 @@
       ]
      ],
      "reporting-api-report-only-sends-reports-on-violation.https.sub.html": [
-      "302025669d4417db670b6ebba18a6a49aed4e2eb",
+      "89e7357fbb9022cc9d068054151a9bf87d846276",
       [
        null,
        {}

tests/wpt/meta/content-security-policy/inside-worker/dedicatedworker-report-only.html.ini

@@ -1,16 +1,7 @@
 [dedicatedworker-report-only.html]
   expected: TIMEOUT
-  [Cross-origin 'fetch()'.]
-    expected: TIMEOUT
-
-  [Cross-origin XHR.]
-    expected: NOTRUN
-
-  [Same-origin => cross-origin 'fetch()'.]
-    expected: NOTRUN
-
   [WebSocket.]
-    expected: NOTRUN
+    expected: TIMEOUT
 
   [connect-src-self-report-only]
     expected: NOTRUN

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

@@ -1,7 +0,0 @@
-[reporting-api-report-only-sends-reports-on-violation.https.sub.html]
-  expected: TIMEOUT
-  [Event is fired]
-    expected: TIMEOUT
-
-  [Violation report status OK.]
-    expected: FAIL

tests/wpt/meta/content-security-policy/reporting/multiple-report-policies.html.ini

@@ -1,6 +0,0 @@
-[multiple-report-policies.html]
-  [2-Violation report status OK]
-    expected: FAIL
-
-  [1-Violation report status OK]
-    expected: FAIL

tests/wpt/meta/content-security-policy/reporting/report-multiple-violations-01.html.ini

@@ -1,6 +1,3 @@
 [report-multiple-violations-01.html]
-  [Violation report status OK.]
-    expected: FAIL
-
   [Test number of sent reports.]
     expected: FAIL

tests/wpt/meta/content-security-policy/reporting/report-multiple-violations-02.html.ini

@@ -1,6 +1,3 @@
 [report-multiple-violations-02.html]
-  [Violation report status OK.]
-    expected: FAIL
-
   [Test number of sent reports.]
     expected: FAIL

tests/wpt/meta/content-security-policy/reporting/report-only-unsafe-eval.html.ini

@@ -1,6 +0,0 @@
-[report-only-unsafe-eval.html]
-  [SPV event is still raised]
-    expected: FAIL
-
-  [Violation report status OK.]
-    expected: FAIL

tests/wpt/meta/content-security-policy/script-src/eval-allowed-in-report-only-mode-and-sends-report.html.ini

@@ -1,3 +0,0 @@
-[eval-allowed-in-report-only-mode-and-sends-report.html]
-  [Violation report status OK.]
-    expected: FAIL

tests/wpt/meta/trusted-types/default-policy-report-only.html.ini

@@ -1,31 +0,0 @@
-[default-policy-report-only.html]
-  expected: TIMEOUT
-  [Count SecurityPolicyViolation events.]
-    expected: TIMEOUT
-
-  [script.src default]
-    expected: FAIL
-
-  [script.src throw]
-    expected: FAIL
-
-  [script.src typeerror]
-    expected: FAIL
-
-  [div.innerHTML default]
-    expected: FAIL
-
-  [div.innerHTML throw]
-    expected: FAIL
-
-  [div.innerHTML typeerror]
-    expected: FAIL
-
-  [script.text default]
-    expected: FAIL
-
-  [script.text throw]
-    expected: FAIL
-
-  [script.text typeerror]
-    expected: FAIL

tests/wpt/meta/trusted-types/empty-default-policy-report-only.html.ini

@@ -1,4 +0,0 @@
-[empty-default-policy-report-only.html]
-  expected: TIMEOUT
-  [Count SecurityPolicyViolation events.]
-    expected: TIMEOUT

tests/wpt/meta/trusted-types/should-trusted-type-policy-creation-be-blocked-by-csp-002.html.ini

@@ -18,8 +18,5 @@
   [invalid directive "trusted-type _TTP" (no ascii whitespace)]
     expected: NOTRUN
 
-  [non-ASCII trusted-types directives are discarded ("política")]
-    expected: TIMEOUT
-
   [directive "trusted-type _TTP1_%09_TTP2_%0A%20_TTP3_%0C_TTP4_%0D_TTP5_%20_TTP6_" (required-ascii-whitespace)]
-    expected: NOTRUN
+    expected: TIMEOUT

tests/wpt/meta/trusted-types/trusted-types-reporting-check-report-DedicatedWorker-create-policy.html.ini

@@ -1,6 +0,0 @@
-[trusted-types-reporting-check-report-DedicatedWorker-create-policy.html]
-  [Test report-uri works with trusted-types violation.]
-    expected: FAIL
-
-  [Test number of sent reports.]
-    expected: FAIL

tests/wpt/meta/trusted-types/trusted-types-reporting-check-report-Window-create-policy.html.ini

@@ -1,6 +0,0 @@
-[trusted-types-reporting-check-report-Window-create-policy.html]
-  [Test report-uri works with trusted-types violation.]
-    expected: FAIL
-
-  [Test number of sent reports.]
-    expected: FAIL

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

@@ -7,16 +7,16 @@
 </head>
 <body>
   <script>
-    var t1 = async_test("Test that image does not load");
+    var t1 = async_test("Test that image does load");
     async_test(function(t2) {
     window.addEventListener("securitypolicyviolation", t2.step_func(function(e) {
-        assert_equals(e.blockedURI, "{{location[scheme]}}://{{location[host]}}/content-security-policy/support/fail.png");
+        assert_equals(e.blockedURI, "{{location[scheme]}}://{{location[host]}}/content-security-policy/support/pass.png");
         assert_equals(e.violatedDirective, "img-src");
         t2.done();
       }));
     }, "Event is fired");
   </script>
-  <img src='/content-security-policy/support/fail.png'
+  <img src='/content-security-policy/support/pass.png'
        onload='t1.done();'
        onerror='t1.unreached_func("The image should have loaded");'>