Reporting: Fill in user_agent field in reports
Per https://github.com/w3c/reporting/issues/52, we are now supposed to
include the User-Agent string of the original request in any reports
generated about that request.
TBR=markusheintz@chromium.org
Bug: 865394
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ib49244399ed05398d598da1d02131153dd509306
Reviewed-on: https://chromium-review.googlesource.com/1140203
Commit-Queue: Douglas Creager <dcreager@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576599}diff --git a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
index 39c460c7..af54b0e 100644
--- a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
+++ b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
@@ -988,6 +988,7 @@
~MockReportingService() override = default;
void QueueReport(const GURL& url,
+ const std::string& user_agent,
const std::string& group,
const std::string& type,
std::unique_ptr<const base::Value> body,
diff --git a/chrome/browser/net/reporting_browsertest.cc b/chrome/browser/net/reporting_browsertest.cc
index 28c3fe85..ce4eea42 100644
--- a/chrome/browser/net/reporting_browsertest.cc
+++ b/chrome/browser/net/reporting_browsertest.cc
@@ -113,6 +113,10 @@
for (auto& report : parsed_payload->GetList()) {
report.RemoveKey("age");
report.RemovePath({"body", "elapsed_time"});
+ auto* user_agent =
+ report.FindKeyOfType("user_agent", base::Value::Type::STRING);
+ if (user_agent != nullptr)
+ *user_agent = base::Value("Mozilla/1.0");
}
return parsed_payload;
}
@@ -155,6 +159,7 @@
},
"type": "network-error",
"url": "https://example.com:%d/original",
+ "user_agent": "Mozilla/1.0",
},
]
)json",
diff --git a/content/browser/net/reporting_service_proxy.cc b/content/browser/net/reporting_service_proxy.cc
index b231bd7..6f525514 100644
--- a/content/browser/net/reporting_service_proxy.cc
+++ b/content/browser/net/reporting_service_proxy.cc
@@ -16,6 +16,7 @@
#include "mojo/public/cpp/bindings/strong_binding.h"
#include "net/reporting/reporting_report.h"
#include "net/reporting/reporting_service.h"
+#include "net/url_request/http_user_agent_settings.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_getter.h"
#include "third_party/blink/public/platform/reporting.mojom.h"
@@ -125,7 +126,11 @@
// Depth is only non-zero for NEL reports, and those can't come from the
// renderer.
- reporting_service->QueueReport(url, group, type, std::move(body),
+ std::string user_agent;
+ if (request_context->http_user_agent_settings() != nullptr)
+ user_agent = request_context->http_user_agent_settings()->GetUserAgent();
+ reporting_service->QueueReport(url, user_agent, group, type,
+ std::move(body),
/* depth= */ 0);
}
diff --git a/net/network_error_logging/network_error_logging_service.cc b/net/network_error_logging/network_error_logging_service.cc
index b694d19..1a95e58 100644
--- a/net/network_error_logging/network_error_logging_service.cc
+++ b/net/network_error_logging/network_error_logging_service.cc
@@ -307,7 +307,7 @@
}
reporting_service_->QueueReport(
- details.uri, policy->report_to, kReportType,
+ details.uri, details.user_agent, policy->report_to, kReportType,
CreateReportBody(phase_string, type_string, sampling_fraction, details),
details.reporting_upload_depth);
RecordRequestOutcome(RequestOutcome::QUEUED);
diff --git a/net/network_error_logging/network_error_logging_service.h b/net/network_error_logging/network_error_logging_service.h
index cfb4968..058c0ad2 100644
--- a/net/network_error_logging/network_error_logging_service.h
+++ b/net/network_error_logging/network_error_logging_service.h
@@ -53,6 +53,7 @@
GURL uri;
GURL referrer;
+ std::string user_agent;
IPAddress server_ip;
std::string protocol;
std::string method;
diff --git a/net/network_error_logging/network_error_logging_service_unittest.cc b/net/network_error_logging/network_error_logging_service_unittest.cc
index 86574e0..da785393 100644
--- a/net/network_error_logging/network_error_logging_service_unittest.cc
+++ b/net/network_error_logging/network_error_logging_service_unittest.cc
@@ -33,17 +33,20 @@
Report(Report&& other)
: url(other.url),
+ user_agent(other.user_agent),
group(other.group),
type(other.type),
body(std::move(other.body)),
depth(other.depth) {}
Report(const GURL& url,
+ const std::string& user_agent,
const std::string& group,
const std::string& type,
std::unique_ptr<const base::Value> body,
int depth)
: url(url),
+ user_agent(user_agent),
group(group),
type(type),
body(std::move(body)),
@@ -52,6 +55,7 @@
~Report() = default;
GURL url;
+ std::string user_agent;
std::string group;
std::string type;
std::unique_ptr<const base::Value> body;
@@ -70,11 +74,13 @@
~TestReportingService() override = default;
void QueueReport(const GURL& url,
+ const std::string& user_agent,
const std::string& group,
const std::string& type,
std::unique_ptr<const base::Value> body,
int depth) override {
- reports_.push_back(Report(url, group, type, std::move(body), depth));
+ reports_.push_back(
+ Report(url, user_agent, group, type, std::move(body), depth));
}
void ProcessHeader(const GURL& url,
@@ -139,6 +145,7 @@
details.uri = url;
details.referrer = kReferrer_;
+ details.user_agent = kUserAgent_;
details.server_ip = server_ip.IsValid() ? server_ip : kServerIP_;
details.method = std::move(method);
details.status_code = status_code;
@@ -179,6 +186,7 @@
"{\"report_to\":\"group\",\"max_age\":86400,\"junk\":[[[[[[[[[[]]]]]]]]]]"
"}";
+ const std::string kUserAgent_ = "Mozilla/1.0";
const std::string kGroup_ = "group";
const std::string kType_ = NetworkErrorLoggingService::kReportType;
@@ -254,6 +262,7 @@
ASSERT_EQ(1u, reports().size());
EXPECT_EQ(kUrl_, reports()[0].url);
+ EXPECT_EQ(kUserAgent_, reports()[0].user_agent);
EXPECT_EQ(kGroup_, reports()[0].group);
EXPECT_EQ(kType_, reports()[0].type);
EXPECT_EQ(0, reports()[0].depth);
@@ -290,6 +299,7 @@
ASSERT_EQ(1u, reports().size());
EXPECT_EQ(kUrl_, reports()[0].url);
+ EXPECT_EQ(kUserAgent_, reports()[0].user_agent);
EXPECT_EQ(kGroup_, reports()[0].group);
EXPECT_EQ(kType_, reports()[0].type);
EXPECT_EQ(0, reports()[0].depth);
@@ -326,6 +336,7 @@
ASSERT_EQ(1u, reports().size());
EXPECT_EQ(kUrl_, reports()[0].url);
+ EXPECT_EQ(kUserAgent_, reports()[0].user_agent);
EXPECT_EQ(kGroup_, reports()[0].group);
EXPECT_EQ(kType_, reports()[0].type);
EXPECT_EQ(0, reports()[0].depth);
diff --git a/net/reporting/reporting_browsing_data_remover_unittest.cc b/net/reporting/reporting_browsing_data_remover_unittest.cc
index 0704806..d32128b3 100644
--- a/net/reporting/reporting_browsing_data_remover_unittest.cc
+++ b/net/reporting/reporting_browsing_data_remover_unittest.cc
@@ -41,7 +41,7 @@
}
void AddReport(const GURL& url) {
- cache()->AddReport(url, kGroup_, kType_,
+ cache()->AddReport(url, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
}
@@ -74,6 +74,7 @@
const url::Origin kOrigin1_ = url::Origin::Create(kUrl1_);
const url::Origin kOrigin2_ = url::Origin::Create(kUrl2_);
const GURL kEndpoint_ = GURL("https://endpoint/");
+ const std::string kUserAgent_ = "Mozilla/1.0";
const std::string kGroup_ = "group";
const std::string kType_ = "default";
};
diff --git a/net/reporting/reporting_cache.cc b/net/reporting/reporting_cache.cc
index 3f30793..9756fc9 100644
--- a/net/reporting/reporting_cache.cc
+++ b/net/reporting/reporting_cache.cc
@@ -71,6 +71,7 @@
}
void AddReport(const GURL& url,
+ const std::string& user_agent,
const std::string& group,
const std::string& type,
std::unique_ptr<const base::Value> body,
@@ -78,7 +79,7 @@
base::TimeTicks queued,
int attempts) override {
auto report = std::make_unique<ReportingReport>(
- url, group, type, std::move(body), depth, queued, attempts);
+ url, user_agent, group, type, std::move(body), depth, queued, attempts);
auto inserted =
reports_.insert(std::make_pair(report.get(), std::move(report)));
diff --git a/net/reporting/reporting_cache.h b/net/reporting/reporting_cache.h
index eda09b3..18ed839 100644
--- a/net/reporting/reporting_cache.h
+++ b/net/reporting/reporting_cache.h
@@ -60,6 +60,7 @@
// All parameters correspond to the desired values for the relevant fields in
// ReportingReport.
virtual void AddReport(const GURL& url,
+ const std::string& user_agent,
const std::string& group,
const std::string& type,
std::unique_ptr<const base::Value> body,
diff --git a/net/reporting/reporting_cache_unittest.cc b/net/reporting/reporting_cache_unittest.cc
index 6d4dd30..93fd1e6 100644
--- a/net/reporting/reporting_cache_unittest.cc
+++ b/net/reporting/reporting_cache_unittest.cc
@@ -77,6 +77,7 @@
// Adds a new report to the cache, and returns it.
const ReportingReport* AddAndReturnReport(
const GURL& url,
+ const std::string& user_agent,
const std::string& group,
const std::string& type,
std::unique_ptr<const base::Value> body,
@@ -91,8 +92,8 @@
// in test cases, so I've optimized for readability over execution speed.
std::vector<const ReportingReport*> before;
cache()->GetReports(&before);
- cache()->AddReport(url, group, type, std::move(body), depth, queued,
- attempts);
+ cache()->AddReport(url, user_agent, group, type, std::move(body), depth,
+ queued, attempts);
std::vector<const ReportingReport*> after;
cache()->GetReports(&after);
@@ -101,6 +102,7 @@
if (std::find(before.begin(), before.end(), report) == before.end()) {
// Sanity check the result before we return it.
EXPECT_EQ(url, report->url);
+ EXPECT_EQ(user_agent, report->user_agent);
EXPECT_EQ(group, report->group);
EXPECT_EQ(type, report->type);
EXPECT_EQ(*body_unowned, *report->body);
@@ -123,6 +125,7 @@
const url::Origin kOrigin2_ = url::Origin::Create(GURL("https://origin2/"));
const GURL kEndpoint1_ = GURL("https://endpoint1/");
const GURL kEndpoint2_ = GURL("https://endpoint2/");
+ const std::string kUserAgent_ = "Mozilla/1.0";
const std::string kGroup1_ = "group1";
const std::string kGroup2 = "group2";
const std::string kType_ = "default";
@@ -139,7 +142,7 @@
cache()->GetReports(&reports);
EXPECT_TRUE(reports.empty());
- cache()->AddReport(kUrl1_, kGroup1_, kType_,
+ cache()->AddReport(kUrl1_, kUserAgent_, kGroup1_, kType_,
std::make_unique<base::DictionaryValue>(), 0, kNow_, 0);
EXPECT_EQ(1, observer()->cache_update_count());
@@ -148,6 +151,7 @@
const ReportingReport* report = reports[0];
ASSERT_TRUE(report);
EXPECT_EQ(kUrl1_, report->url);
+ EXPECT_EQ(kUserAgent_, report->user_agent);
EXPECT_EQ(kGroup1_, report->group);
EXPECT_EQ(kType_, report->type);
// TODO(juliatuttle): Check body?
@@ -173,9 +177,9 @@
}
TEST_F(ReportingCacheTest, RemoveAllReports) {
- cache()->AddReport(kUrl1_, kGroup1_, kType_,
+ cache()->AddReport(kUrl1_, kUserAgent_, kGroup1_, kType_,
std::make_unique<base::DictionaryValue>(), 0, kNow_, 0);
- cache()->AddReport(kUrl1_, kGroup1_, kType_,
+ cache()->AddReport(kUrl1_, kUserAgent_, kGroup1_, kType_,
std::make_unique<base::DictionaryValue>(), 0, kNow_, 0);
EXPECT_EQ(2, observer()->cache_update_count());
@@ -191,7 +195,7 @@
}
TEST_F(ReportingCacheTest, RemovePendingReports) {
- cache()->AddReport(kUrl1_, kGroup1_, kType_,
+ cache()->AddReport(kUrl1_, kUserAgent_, kGroup1_, kType_,
std::make_unique<base::DictionaryValue>(), 0, kNow_, 0);
EXPECT_EQ(1, observer()->cache_update_count());
@@ -222,7 +226,7 @@
}
TEST_F(ReportingCacheTest, RemoveAllPendingReports) {
- cache()->AddReport(kUrl1_, kGroup1_, kType_,
+ cache()->AddReport(kUrl1_, kUserAgent_, kGroup1_, kType_,
std::make_unique<base::DictionaryValue>(), 0, kNow_, 0);
EXPECT_EQ(1, observer()->cache_update_count());
@@ -255,16 +259,18 @@
TEST_F(ReportingCacheTest, GetReportsAsValue) {
// We need a reproducible expiry timestamp for this test case.
const base::TimeTicks now = base::TimeTicks();
- const ReportingReport* report1 = AddAndReturnReport(
- kUrl1_, kGroup1_, kType_, std::make_unique<base::DictionaryValue>(), 0,
- now + base::TimeDelta::FromSeconds(200), 0);
- const ReportingReport* report2 = AddAndReturnReport(
- kUrl1_, kGroup2, kType_, std::make_unique<base::DictionaryValue>(), 0,
- now + base::TimeDelta::FromSeconds(100), 1);
- cache()->AddReport(kUrl2_, kGroup1_, kType_,
+ const ReportingReport* report1 =
+ AddAndReturnReport(kUrl1_, kUserAgent_, kGroup1_, kType_,
+ std::make_unique<base::DictionaryValue>(), 0,
+ now + base::TimeDelta::FromSeconds(200), 0);
+ const ReportingReport* report2 =
+ AddAndReturnReport(kUrl1_, kUserAgent_, kGroup2, kType_,
+ std::make_unique<base::DictionaryValue>(), 0,
+ now + base::TimeDelta::FromSeconds(100), 1);
+ cache()->AddReport(kUrl2_, kUserAgent_, kGroup1_, kType_,
std::make_unique<base::DictionaryValue>(), 2,
now + base::TimeDelta::FromSeconds(200), 0);
- cache()->AddReport(kUrl1_, kGroup1_, kType_,
+ cache()->AddReport(kUrl1_, kUserAgent_, kGroup1_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
now + base::TimeDelta::FromSeconds(300), 0);
// Mark report1 as pending as report2 as doomed
@@ -570,7 +576,7 @@
// Enqueue the maximum number of reports, spaced apart in time.
for (size_t i = 0; i < max_report_count; ++i) {
- cache()->AddReport(kUrl1_, kGroup1_, kType_,
+ cache()->AddReport(kUrl1_, kUserAgent_, kGroup1_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
tick_clock()->Advance(base::TimeDelta::FromMinutes(1));
@@ -578,7 +584,7 @@
EXPECT_EQ(max_report_count, report_count());
// Add one more report to force the cache to evict one.
- cache()->AddReport(kUrl1_, kGroup1_, kType_,
+ cache()->AddReport(kUrl1_, kUserAgent_, kGroup1_, kType_,
std::make_unique<base::DictionaryValue>(), 0, kNow_, 0);
// Make sure the cache evicted a report to make room for the new one, and make
@@ -598,7 +604,7 @@
// Enqueue the maximum number of reports, spaced apart in time.
for (size_t i = 0; i < max_report_count; ++i) {
- cache()->AddReport(kUrl1_, kGroup1_, kType_,
+ cache()->AddReport(kUrl1_, kUserAgent_, kGroup1_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
tick_clock()->Advance(base::TimeDelta::FromMinutes(1));
@@ -612,7 +618,7 @@
// Add one more report to force the cache to evict one. Since the cache has
// only pending reports, it will be forced to evict the *new* report!
- cache()->AddReport(kUrl1_, kGroup1_, kType_,
+ cache()->AddReport(kUrl1_, kUserAgent_, kGroup1_, kType_,
std::make_unique<base::DictionaryValue>(), 0, kNow_, 0);
// Make sure the cache evicted a report, and make sure the report evicted was
diff --git a/net/reporting/reporting_delivery_agent.cc b/net/reporting/reporting_delivery_agent.cc
index e2d2f766..a0f87af 100644
--- a/net/reporting/reporting_delivery_agent.cc
+++ b/net/reporting/reporting_delivery_agent.cc
@@ -41,6 +41,7 @@
report_value->SetInteger("age", (now - report->queued).InMilliseconds());
report_value->SetString("type", report->type);
report_value->SetString("url", report->url.spec());
+ report_value->SetString("user_agent", report->user_agent);
report_value->SetKey("body", report->body->Clone());
reports_value.Append(std::move(report_value));
diff --git a/net/reporting/reporting_delivery_agent_unittest.cc b/net/reporting/reporting_delivery_agent_unittest.cc
index d58393a..7f87e30 100644
--- a/net/reporting/reporting_delivery_agent_unittest.cc
+++ b/net/reporting/reporting_delivery_agent_unittest.cc
@@ -56,6 +56,7 @@
const GURL kSubdomainUrl_ = GURL("https://sub.origin/path");
const url::Origin kOrigin_ = url::Origin::Create(GURL("https://origin/"));
const GURL kEndpoint_ = GURL("https://endpoint/");
+ const std::string kUserAgent_ = "Mozilla/1.0";
const std::string kGroup_ = "group";
const std::string kType_ = "type";
};
@@ -65,8 +66,8 @@
body.SetString("key", "value");
SetClient(kOrigin_, kEndpoint_, kGroup_);
- cache()->AddReport(kUrl_, kGroup_, kType_, body.CreateDeepCopy(), 0,
- tick_clock()->NowTicks(), 0);
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_, body.CreateDeepCopy(),
+ 0, tick_clock()->NowTicks(), 0);
// Upload is automatically started when cache is modified.
@@ -81,11 +82,12 @@
base::DictionaryValue* report;
ASSERT_TRUE(list->GetDictionary(0, &report));
- EXPECT_EQ(4u, report->size());
+ EXPECT_EQ(5u, report->size());
ExpectDictIntegerValue(0, *report, "age");
ExpectDictStringValue(kType_, *report, "type");
ExpectDictStringValue(kUrl_.spec(), *report, "url");
+ ExpectDictStringValue(kUserAgent_, *report, "user_agent");
ExpectDictDictionaryValue(body, *report, "body");
}
pending_uploads()[0]->Complete(ReportingUploader::Outcome::SUCCESS);
@@ -113,8 +115,8 @@
SetClient(kOrigin_, kEndpoint_, kGroup_,
ReportingClient::Subdomains::INCLUDE);
- cache()->AddReport(kSubdomainUrl_, kGroup_, kType_, body.CreateDeepCopy(), 0,
- tick_clock()->NowTicks(), 0);
+ cache()->AddReport(kSubdomainUrl_, kUserAgent_, kGroup_, kType_,
+ body.CreateDeepCopy(), 0, tick_clock()->NowTicks(), 0);
// Upload is automatically started when cache is modified.
@@ -129,11 +131,12 @@
base::DictionaryValue* report;
ASSERT_TRUE(list->GetDictionary(0, &report));
- EXPECT_EQ(4u, report->size());
+ EXPECT_EQ(5u, report->size());
ExpectDictIntegerValue(0, *report, "age");
ExpectDictStringValue(kType_, *report, "type");
ExpectDictStringValue(kSubdomainUrl_.spec(), *report, "url");
+ ExpectDictStringValue(kUserAgent_, *report, "user_agent");
ExpectDictDictionaryValue(body, *report, "body");
}
pending_uploads()[0]->Complete(ReportingUploader::Outcome::SUCCESS);
@@ -162,8 +165,8 @@
SetClient(kOrigin_, kEndpoint_, kGroup_,
ReportingClient::Subdomains::INCLUDE);
- cache()->AddReport(kSubdomainUrl_, kGroup_, kType_, body.CreateDeepCopy(), 0,
- tick_clock()->NowTicks(), 0);
+ cache()->AddReport(kSubdomainUrl_, kUserAgent_, kGroup_, kType_,
+ body.CreateDeepCopy(), 0, tick_clock()->NowTicks(), 0);
// Upload is automatically started when cache is modified.
@@ -194,13 +197,13 @@
// Trigger and complete an upload to start the delivery timer.
SetClient(kOrigin_, kEndpoint_, kGroup_);
- cache()->AddReport(kUrl_, kGroup_, kType_, body.CreateDeepCopy(), 0,
- tick_clock()->NowTicks(), 0);
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_, body.CreateDeepCopy(),
+ 0, tick_clock()->NowTicks(), 0);
pending_uploads()[0]->Complete(ReportingUploader::Outcome::SUCCESS);
SetClient(kOrigin_, kEndpoint_, kGroup_);
- cache()->AddReport(kUrl_, kGroup_, kType_, body.CreateDeepCopy(), 0,
- tick_clock()->NowTicks(), 0);
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_, body.CreateDeepCopy(),
+ 0, tick_clock()->NowTicks(), 0);
EXPECT_TRUE(delivery_timer()->IsRunning());
delivery_timer()->Fire();
@@ -216,11 +219,12 @@
base::DictionaryValue* report;
ASSERT_TRUE(list->GetDictionary(0, &report));
- EXPECT_EQ(4u, report->size());
+ EXPECT_EQ(5u, report->size());
ExpectDictIntegerValue(0, *report, "age");
ExpectDictStringValue(kType_, *report, "type");
ExpectDictStringValue(kUrl_.spec(), *report, "url");
+ ExpectDictStringValue(kUserAgent_, *report, "user_agent");
ExpectDictDictionaryValue(body, *report, "body");
}
pending_uploads()[0]->Complete(ReportingUploader::Outcome::SUCCESS);
@@ -244,7 +248,7 @@
TEST_F(ReportingDeliveryAgentTest, FailedUpload) {
SetClient(kOrigin_, kEndpoint_, kGroup_);
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
@@ -297,8 +301,8 @@
body.SetString("key", "value");
SetClient(kOrigin_, kEndpoint_, kGroup_);
- cache()->AddReport(kUrl_, kGroup_, kType_, body.CreateDeepCopy(), 0,
- tick_clock()->NowTicks(), 0);
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_, body.CreateDeepCopy(),
+ 0, tick_clock()->NowTicks(), 0);
tick_clock()->Advance(base::TimeDelta::FromMilliseconds(kAgeMillis));
@@ -333,7 +337,7 @@
ASSERT_TRUE(FindClientInCache(cache(), kOrigin_, kEndpoint_));
ASSERT_TRUE(FindClientInCache(cache(), kDifferentOrigin, kEndpoint_));
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
@@ -362,7 +366,7 @@
TEST_F(ReportingDeliveryAgentTest, ConcurrentRemove) {
SetClient(kOrigin_, kEndpoint_, kGroup_);
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
@@ -401,7 +405,7 @@
context()->test_delegate()->set_pause_permissions_check(true);
SetClient(kOrigin_, kEndpoint_, kGroup_);
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
@@ -444,17 +448,17 @@
SetClient(kDifferentOrigin, kEndpoint_, kGroup_);
// Trigger and complete an upload to start the delivery timer.
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
pending_uploads()[0]->Complete(ReportingUploader::Outcome::SUCCESS);
// Now that the delivery timer is running, these reports won't be immediately
// uploaded.
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
- cache()->AddReport(kDifferentUrl, kGroup_, kType_,
+ cache()->AddReport(kDifferentUrl, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
EXPECT_EQ(0u, pending_uploads().size());
@@ -477,7 +481,7 @@
TEST_F(ReportingDeliveryAgentTest, SerializeUploadsToEndpoint) {
SetClient(kOrigin_, kEndpoint_, kGroup_);
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
@@ -485,7 +489,7 @@
delivery_timer()->Fire();
EXPECT_EQ(1u, pending_uploads().size());
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
@@ -513,7 +517,7 @@
SetClient(kOrigin_, kEndpoint_, kGroup_);
SetClient(kOrigin_, kDifferentEndpoint, kGroup_);
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
@@ -521,7 +525,7 @@
delivery_timer()->Fire();
EXPECT_EQ(1u, pending_uploads().size());
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
@@ -549,10 +553,10 @@
SetClient(kOrigin_, kEndpoint_, kGroup_);
SetClient(kOrigin_, kDifferentEndpoint, kDifferentGroup);
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
- cache()->AddReport(kUrl_, kDifferentGroup, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kDifferentGroup, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
diff --git a/net/reporting/reporting_garbage_collector_unittest.cc b/net/reporting/reporting_garbage_collector_unittest.cc
index 5ae177b..78ad1601 100644
--- a/net/reporting/reporting_garbage_collector_unittest.cc
+++ b/net/reporting/reporting_garbage_collector_unittest.cc
@@ -27,6 +27,7 @@
}
const GURL kUrl_ = GURL("https://origin/path");
+ const std::string kUserAgent_ = "Mozilla/1.0";
const std::string kGroup_ = "group";
const std::string kType_ = "default";
};
@@ -40,7 +41,7 @@
TEST_F(ReportingGarbageCollectorTest, Timer) {
EXPECT_FALSE(garbage_collection_timer()->IsRunning());
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
@@ -52,7 +53,7 @@
}
TEST_F(ReportingGarbageCollectorTest, Report) {
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
garbage_collection_timer()->Fire();
@@ -61,7 +62,7 @@
}
TEST_F(ReportingGarbageCollectorTest, ExpiredReport) {
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
tick_clock()->Advance(2 * policy().max_report_age);
@@ -71,7 +72,7 @@
}
TEST_F(ReportingGarbageCollectorTest, FailedReport) {
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
diff --git a/net/reporting/reporting_network_change_observer_unittest.cc b/net/reporting/reporting_network_change_observer_unittest.cc
index db1aabc..c5610d0 100644
--- a/net/reporting/reporting_network_change_observer_unittest.cc
+++ b/net/reporting/reporting_network_change_observer_unittest.cc
@@ -54,6 +54,7 @@
const GURL kUrl_ = GURL("https://origin/path");
const url::Origin kOrigin_ = url::Origin::Create(kUrl_);
const GURL kEndpoint_ = GURL("https://endpoint/");
+ const std::string kUserAgent_ = "Mozilla/1.0";
const std::string kGroup_ = "group";
const std::string kType_ = "default";
};
@@ -64,7 +65,7 @@
new_policy.persist_clients_across_network_changes = true;
UsePolicy(new_policy);
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
SetClient();
@@ -83,7 +84,7 @@
new_policy.persist_clients_across_network_changes = true;
UsePolicy(new_policy);
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
SetClient();
@@ -102,7 +103,7 @@
new_policy.persist_clients_across_network_changes = false;
UsePolicy(new_policy);
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
SetClient();
@@ -121,7 +122,7 @@
new_policy.persist_clients_across_network_changes = false;
UsePolicy(new_policy);
- cache()->AddReport(kUrl_, kGroup_, kType_,
+ cache()->AddReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0,
tick_clock()->NowTicks(), 0);
SetClient();
diff --git a/net/reporting/reporting_report.cc b/net/reporting/reporting_report.cc
index c6dd8204..6818f0d4 100644
--- a/net/reporting/reporting_report.cc
+++ b/net/reporting/reporting_report.cc
@@ -25,6 +25,7 @@
} // namespace
ReportingReport::ReportingReport(const GURL& url,
+ const std::string& user_agent,
const std::string& group,
const std::string& type,
std::unique_ptr<const base::Value> body,
@@ -32,6 +33,7 @@
base::TimeTicks queued,
int attempts)
: url(url),
+ user_agent(user_agent),
group(group),
type(type),
body(std::move(body)),
diff --git a/net/reporting/reporting_report.h b/net/reporting/reporting_report.h
index 196be9e..18d329c 100644
--- a/net/reporting/reporting_report.h
+++ b/net/reporting/reporting_report.h
@@ -38,6 +38,7 @@
};
ReportingReport(const GURL& url,
+ const std::string& user_agent,
const std::string& group,
const std::string& type,
std::unique_ptr<const base::Value> body,
@@ -55,6 +56,9 @@
// delivered report.)
GURL url;
+ // The User-Agent header that was used for the request.
+ std::string user_agent;
+
// The endpoint group that should be used to deliver the report. (Not included
// in the delivered report.)
std::string group;
diff --git a/net/reporting/reporting_service.cc b/net/reporting/reporting_service.cc
index fc53e4e..58c48541 100644
--- a/net/reporting/reporting_service.cc
+++ b/net/reporting/reporting_service.cc
@@ -34,6 +34,7 @@
~ReportingServiceImpl() override = default;
void QueueReport(const GURL& url,
+ const std::string& user_agent,
const std::string& group,
const std::string& type,
std::unique_ptr<const base::Value> body,
@@ -44,8 +45,8 @@
if (!context_->delegate()->CanQueueReport(url::Origin::Create(url)))
return;
- context_->cache()->AddReport(url, group, type, std::move(body), depth,
- context_->tick_clock()->NowTicks(), 0);
+ context_->cache()->AddReport(url, user_agent, group, type, std::move(body),
+ depth, context_->tick_clock()->NowTicks(), 0);
}
void ProcessHeader(const GURL& url,
diff --git a/net/reporting/reporting_service.h b/net/reporting/reporting_service.h
index e30d122..a852d0f 100644
--- a/net/reporting/reporting_service.h
+++ b/net/reporting/reporting_service.h
@@ -45,12 +45,14 @@
std::unique_ptr<ReportingContext> reporting_context);
// Queues a report for delivery. |url| is the URL that originated the report.
+ // |user_agent| is the User-Agent header that was used for the request.
// |group| is the endpoint group to which the report should be delivered.
// |type| is the type of the report. |body| is the body of the report.
//
// The Reporting system will take ownership of |body|; all other parameters
// will be copied.
virtual void QueueReport(const GURL& url,
+ const std::string& user_agent,
const std::string& group,
const std::string& type,
std::unique_ptr<const base::Value> body,
diff --git a/net/reporting/reporting_service_unittest.cc b/net/reporting/reporting_service_unittest.cc
index 8cf5689..42613ede 100644
--- a/net/reporting/reporting_service_unittest.cc
+++ b/net/reporting/reporting_service_unittest.cc
@@ -27,6 +27,7 @@
const GURL kUrl_ = GURL("https://origin/path");
const url::Origin kOrigin_ = url::Origin::Create(kUrl_);
const GURL kEndpoint_ = GURL("https://endpoint/");
+ const std::string kUserAgent_ = "Mozilla/1.0";
const std::string kGroup_ = "group";
const std::string kType_ = "type";
@@ -48,13 +49,14 @@
};
TEST_F(ReportingServiceTest, QueueReport) {
- service()->QueueReport(kUrl_, kGroup_, kType_,
+ service()->QueueReport(kUrl_, kUserAgent_, kGroup_, kType_,
std::make_unique<base::DictionaryValue>(), 0);
std::vector<const ReportingReport*> reports;
context()->cache()->GetReports(&reports);
ASSERT_EQ(1u, reports.size());
EXPECT_EQ(kUrl_, reports[0]->url);
+ EXPECT_EQ(kUserAgent_, reports[0]->user_agent);
EXPECT_EQ(kGroup_, reports[0]->group);
EXPECT_EQ(kType_, reports[0]->type);
}
diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index bfd2627..0f5fe6f 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -33,6 +33,7 @@
#include "net/log/net_log_source_type.h"
#include "net/socket/next_proto.h"
#include "net/ssl/ssl_cert_request_info.h"
+#include "net/url_request/http_user_agent_settings.h"
#include "net/url_request/redirect_info.h"
#include "net/url_request/redirect_util.h"
#include "net/url_request/url_request_context.h"
@@ -1178,6 +1179,19 @@
}
#if BUILDFLAG(ENABLE_REPORTING)
+std::string URLRequest::GetUserAgent() const {
+ // This should be kept in sync with the corresponding code in
+ // URLRequestHttpJob::Start.
+ // TODO(dcreager): Consider whether we can share code instead of copy-pasting
+ std::string user_agent;
+ if (extra_request_headers_.GetHeader(net::HttpRequestHeaders::kUserAgent,
+ &user_agent))
+ return user_agent;
+ if (context()->http_user_agent_settings())
+ return context()->http_user_agent_settings()->GetUserAgent();
+ return std::string();
+}
+
void URLRequest::MaybeGenerateNetworkErrorLoggingReport() {
NetworkErrorLoggingService* service =
context()->network_error_logging_service();
@@ -1194,6 +1208,7 @@
details.uri = url();
details.referrer = GURL(referrer());
+ details.user_agent = GetUserAgent();
IPEndPoint endpoint;
if (GetRemoteEndpoint(&endpoint))
details.server_ip = endpoint.address();
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index b3c59f6a..6f23eed 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -832,6 +832,7 @@
void OnCallToDelegateComplete();
#if BUILDFLAG(ENABLE_REPORTING)
+ std::string GetUserAgent() const;
void MaybeGenerateNetworkErrorLoggingReport();
#endif // BUILDFLAG(ENABLE_REPORTING)
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index ff0bb02..be69cc9 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -443,6 +443,8 @@
request_info_.token_binding_referrer = request_->token_binding_referrer();
+ // This should be kept in sync with the corresponding code in
+ // URLRequest::GetUserAgent.
request_info_.extra_headers.SetHeaderIfMissing(
HttpRequestHeaders::kUserAgent,
http_user_agent_settings_ ?
diff --git a/net/url_request/url_request_test_util.cc b/net/url_request/url_request_test_util.cc
index f25913bc..5844a82 100644
--- a/net/url_request/url_request_test_util.cc
+++ b/net/url_request/url_request_test_util.cc
@@ -140,7 +140,7 @@
context_storage_.http_network_session(),
HttpCache::DefaultBackend::InMemory(0), true /* is_main_cache */));
}
- if (!http_user_agent_settings()) {
+ if (!http_user_agent_settings() && create_default_http_user_agent_settings_) {
context_storage_.set_http_user_agent_settings(
std::make_unique<StaticHttpUserAgentSettings>("en-us,fr",
std::string()));
diff --git a/net/url_request/url_request_test_util.h b/net/url_request/url_request_test_util.h
index feb3153..f6b9249 100644
--- a/net/url_request/url_request_test_util.h
+++ b/net/url_request/url_request_test_util.h
@@ -92,6 +92,10 @@
context_storage_.set_ct_policy_enforcer(std::move(ct_policy_enforcer));
}
+ void set_create_default_http_user_agent_settings(bool value) {
+ create_default_http_user_agent_settings_ = value;
+ }
+
private:
bool initialized_ = false;
@@ -106,6 +110,8 @@
ProxyDelegate* proxy_delegate_ = nullptr;
+ bool create_default_http_user_agent_settings_ = true;
+
protected:
URLRequestContextStorage context_storage_;
};
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 478e912..147539d 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -7482,6 +7482,7 @@
~TestReportingService() override = default;
void QueueReport(const GURL& url,
+ const std::string& user_agent,
const std::string& group,
const std::string& type,
std::unique_ptr<const base::Value> body,
@@ -7694,6 +7695,11 @@
return std::make_unique<test_server::RawHttpResponse>("", "");
}
+// Distinct User-Agent header values that we use to ensure that URLRequest
+// passes along user agents into NEL reports correctly.
+constexpr char kHeaderUserAgent[] = "MozillaFromHeader/1.0";
+constexpr char kSettingsUserAgent[] = "MozillaFromSettings/1.0";
+
} // namespace
TEST_F(URLRequestTestHTTP, DontProcessNelHeaderNoDelegate) {
@@ -7890,6 +7896,102 @@
EXPECT_EQ(ERR_EMPTY_RESPONSE, nel_service.errors()[0].type);
}
+TEST_F(URLRequestTestHTTP, NelReportUserAgentWithHeaderWithSettings) {
+ EmbeddedTestServer https_test_server(net::EmbeddedTestServer::TYPE_HTTPS);
+ ASSERT_TRUE(https_test_server.Start());
+ GURL request_url = https_test_server.base_url();
+
+ StaticHttpUserAgentSettings settings("en", kSettingsUserAgent);
+ TestNetworkDelegate network_delegate;
+ TestNetworkErrorLoggingService nel_service;
+ TestURLRequestContext context(true);
+ context.set_network_delegate(&network_delegate);
+ context.set_network_error_logging_service(&nel_service);
+ context.set_http_user_agent_settings(&settings);
+ context.Init();
+
+ TestDelegate d;
+ std::unique_ptr<URLRequest> request(context.CreateRequest(
+ request_url, DEFAULT_PRIORITY, &d, TRAFFIC_ANNOTATION_FOR_TESTS));
+ request->SetExtraRequestHeaderByName("User-Agent", kHeaderUserAgent, true);
+ request->Start();
+ d.RunUntilComplete();
+
+ ASSERT_EQ(1u, nel_service.errors().size());
+ EXPECT_EQ(kHeaderUserAgent, nel_service.errors()[0].user_agent);
+}
+
+TEST_F(URLRequestTestHTTP, NelReportUserAgentWithHeaderWithoutSettings) {
+ EmbeddedTestServer https_test_server(net::EmbeddedTestServer::TYPE_HTTPS);
+ ASSERT_TRUE(https_test_server.Start());
+ GURL request_url = https_test_server.base_url();
+
+ TestNetworkDelegate network_delegate;
+ TestNetworkErrorLoggingService nel_service;
+ TestURLRequestContext context(true);
+ context.set_network_delegate(&network_delegate);
+ context.set_network_error_logging_service(&nel_service);
+ context.set_create_default_http_user_agent_settings(false);
+ context.Init();
+
+ TestDelegate d;
+ std::unique_ptr<URLRequest> request(context.CreateRequest(
+ request_url, DEFAULT_PRIORITY, &d, TRAFFIC_ANNOTATION_FOR_TESTS));
+ request->SetExtraRequestHeaderByName("User-Agent", kHeaderUserAgent, true);
+ request->Start();
+ d.RunUntilComplete();
+
+ ASSERT_EQ(1u, nel_service.errors().size());
+ EXPECT_EQ(kHeaderUserAgent, nel_service.errors()[0].user_agent);
+}
+
+TEST_F(URLRequestTestHTTP, NelReportUserAgentWithoutHeaderWithSettings) {
+ EmbeddedTestServer https_test_server(net::EmbeddedTestServer::TYPE_HTTPS);
+ ASSERT_TRUE(https_test_server.Start());
+ GURL request_url = https_test_server.base_url();
+
+ StaticHttpUserAgentSettings settings("en", kSettingsUserAgent);
+ TestNetworkDelegate network_delegate;
+ TestNetworkErrorLoggingService nel_service;
+ TestURLRequestContext context(true);
+ context.set_network_delegate(&network_delegate);
+ context.set_network_error_logging_service(&nel_service);
+ context.set_http_user_agent_settings(&settings);
+ context.Init();
+
+ TestDelegate d;
+ std::unique_ptr<URLRequest> request(context.CreateRequest(
+ request_url, DEFAULT_PRIORITY, &d, TRAFFIC_ANNOTATION_FOR_TESTS));
+ request->Start();
+ d.RunUntilComplete();
+
+ ASSERT_EQ(1u, nel_service.errors().size());
+ EXPECT_EQ(kSettingsUserAgent, nel_service.errors()[0].user_agent);
+}
+
+TEST_F(URLRequestTestHTTP, NelReportUserAgentWithoutHeaderWithoutSettings) {
+ EmbeddedTestServer https_test_server(net::EmbeddedTestServer::TYPE_HTTPS);
+ ASSERT_TRUE(https_test_server.Start());
+ GURL request_url = https_test_server.base_url();
+
+ TestNetworkDelegate network_delegate;
+ TestNetworkErrorLoggingService nel_service;
+ TestURLRequestContext context(true);
+ context.set_network_delegate(&network_delegate);
+ context.set_network_error_logging_service(&nel_service);
+ context.set_create_default_http_user_agent_settings(false);
+ context.Init();
+
+ TestDelegate d;
+ std::unique_ptr<URLRequest> request(context.CreateRequest(
+ request_url, DEFAULT_PRIORITY, &d, TRAFFIC_ANNOTATION_FOR_TESTS));
+ request->Start();
+ d.RunUntilComplete();
+
+ ASSERT_EQ(1u, nel_service.errors().size());
+ EXPECT_EQ("", nel_service.errors()[0].user_agent);
+}
+
#endif // BUILDFLAG(ENABLE_REPORTING)
TEST_F(URLRequestTestHTTP, ContentTypeNormalizationTest) {
diff --git a/services/network/network_context_unittest.cc b/services/network/network_context_unittest.cc
index 3aaaab47..5d20147 100644
--- a/services/network/network_context_unittest.cc
+++ b/services/network/network_context_unittest.cc
@@ -1535,7 +1535,8 @@
reporting_service.get());
GURL domain("http://google.com");
- reporting_service->QueueReport(domain, "group", "type", nullptr, 0);
+ reporting_service->QueueReport(domain, "Mozilla/1.0", "group", "type",
+ nullptr, 0);
std::vector<const net::ReportingReport*> reports;
reporting_cache->GetReports(&reports);
@@ -1564,9 +1565,11 @@
reporting_service.get());
GURL domain1("http://google.com");
- reporting_service->QueueReport(domain1, "group", "type", nullptr, 0);
+ reporting_service->QueueReport(domain1, "Mozilla/1.0", "group", "type",
+ nullptr, 0);
GURL domain2("http://chromium.org");
- reporting_service->QueueReport(domain2, "group", "type", nullptr, 0);
+ reporting_service->QueueReport(domain2, "Mozilla/1.0", "group", "type",
+ nullptr, 0);
std::vector<const net::ReportingReport*> reports;
reporting_cache->GetReports(&reports);
@@ -1601,9 +1604,11 @@
reporting_service.get());
GURL domain1("http://192.168.0.1");
- reporting_service->QueueReport(domain1, "group", "type", nullptr, 0);
+ reporting_service->QueueReport(domain1, "Mozilla/1.0", "group", "type",
+ nullptr, 0);
GURL domain2("http://192.168.0.2");
- reporting_service->QueueReport(domain2, "group", "type", nullptr, 0);
+ reporting_service->QueueReport(domain2, "Mozilla/1.0", "group", "type",
+ nullptr, 0);
std::vector<const net::ReportingReport*> reports;
reporting_cache->GetReports(&reports);
