Report DevTools issues for invalid attribution source headers

For now, the issue only contains the generic InvalidHeader label and the
invalid header value, but in the future we intend to provide richer
information about what exactly was invalid.

We use a single InvalidHeader label, rather than one per header, so that
the issues can be collapsed in the UI more easily.

Bug: 1302318
Change-Id: Ifd566dfa474f8cb7398a672f52a1a7e10ab593bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3629640
Reviewed-by: John Delaney <johnidel@chromium.org>
Quick-Run: Andrew Paseltiner <apaseltiner@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001772}

third_party/blink/public/devtools_protocol/browser_protocol.pdl

@@ -699,6 +699,7 @@ experimental domain Audits
       PermissionPolicyDisabled
       AttributionSourceUntrustworthyOrigin
       AttributionUntrustworthyOrigin
+      InvalidHeader
 
   # Details for issues around "Attribution Reporting API" usage.
   # Explainer: https://github.com/WICG/conversion-measurement-api

third_party/blink/renderer/core/frame/attribution_src_loader.cc

@@ -121,8 +121,10 @@ class AttributionSrcLoader::ResourceClient
   }
 
  private:
-  void HandleResponseHeaders(const ResourceResponse& response);
-  void HandleSourceRegistration(const ResourceResponse& response);
+  void HandleResponseHeaders(const ResourceResponse& response,
+                             uint64_t request_id);
+  void HandleSourceRegistration(const ResourceResponse& response,
+                                uint64_t request_id);
   void HandleTriggerRegistration(const ResourceResponse& response);
 
   // RawResourceClient:
@@ -282,7 +284,7 @@ AttributionSrcLoader::CanRegisterAttribution(
     RegisterContext context,
     const KURL& url,
     HTMLElement* element,
-    const absl::optional<String>& request_id) {
+    absl::optional<uint64_t> request_id) {
   LocalDOMWindow* window = local_frame_->DomWindow();
   DCHECK(window);
 
@@ -343,10 +345,9 @@ void AttributionSrcLoader::MaybeRegisterTrigger(
   if (!ContainsTriggerHeaders(response.HttpHeaderFields()))
     return;
 
-  if (CanRegisterAttribution(
-          RegisterContext::kResourceTrigger, response.CurrentRequestUrl(),
-          /*element=*/nullptr,
-          IdentifiersFactory::SubresourceRequestId(request.InspectorId())) !=
+  if (CanRegisterAttribution(RegisterContext::kResourceTrigger,
+                             response.CurrentRequestUrl(),
+                             /*element=*/nullptr, request.InspectorId()) !=
       RegisterResult::kSuccess) {
     return;
   }
@@ -389,14 +390,14 @@ String AttributionSrcLoader::ResourceClient::DebugName() const {
 void AttributionSrcLoader::ResourceClient::ResponseReceived(
     Resource* resource,
     const ResourceResponse& response) {
-  HandleResponseHeaders(response);
+  HandleResponseHeaders(response, resource->InspectorId());
 }
 
 bool AttributionSrcLoader::ResourceClient::RedirectReceived(
     Resource* resource,
     const ResourceRequest& request,
     const ResourceResponse& response) {
-  HandleResponseHeaders(response);
+  HandleResponseHeaders(response, request.InspectorId());
   return true;
 }
 
@@ -416,15 +417,16 @@ void AttributionSrcLoader::ResourceClient::NotifyFinished(Resource* resource) {
 }
 
 void AttributionSrcLoader::ResourceClient::HandleResponseHeaders(
-    const ResourceResponse& response) {
+    const ResourceResponse& response,
+    uint64_t request_id) {
   const auto& headers = response.HttpHeaderFields();
 
   bool can_process_source =
       type_ == SrcType::kUndetermined || type_ == SrcType::kSource;
   if (can_process_source &&
       headers.Contains(http_names::kAttributionReportingRegisterSource)) {
     type_ = SrcType::kSource;
-    HandleSourceRegistration(response);
+    HandleSourceRegistration(response, request_id);
     return;
   }
 
@@ -439,7 +441,8 @@ void AttributionSrcLoader::ResourceClient::HandleResponseHeaders(
 }
 
 void AttributionSrcLoader::ResourceClient::HandleSourceRegistration(
-    const ResourceResponse& response) {
+    const ResourceResponse& response,
+    uint64_t request_id) {
   DCHECK_EQ(type_, SrcType::kSource);
 
   mojom::blink::AttributionSourceDataPtr source_data =
@@ -452,10 +455,14 @@ void AttributionSrcLoader::ResourceClient::HandleSourceRegistration(
     return;
   source_data->reporting_origin = std::move(reporting_origin);
 
+  const AtomicString& source_json =
+      response.HttpHeaderField(http_names::kAttributionReportingRegisterSource);
+
   if (!attribution_response_parsing::ParseSourceRegistrationHeader(
-          response.HttpHeaderField(
-              http_names::kAttributionReportingRegisterSource),
-          *source_data)) {
+          source_json, *source_data)) {
+    loader_->LogAuditIssue(AttributionReportingIssueType::kInvalidHeader,
+                           source_json,
+                           /*element=*/nullptr, request_id);
     return;
   }
 
@@ -467,6 +474,9 @@ void AttributionSrcLoader::ResourceClient::HandleSourceRegistration(
   if (!aggregatable_source_json.IsNull() &&
       !attribution_response_parsing::ParseAttributionAggregatableSource(
           aggregatable_source_json, *source_data->aggregatable_source)) {
+    loader_->LogAuditIssue(AttributionReportingIssueType::kInvalidHeader,
+                           aggregatable_source_json,
+                           /*element=*/nullptr, request_id);
     return;
   }
 
@@ -477,6 +487,7 @@ void AttributionSrcLoader::ResourceClient::HandleTriggerRegistration(
     const ResourceResponse& response) {
   DCHECK_EQ(type_, SrcType::kTrigger);
 
+  // TODO(apaseltiner): Report DevTools issue(s) if this fails.
   mojom::blink::AttributionTriggerDataPtr trigger_data =
       attribution_response_parsing::ParseAttributionTriggerData(response);
 
@@ -490,12 +501,17 @@ void AttributionSrcLoader::LogAuditIssue(
     AttributionReportingIssueType issue_type,
     const absl::optional<String>& string,
     HTMLElement* element,
-    const absl::optional<String>& request_id) {
+    absl::optional<uint64_t> request_id) {
   if (!local_frame_->IsAttached())
     return;
+
+  absl::optional<String> id_string;
+  if (request_id)
+    id_string = IdentifiersFactory::SubresourceRequestId(*request_id);
+
   AuditsIssue::ReportAttributionIssue(local_frame_->DomWindow(), issue_type,
                                       local_frame_->GetDevToolsFrameToken(),
-                                      element, request_id, string);
+                                      element, id_string, string);
 }
 
 }  // namespace blink

third_party/blink/renderer/core/frame/attribution_src_loader.h

@@ -6,6 +6,7 @@
 #define THIRD_PARTY_BLINK_RENDERER_CORE_FRAME_ATTRIBUTION_SRC_LOADER_H_
 
 #include <stddef.h>
+#include <stdint.h>
 
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "third_party/blink/public/mojom/conversions/attribution_data_host.mojom-blink-forward.h"
@@ -87,11 +88,10 @@ class CORE_EXPORT AttributionSrcLoader
 
   // Returns whether the attribution is allowed to be registered. Devtool issue
   // might be reported if it's not allowed.
-  RegisterResult CanRegisterAttribution(
-      RegisterContext context,
-      const KURL& url,
-      HTMLElement* element,
-      const absl::optional<String>& request_id);
+  RegisterResult CanRegisterAttribution(RegisterContext context,
+                                        const KURL& url,
+                                        HTMLElement* element,
+                                        absl::optional<uint64_t> request_id);
 
   void RegisterTrigger(
       mojom::blink::AttributionTriggerDataPtr trigger_data) const;
@@ -105,7 +105,7 @@ class CORE_EXPORT AttributionSrcLoader
   void LogAuditIssue(AttributionReportingIssueType issue_type,
                      const absl::optional<String>& string,
                      HTMLElement* element,
-                     const absl::optional<String>& request_id);
+                     absl::optional<uint64_t> request_id);
 
   const Member<LocalFrame> local_frame_;
   size_t num_resource_clients_ = 0;

third_party/blink/renderer/core/inspector/inspector_audits_issue.cc

@@ -149,6 +149,8 @@ BuildAttributionReportingIssueType(AttributionReportingIssueType type) {
     case AttributionReportingIssueType::kAttributionUntrustworthyOrigin:
       return protocol::Audits::AttributionReportingIssueTypeEnum::
           AttributionUntrustworthyOrigin;
+    case AttributionReportingIssueType::kInvalidHeader:
+      return protocol::Audits::AttributionReportingIssueTypeEnum::InvalidHeader;
   }
 }
 

third_party/blink/renderer/core/inspector/inspector_audits_issue.h

@@ -105,6 +105,7 @@ enum class AttributionReportingIssueType {
   kPermissionPolicyDisabled,
   kAttributionSourceUntrustworthyOrigin,
   kAttributionUntrustworthyOrigin,
+  kInvalidHeader,
 };
 
 enum class SharedArrayBufferIssueType {

third_party/blink/web_tests/http/tests/inspector-protocol/conversion/invalid-aggregatable-source-header.js

@@ -0,0 +1,21 @@
+// Copyright 2022 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+(async function(testRunner) {
+  const {page, dp} = await testRunner.startBlank(
+      `Test that an attributionsrc response with an invalid Attribution-Reporting-Register-Aggregatable-Source header triggers an issue.`);
+
+  await dp.Audits.enable();
+  await page.navigate(
+      'https://devtools.test:8443/inspector-protocol/conversion/resources/impression.html');
+
+  await page.loadHTML(
+      `<img attributionsrc="https://devtools.test:8443/inspector-protocol/conversion/resources/register-invalid-aggregatable-source.php">`);
+
+  const issuePromise = dp.Audits.onceIssueAdded();
+  const issue = await issuePromise;
+  testRunner.log(
+      issue.params.issue, 'Issue reported: ', ['frame', 'request']);
+  testRunner.completeTest();
+})

third_party/blink/web_tests/http/tests/inspector-protocol/conversion/invalid-source-header.js

@@ -0,0 +1,21 @@
+// Copyright 2022 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+(async function(testRunner) {
+  const {page, dp} = await testRunner.startBlank(
+      `Test that an attributionsrc response with an invalid Attribution-Reporting-Register-Source header triggers an issue.`);
+
+  await dp.Audits.enable();
+  await page.navigate(
+      'https://devtools.test:8443/inspector-protocol/conversion/resources/impression.html');
+
+  await page.loadHTML(
+      `<img attributionsrc="https://devtools.test:8443/inspector-protocol/conversion/resources/register-invalid-source.php">`);
+
+  const issuePromise = dp.Audits.onceIssueAdded();
+  const issue = await issuePromise;
+  testRunner.log(
+      issue.params.issue, 'Issue reported: ', ['frame', 'request']);
+  testRunner.completeTest();
+})

third_party/blink/web_tests/http/tests/inspector-protocol/conversion/resources/register-invalid-aggregatable-source.php

@@ -0,0 +1,5 @@
+<?php
+// The event-level header must be valid for the aggregatable one to be parsed at all.
+header('Attribution-Reporting-Register-Source: {"source_event_id":"0","destination":"https://irrelevant.test"}');
+header('Attribution-Reporting-Register-Aggregatable-Source: @');
+?>

third_party/blink/web_tests/http/tests/inspector-protocol/conversion/resources/register-invalid-source.php

@@ -0,0 +1,3 @@
+<?php
+header('Attribution-Reporting-Register-Source: !');
+?>

third_party/blink/web_tests/platform/generic/http/tests/inspector-protocol/conversion/invalid-aggregatable-source-header-expected.txt

@@ -0,0 +1,13 @@
+Test that an attributionsrc response with an invalid Attribution-Reporting-Register-Aggregatable-Source header triggers an issue.
+Issue reported: {
+    code : AttributionReportingIssue
+    details : {
+        attributionReportingIssueDetails : {
+            frame : <object>
+            invalidParameter : @
+            request : <object>
+            violationType : InvalidHeader
+        }
+    }
+}
+

third_party/blink/web_tests/platform/generic/http/tests/inspector-protocol/conversion/invalid-source-header-expected.txt

@@ -0,0 +1,13 @@
+Test that an attributionsrc response with an invalid Attribution-Reporting-Register-Source header triggers an issue.
+Issue reported: {
+    code : AttributionReportingIssue
+    details : {
+        attributionReportingIssueDetails : {
+            frame : <object>
+            invalidParameter : !
+            request : <object>
+            violationType : InvalidHeader
+        }
+    }
+}
+