Skip to content

Commit

Permalink
Check Attrribution-Reporting-Eligible header with structured parser
Browse files Browse the repository at this point in the history 
Rather than string comparisons. The exact serialization of this header,
which contains a structured dictionary, is not guaranteed, and may be
subject to greasing in the future.

Bug: 1446382
Change-Id: I7ec7e12ae0e736df736d720c2f6c4d21f7e7514b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4544663
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Garrett Tanzer <gtanzer@chromium.org>
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1146534}
  • Loading branch information
Andrew Paseltiner authored and Chromium LUCI CQ committed May 19, 2023
1 parent b220bc6 commit 0e8a752
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 70 deletions.
24 changes: 12 additions & 12 deletions content/browser/attribution_reporting/attribution_src_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -566,9 +566,9 @@ IN_PROC_BROWSER_TEST_F(AttributionSrcBrowserTest,
JsReplace("createAttributionSrcImg($1);", register_url)));

register_response1->WaitForRequest();
ASSERT_EQ(register_response1->http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"event-source, trigger");
ExpectValidAttributionReportingEligibleHeaderForImg(
register_response1->http_request()->headers.at(
"Attribution-Reporting-Eligible"));
ASSERT_FALSE(base::Contains(register_response1->http_request()->headers,
"Attribution-Reporting-Support"));

Expand All @@ -580,9 +580,9 @@ IN_PROC_BROWSER_TEST_F(AttributionSrcBrowserTest,

// Ensure that redirect requests also contain the header.
register_response2->WaitForRequest();
ASSERT_EQ(register_response2->http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"event-source, trigger");
ExpectValidAttributionReportingEligibleHeaderForImg(
register_response2->http_request()->headers.at(
"Attribution-Reporting-Eligible"));
ASSERT_FALSE(base::Contains(register_response2->http_request()->headers,
"Attribution-Reporting-Support"));
}
Expand Down Expand Up @@ -1192,9 +1192,9 @@ IN_PROC_BROWSER_TEST_F(AttributionSrcCrossAppWebRuntimeDisabledBrowserTest,
JsReplace("createAttributionSrcImg($1);", register_url)));

register_response1->WaitForRequest();
ASSERT_EQ(register_response1->http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"event-source, trigger");
ExpectValidAttributionReportingEligibleHeaderForImg(
register_response1->http_request()->headers.at(
"Attribution-Reporting-Eligible"));
ASSERT_FALSE(base::Contains(register_response1->http_request()->headers,
"Attribution-Reporting-Support"));

Expand All @@ -1207,9 +1207,9 @@ IN_PROC_BROWSER_TEST_F(AttributionSrcCrossAppWebRuntimeDisabledBrowserTest,
// Ensure that redirect requests also don't contain the
// Attribution-Reporting-Support header.
register_response2->WaitForRequest();
ASSERT_EQ(register_response2->http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"event-source, trigger");
ExpectValidAttributionReportingEligibleHeaderForImg(
register_response2->http_request()->headers.at(
"Attribution-Reporting-Eligible"));
ASSERT_FALSE(base::Contains(register_response2->http_request()->headers,
"Attribution-Reporting-Support"));
}
Expand Down
49 changes: 49 additions & 0 deletions content/browser/attribution_reporting/attribution_test_utils.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@
#include "content/public/browser/attribution_data_model.h"
#include "net/base/net_errors.h"
#include "net/base/schemeful_site.h"
#include "net/http/structured_headers.h"
#include "services/network/public/cpp/trigger_verification.h"
#include "services/network/public/cpp/trigger_verification_test_utils.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/abseil-cpp/absl/numeric/int128.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "url/gurl.h"
Expand Down Expand Up @@ -1171,4 +1173,51 @@ std::ostream& operator<<(std::ostream& out, const OsRegistration& r) {
<< ",type=" << r.GetType() << "}";
}

namespace {

void CheckAttributionReportingEligibleHeader(
const std::string& header,
const std::vector<std::string>& required_keys,
const std::vector<std::string>& prohibited_keys) {
auto dict = net::structured_headers::ParseDictionary(header);
EXPECT_TRUE(dict.has_value());
if (!dict.has_value()) {
return;
}

for (const auto& key : required_keys) {
EXPECT_TRUE(dict->contains(key)) << key;
}

for (const auto& key : prohibited_keys) {
EXPECT_FALSE(dict->contains(key)) << key;
}
}

} // namespace

void ExpectValidAttributionReportingEligibleHeaderForEventBeacon(
const std::string& header) {
CheckAttributionReportingEligibleHeader(
header,
/*required_keys=*/{"event-source"},
/*prohibited_keys=*/{"navigation-source", "trigger"});
}

void ExpectValidAttributionReportingEligibleHeaderForImg(
const std::string& header) {
CheckAttributionReportingEligibleHeader(
header,
/*required_keys=*/{"event-source", "trigger"},
/*prohibited_keys=*/{"navigation-source"});
}

void ExpectValidAttributionReportingEligibleHeaderForNavigation(
const std::string& header) {
CheckAttributionReportingEligibleHeader(
header,
/*required_keys=*/{"navigation-source"},
/*prohibited_keys=*/{"event-source", "trigger"});
}

} // namespace content
7 changes: 7 additions & 0 deletions content/browser/attribution_reporting/attribution_test_utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,13 @@ bool operator==(const OsRegistration&, const OsRegistration&);

std::ostream& operator<<(std::ostream&, const OsRegistration&);

void ExpectValidAttributionReportingEligibleHeaderForEventBeacon(
const std::string& header);
void ExpectValidAttributionReportingEligibleHeaderForImg(
const std::string& header);
void ExpectValidAttributionReportingEligibleHeaderForNavigation(
const std::string& header);

} // namespace content

#endif // CONTENT_BROWSER_ATTRIBUTION_REPORTING_ATTRIBUTION_TEST_UTILS_H_
24 changes: 12 additions & 12 deletions content/browser/attribution_reporting/attributions_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -596,9 +596,9 @@ IN_PROC_BROWSER_TEST_F(AttributionsBrowserTest,

// Verify the navigation redirects contain the eligibility header.
register_response1->WaitForRequest();
EXPECT_EQ(register_response1->http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"navigation-source");
ExpectValidAttributionReportingEligibleHeaderForNavigation(
register_response1->http_request()->headers.at(
"Attribution-Reporting-Eligible"));
EXPECT_FALSE(base::Contains(register_response1->http_request()->headers,
"Attribution-Reporting-Support"));

Expand All @@ -610,9 +610,9 @@ IN_PROC_BROWSER_TEST_F(AttributionsBrowserTest,

// Ensure that redirect requests also contain the header.
register_response2->WaitForRequest();
ASSERT_EQ(register_response2->http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"navigation-source");
ExpectValidAttributionReportingEligibleHeaderForNavigation(
register_response2->http_request()->headers.at(
"Attribution-Reporting-Eligible"));
ASSERT_FALSE(base::Contains(register_response2->http_request()->headers,
"Attribution-Reporting-Support"));
}
Expand Down Expand Up @@ -1234,9 +1234,9 @@ IN_PROC_BROWSER_TEST_F(AttributionsCrossAppWebRuntimeDisabledBrowserTest,

// Verify the navigation redirects contain the eligibility header.
register_response1->WaitForRequest();
ASSERT_EQ(register_response1->http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"navigation-source");
ExpectValidAttributionReportingEligibleHeaderForNavigation(
register_response1->http_request()->headers.at(
"Attribution-Reporting-Eligible"));
ASSERT_FALSE(base::Contains(register_response1->http_request()->headers,
"Attribution-Reporting-Support"));

Expand All @@ -1249,9 +1249,9 @@ IN_PROC_BROWSER_TEST_F(AttributionsCrossAppWebRuntimeDisabledBrowserTest,
// Ensure that redirect requests also don't contain the
// Attribution-Reporting-Support header.
register_response2->WaitForRequest();
EXPECT_EQ(register_response2->http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"navigation-source");
ExpectValidAttributionReportingEligibleHeaderForNavigation(
register_response2->http_request()->headers.at(
"Attribution-Reporting-Eligible"));
EXPECT_FALSE(base::Contains(register_response2->http_request()->headers,
"Attribution-Reporting-Support"));
}
Expand Down
52 changes: 25 additions & 27 deletions content/browser/fenced_frame/fenced_frame_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "components/ukm/test_ukm_recorder.h"
#include "content/browser/attribution_reporting/attribution_manager.h"
#include "content/browser/attribution_reporting/attribution_os_level_manager.h"
#include "content/browser/attribution_reporting/attribution_test_utils.h"
#include "content/browser/attribution_reporting/test/mock_content_browser_client.h"
#include "content/browser/back_forward_cache_browsertest.h"
#include "content/browser/fenced_frame/fenced_frame.h"
Expand Down Expand Up @@ -4982,9 +4983,9 @@ class FencedFrameReportEventBrowserTest
}
// Verify the request contains the eligibility header.
if (step.expect_attribution_reporting_allowed) {
EXPECT_EQ(response.http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"event-source");
ExpectValidAttributionReportingEligibleHeaderForEventBeacon(
response.http_request()->headers.at(
"Attribution-Reporting-Eligible"));
} else {
EXPECT_FALSE(base::Contains(response.http_request()->headers,
"Attribution-Reporting-Eligible"));
Expand Down Expand Up @@ -5512,9 +5513,9 @@ IN_PROC_BROWSER_TEST_F(FencedFrameReportEventBrowserTest,
// Verify the request has the correct content.
EXPECT_EQ(reporting_response.http_request()->content, event_data);
// Verify the request contains the eligibility header.
EXPECT_EQ(reporting_response.http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"event-source");
ExpectValidAttributionReportingEligibleHeaderForEventBeacon(
reporting_response.http_request()->headers.at(
"Attribution-Reporting-Eligible"));
EXPECT_FALSE(base::Contains(reporting_response.http_request()->headers,
"Attribution-Reporting-Support"));
}
Expand Down Expand Up @@ -5600,9 +5601,8 @@ IN_PROC_BROWSER_TEST_F(FencedFrameReportEventBrowserTest,
EXPECT_EQ(response.http_request()->content, event_data);
EXPECT_EQ(response.http_request()->method,
net::test_server::HttpMethod::METHOD_POST);
EXPECT_EQ(
response.http_request()->headers.at("Attribution-Reporting-Eligible"),
"event-source");
ExpectValidAttributionReportingEligibleHeaderForEventBeacon(
response.http_request()->headers.at("Attribution-Reporting-Eligible"));
EXPECT_TRUE(
base::Contains(response.http_request()->headers, "Content-Length"));
EXPECT_TRUE(
Expand Down Expand Up @@ -5637,9 +5637,9 @@ IN_PROC_BROWSER_TEST_F(FencedFrameReportEventBrowserTest,
// Check that the content body was stripped.
EXPECT_TRUE(redirect_response.http_request()->content.empty());
// These extra request headers were not stripped.
EXPECT_EQ(redirect_response.http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"event-source");
ExpectValidAttributionReportingEligibleHeaderForEventBeacon(
redirect_response.http_request()->headers.at(
"Attribution-Reporting-Eligible"));
EXPECT_FALSE(base::Contains(response.http_request()->headers,
"Attribution-Reporting-Support"));
}
Expand Down Expand Up @@ -5733,9 +5733,9 @@ IN_PROC_BROWSER_TEST_F(FencedFrameReportEventBrowserTest,
"Content-Type"));
EXPECT_TRUE(
base::Contains(reporting_response.http_request()->headers, "Origin"));
EXPECT_EQ(reporting_response.http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"event-source");
ExpectValidAttributionReportingEligibleHeaderForEventBeacon(
reporting_response.http_request()->headers.at(
"Attribution-Reporting-Eligible"));
EXPECT_FALSE(base::Contains(reporting_response.http_request()->headers,
"Attribution-Reporting-Support"));

Expand Down Expand Up @@ -5764,9 +5764,9 @@ IN_PROC_BROWSER_TEST_F(FencedFrameReportEventBrowserTest,
// Check that the content body was stripped.
EXPECT_TRUE(redirect_response.http_request()->content.empty());
// These extra request headers were not stripped.
EXPECT_EQ(redirect_response.http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"event-source");
ExpectValidAttributionReportingEligibleHeaderForEventBeacon(
redirect_response.http_request()->headers.at(
"Attribution-Reporting-Eligible"));
EXPECT_FALSE(base::Contains(reporting_response.http_request()->headers,
"Attribution-Reporting-Support"));
}
Expand Down Expand Up @@ -5938,9 +5938,8 @@ IN_PROC_BROWSER_TEST_F(
// Verify the request contains the eligibility header.
response.WaitForRequest();
EXPECT_EQ(response.http_request()->content, event_data);
EXPECT_EQ(
response.http_request()->headers.at("Attribution-Reporting-Eligible"),
"event-source");
ExpectValidAttributionReportingEligibleHeaderForEventBeacon(
response.http_request()->headers.at("Attribution-Reporting-Eligible"));
EXPECT_EQ(
response.http_request()->headers.at("Attribution-Reporting-Support"),
"os, web");
Expand Down Expand Up @@ -6018,9 +6017,9 @@ IN_PROC_BROWSER_TEST_F(
{
reporting_response.WaitForRequest();
EXPECT_EQ(reporting_response.http_request()->content, event_data);
EXPECT_EQ(reporting_response.http_request()->headers.at(
"Attribution-Reporting-Eligible"),
"event-source");
ExpectValidAttributionReportingEligibleHeaderForEventBeacon(
reporting_response.http_request()->headers.at(
"Attribution-Reporting-Eligible"));
EXPECT_EQ(reporting_response.http_request()->headers.at(
"Attribution-Reporting-Support"),
"web");
Expand Down Expand Up @@ -6436,9 +6435,8 @@ class FencedFrameAutomaticBeaconBrowserTest
EXPECT_EQ(response.http_request()->content, config.message);
}
// Verify the request contains the eligibility header.
EXPECT_EQ(
response.http_request()->headers.at("Attribution-Reporting-Eligible"),
"navigation-source");
ExpectValidAttributionReportingEligibleHeaderForNavigation(
response.http_request()->headers.at("Attribution-Reporting-Eligible"));
EXPECT_FALSE(base::Contains(response.http_request()->headers,
"Attribution-Reporting-Support"));
response.Done();
Expand Down

0 comments on commit 0e8a752

Please sign in to comment.