Network Error Logging: Add request method to reports

Per https://github.com/WICG/network-error-logging/issues/73, we need to
add the request method to NEL reports.

Bug: 748549
Change-Id: I306ce389612a8ac794be732a47744b18db8cb4e2
Reviewed-on: https://chromium-review.googlesource.com/1118525Reviewed-by: Misha Efimov <mef@chromium.org>
Commit-Queue: Douglas Creager <dcreager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575635}

chrome/browser/net/reporting_browsertest.cc

@@ -148,6 +148,7 @@ IN_PROC_BROWSER_TEST_F(ReportingBrowserTest, TestReportingHeadersProcessed) {
               "referrer": "",
               "sampling_fraction": 1.0,
               "server_ip": "127.0.0.1",
+              "method": "GET",
               "status_code": 204,
               "phase": "application",
               "type": "ok",

net/network_error_logging/network_error_logging_service.cc

@@ -551,6 +551,7 @@ class NetworkErrorLoggingServiceImpl : public NetworkErrorLoggingService {
     body->SetDouble(kSamplingFractionKey, sampling_fraction);
     body->SetString(kServerIpKey, details.server_ip.ToString());
     body->SetString(kProtocolKey, details.protocol);
+    body->SetString(kMethodKey, details.method);
     body->SetInteger(kStatusCodeKey, details.status_code);
     body->SetInteger(kElapsedTimeKey, details.elapsed_time.InMilliseconds());
     body->SetString(kPhaseKey, phase);
@@ -591,6 +592,7 @@ const char NetworkErrorLoggingService::kSamplingFractionKey[] =
     "sampling_fraction";
 const char NetworkErrorLoggingService::kServerIpKey[] = "server_ip";
 const char NetworkErrorLoggingService::kProtocolKey[] = "protocol";
+const char NetworkErrorLoggingService::kMethodKey[] = "method";
 const char NetworkErrorLoggingService::kStatusCodeKey[] = "status_code";
 const char NetworkErrorLoggingService::kElapsedTimeKey[] = "elapsed_time";
 const char NetworkErrorLoggingService::kPhaseKey[] = "phase";

net/network_error_logging/network_error_logging_service.h

@@ -55,6 +55,7 @@ class NET_EXPORT NetworkErrorLoggingService {
     GURL referrer;
     IPAddress server_ip;
     std::string protocol;
+    std::string method;
     int status_code;
     base::TimeDelta elapsed_time;
     Error type;
@@ -82,6 +83,7 @@ class NET_EXPORT NetworkErrorLoggingService {
   static const char kSamplingFractionKey[];
   static const char kServerIpKey[];
   static const char kProtocolKey[];
+  static const char kMethodKey[];
   static const char kStatusCodeKey[];
   static const char kElapsedTimeKey[];
   static const char kPhaseKey[];

net/network_error_logging/network_error_logging_service_unittest.cc

@@ -132,6 +132,7 @@ class NetworkErrorLoggingServiceTest : public ::testing::Test {
   NetworkErrorLoggingService::RequestDetails MakeRequestDetails(
       GURL url,
       Error error_type,
+      std::string method = "GET",
       int status_code = 0,
       IPAddress server_ip = IPAddress()) {
     NetworkErrorLoggingService::RequestDetails details;
@@ -139,6 +140,7 @@ class NetworkErrorLoggingServiceTest : public ::testing::Test {
     details.uri = url;
     details.referrer = kReferrer_;
     details.server_ip = server_ip.IsValid() ? server_ip : kServerIP_;
+    details.method = std::move(method);
     details.status_code = status_code;
     details.elapsed_time = base::TimeDelta::FromSeconds(1);
     details.type = error_type;
@@ -269,6 +271,8 @@ TEST_F(NetworkErrorLoggingServiceTest, SuccessReportQueued) {
                               NetworkErrorLoggingService::kServerIpKey);
   base::ExpectDictStringValue("", *body,
                               NetworkErrorLoggingService::kProtocolKey);
+  base::ExpectDictStringValue("GET", *body,
+                              NetworkErrorLoggingService::kMethodKey);
   base::ExpectDictIntegerValue(0, *body,
                                NetworkErrorLoggingService::kStatusCodeKey);
   base::ExpectDictIntegerValue(1000, *body,
@@ -305,6 +309,8 @@ TEST_F(NetworkErrorLoggingServiceTest, FailureReportQueued) {
                               NetworkErrorLoggingService::kServerIpKey);
   base::ExpectDictStringValue("", *body,
                               NetworkErrorLoggingService::kProtocolKey);
+  base::ExpectDictStringValue("GET", *body,
+                              NetworkErrorLoggingService::kMethodKey);
   base::ExpectDictIntegerValue(0, *body,
                                NetworkErrorLoggingService::kStatusCodeKey);
   base::ExpectDictIntegerValue(1000, *body,
@@ -320,7 +326,7 @@ TEST_F(NetworkErrorLoggingServiceTest, HttpErrorReportQueued) {
       "{\"report_to\":\"group\",\"max_age\":86400,\"failure_fraction\":1.0}";
   service()->OnHeader(kOrigin_, kServerIP_, kHeaderFailureFraction1);
 
-  service()->OnRequest(MakeRequestDetails(kUrl_, OK, 504));
+  service()->OnRequest(MakeRequestDetails(kUrl_, OK, "GET", 504));
 
   ASSERT_EQ(1u, reports().size());
   EXPECT_EQ(kUrl_, reports()[0].url);
@@ -341,6 +347,8 @@ TEST_F(NetworkErrorLoggingServiceTest, HttpErrorReportQueued) {
                               NetworkErrorLoggingService::kServerIpKey);
   base::ExpectDictStringValue("", *body,
                               NetworkErrorLoggingService::kProtocolKey);
+  base::ExpectDictStringValue("GET", *body,
+                              NetworkErrorLoggingService::kMethodKey);
   base::ExpectDictIntegerValue(504, *body,
                                NetworkErrorLoggingService::kStatusCodeKey);
   base::ExpectDictIntegerValue(1000, *body,
@@ -356,7 +364,8 @@ TEST_F(NetworkErrorLoggingServiceTest, SuccessReportDowngraded) {
       "{\"report_to\":\"group\",\"max_age\":86400,\"success_fraction\":1.0}";
   service()->OnHeader(kOrigin_, kServerIP_, kHeaderSuccessFraction1);
 
-  service()->OnRequest(MakeRequestDetails(kUrl_, OK, 200, kOtherServerIP_));
+  service()->OnRequest(
+      MakeRequestDetails(kUrl_, OK, "GET", 200, kOtherServerIP_));
 
   ASSERT_EQ(1u, reports().size());
   EXPECT_EQ(kUrl_, reports()[0].url);
@@ -376,6 +385,8 @@ TEST_F(NetworkErrorLoggingServiceTest, SuccessReportDowngraded) {
                               NetworkErrorLoggingService::kServerIpKey);
   base::ExpectDictStringValue("", *body,
                               NetworkErrorLoggingService::kProtocolKey);
+  base::ExpectDictStringValue("GET", *body,
+                              NetworkErrorLoggingService::kMethodKey);
   base::ExpectDictIntegerValue(0, *body,
                                NetworkErrorLoggingService::kStatusCodeKey);
   base::ExpectDictIntegerValue(0, *body,
@@ -391,8 +402,8 @@ TEST_F(NetworkErrorLoggingServiceTest, FailureReportDowngraded) {
       "{\"report_to\":\"group\",\"max_age\":86400,\"success_fraction\":1.0}";
   service()->OnHeader(kOrigin_, kServerIP_, kHeaderSuccessFraction1);
 
-  service()->OnRequest(
-      MakeRequestDetails(kUrl_, ERR_CONNECTION_REFUSED, 200, kOtherServerIP_));
+  service()->OnRequest(MakeRequestDetails(kUrl_, ERR_CONNECTION_REFUSED, "GET",
+                                          200, kOtherServerIP_));
 
   ASSERT_EQ(1u, reports().size());
   EXPECT_EQ(kUrl_, reports()[0].url);
@@ -412,6 +423,8 @@ TEST_F(NetworkErrorLoggingServiceTest, FailureReportDowngraded) {
                               NetworkErrorLoggingService::kServerIpKey);
   base::ExpectDictStringValue("", *body,
                               NetworkErrorLoggingService::kProtocolKey);
+  base::ExpectDictStringValue("GET", *body,
+                              NetworkErrorLoggingService::kMethodKey);
   base::ExpectDictIntegerValue(0, *body,
                                NetworkErrorLoggingService::kStatusCodeKey);
   base::ExpectDictIntegerValue(0, *body,
@@ -427,7 +440,8 @@ TEST_F(NetworkErrorLoggingServiceTest, HttpErrorReportDowngraded) {
       "{\"report_to\":\"group\",\"max_age\":86400,\"success_fraction\":1.0}";
   service()->OnHeader(kOrigin_, kServerIP_, kHeaderSuccessFraction1);
 
-  service()->OnRequest(MakeRequestDetails(kUrl_, OK, 504, kOtherServerIP_));
+  service()->OnRequest(
+      MakeRequestDetails(kUrl_, OK, "GET", 504, kOtherServerIP_));
 
   ASSERT_EQ(1u, reports().size());
   EXPECT_EQ(kUrl_, reports()[0].url);
@@ -447,6 +461,8 @@ TEST_F(NetworkErrorLoggingServiceTest, HttpErrorReportDowngraded) {
                               NetworkErrorLoggingService::kServerIpKey);
   base::ExpectDictStringValue("", *body,
                               NetworkErrorLoggingService::kProtocolKey);
+  base::ExpectDictStringValue("GET", *body,
+                              NetworkErrorLoggingService::kMethodKey);
   base::ExpectDictIntegerValue(0, *body,
                                NetworkErrorLoggingService::kStatusCodeKey);
   base::ExpectDictIntegerValue(0, *body,
@@ -462,8 +478,8 @@ TEST_F(NetworkErrorLoggingServiceTest, DNSFailureReportNotDowngraded) {
       "{\"report_to\":\"group\",\"max_age\":86400,\"success_fraction\":1.0}";
   service()->OnHeader(kOrigin_, kServerIP_, kHeaderSuccessFraction1);
 
-  service()->OnRequest(
-      MakeRequestDetails(kUrl_, ERR_NAME_NOT_RESOLVED, 0, kOtherServerIP_));
+  service()->OnRequest(MakeRequestDetails(kUrl_, ERR_NAME_NOT_RESOLVED, "GET",
+                                          0, kOtherServerIP_));
 
   ASSERT_EQ(1u, reports().size());
   EXPECT_EQ(kUrl_, reports()[0].url);
@@ -483,6 +499,8 @@ TEST_F(NetworkErrorLoggingServiceTest, DNSFailureReportNotDowngraded) {
                               NetworkErrorLoggingService::kServerIpKey);
   base::ExpectDictStringValue("", *body,
                               NetworkErrorLoggingService::kProtocolKey);
+  base::ExpectDictStringValue("GET", *body,
+                              NetworkErrorLoggingService::kMethodKey);
   base::ExpectDictIntegerValue(0, *body,
                                NetworkErrorLoggingService::kStatusCodeKey);
   base::ExpectDictIntegerValue(1000, *body,
@@ -493,6 +511,39 @@ TEST_F(NetworkErrorLoggingServiceTest, DNSFailureReportNotDowngraded) {
                               NetworkErrorLoggingService::kTypeKey);
 }
 
+TEST_F(NetworkErrorLoggingServiceTest, SuccessPOSTReportQueued) {
+  static const std::string kHeaderSuccessFraction1 =
+      "{\"report_to\":\"group\",\"max_age\":86400,\"success_fraction\":1.0}";
+  service()->OnHeader(kOrigin_, kServerIP_, kHeaderSuccessFraction1);
+
+  service()->OnRequest(MakeRequestDetails(kUrl_, OK, "POST"));
+
+  ASSERT_EQ(1u, reports().size());
+  EXPECT_EQ(kUrl_, reports()[0].url);
+  EXPECT_EQ(kGroup_, reports()[0].group);
+  EXPECT_EQ(kType_, reports()[0].type);
+  EXPECT_EQ(0, reports()[0].depth);
+
+  const base::DictionaryValue* body;
+  ASSERT_TRUE(reports()[0].body->GetAsDictionary(&body));
+  base::ExpectDictStringValue(kUrl_.spec(), *body,
+                              NetworkErrorLoggingService::kUriKey);
+  base::ExpectDictStringValue(kReferrer_.spec(), *body,
+                              NetworkErrorLoggingService::kReferrerKey);
+  ExpectDictDoubleValue(1.0, *body,
+                        NetworkErrorLoggingService::kSamplingFractionKey);
+  base::ExpectDictStringValue(kServerIP_.ToString(), *body,
+                              NetworkErrorLoggingService::kServerIpKey);
+  base::ExpectDictStringValue("", *body,
+                              NetworkErrorLoggingService::kProtocolKey);
+  base::ExpectDictStringValue("POST", *body,
+                              NetworkErrorLoggingService::kMethodKey);
+  base::ExpectDictStringValue("application", *body,
+                              NetworkErrorLoggingService::kPhaseKey);
+  base::ExpectDictStringValue("ok", *body,
+                              NetworkErrorLoggingService::kTypeKey);
+}
+
 TEST_F(NetworkErrorLoggingServiceTest, MaxAge0) {
   service()->OnHeader(kOrigin_, kServerIP_, kHeader_);
 

net/url_request/url_request.cc

@@ -1199,6 +1199,7 @@ void URLRequest::MaybeGenerateNetworkErrorLoggingReport() {
   }
   if (response_info().was_alpn_negotiated)
     details.protocol = response_info().alpn_negotiated_protocol;
+  details.method = method();
   details.elapsed_time =
       base::TimeTicks::Now() - load_timing_info_.request_start;
   details.type = status().ToNetError();