From c52ce5c11e9bd2e0581e05771eb42df018d59b1c Mon Sep 17 00:00:00 2001
From: Shunya Shishido <[email protected]>
Date: Thu, 02 Jul 2026 21:13:47 -0700
Subject: [PATCH] NEL: consider all attempted addresses in report downgrade

When a hostname resolves to multiple addresses,
HttpNetworkTransaction::GenerateNetworkErrorLoggingReport() only
reported the last attempted address as RequestDetails::server_ip.
NetworkErrorLoggingService then compared only that single address
against the policy's received_ip_address when deciding whether to
downgrade the report to dns.address_changed, so the downgrade was
skipped whenever the last attempted address happened to match the policy
address even though other addresses had also been contacted.

This CL plumbs the full set of contacted addresses through to the NEL
service:

* RequestDetails gains an `other_server_ips` field.
* HttpNetworkTransaction populates it from
  `connection_attempts_`.
* NetworkErrorLoggingService downgrades the report if
  `server_ip` or any of `other_server_ips` differs from the
  policy's `received_ip_address`.

TAG=agy
CONV=0627b279-d6cf-4c08-953e-131bdc2b4ee7

Bug: 500492844
Change-Id: I936dd6802a1552db699fc2324bd9288338614143
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8030100
Reviewed-by: Adam Rice <[email protected]>
Commit-Queue: Shunya Shishido <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1656349}
---

diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index f36a98f4..129fe65 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -1942,6 +1942,14 @@
   } else {
     details.server_ip = IPAddress();
   }
+  // Also report any other addresses that were contacted, so that the downgrade
+  // step can take all of them into account when the resolved address list
+  // contained more than one address.
+  for (const auto& attempt : connection_attempts_) {
+    if (attempt.endpoint.address() != details.server_ip) {
+      details.other_server_ips.push_back(attempt.endpoint.address());
+    }
+  }
   // HttpResponseHeaders::response_code() returns 0 if response code couldn't
   // be parsed, which is also how NEL represents the same.
   if (response_.headers) {
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 0ee6912..7de336b 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -24738,6 +24738,44 @@
 }
 
 TEST_P(HttpNetworkTransactionNetworkErrorLoggingTest,
+       CreateReportErrorAfterStartMultipleAddresses) {
+  const IPAddress kFirstAddress(1, 2, 3, 4);
+  const IPAddress kSecondAddress(5, 6, 7, 8);
+  session_deps_.host_resolver->rules()->AddRule(GURL(url_).GetHost(),
+                                                "1.2.3.4,5.6.7.8");
+
+  std::unique_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
+  auto trans =
+      std::make_unique<HttpNetworkTransaction>(DEFAULT_PRIORITY, session.get());
+
+  StaticSocketDataProvider data1;
+  data1.set_connect_data(MockConnect(SYNCHRONOUS, ERR_CONNECTION_REFUSED));
+  session_deps_.socket_factory->AddSocketDataProvider(&data1);
+  StaticSocketDataProvider data2;
+  data2.set_connect_data(MockConnect(SYNCHRONOUS, ERR_CONNECTION_REFUSED));
+  session_deps_.socket_factory->AddSocketDataProvider(&data2);
+
+  TestCompletionCallback callback;
+
+  int rv = trans->Start(&request_, callback.callback(), NetLogWithSource());
+  EXPECT_THAT(callback.GetResult(rv), IsError(ERR_CONNECTION_REFUSED));
+
+  trans.reset();
+
+  ASSERT_EQ(1u, network_error_logging_service()->errors().size());
+  const NetworkErrorLoggingService::RequestDetails& error =
+      network_error_logging_service()->errors()[0];
+  EXPECT_EQ(0, error.status_code);
+  EXPECT_EQ(ERR_CONNECTION_REFUSED, error.type);
+  // Both resolved addresses were attempted, so both should be reported: one as
+  // `server_ip` and the other in `other_server_ips`.
+  std::vector<IPAddress> all_ips = error.other_server_ips;
+  all_ips.push_back(error.server_ip);
+  EXPECT_THAT(all_ips,
+              testing::UnorderedElementsAre(kFirstAddress, kSecondAddress));
+}
+
+TEST_P(HttpNetworkTransactionNetworkErrorLoggingTest,
        CreateReportReadBodyError) {
   std::string extra_header_string = extra_headers_.ToString();
   MockWrite data_writes[] = {
diff --git a/net/network_error_logging/network_error_logging_service.cc b/net/network_error_logging/network_error_logging_service.cc
index 4916183e..7006226 100644
--- a/net/network_error_logging/network_error_logging_service.cc
+++ b/net/network_error_logging/network_error_logging_service.cc
@@ -486,9 +486,16 @@
     // If the server that handled the request is different than the server that
     // delivered the NEL policy (as determined by their IP address), then we
     // have to "downgrade" the NEL report, so that it only includes information
-    // about DNS resolution.
-    if (phase_string != kDnsPhase && details.server_ip.IsValid() &&
-        details.server_ip != policy->received_ip_address) {
+    // about DNS resolution. This also applies if any other address contacted
+    // during the request differs from the policy's address, since the report
+    // would otherwise reflect the behaviour of those addresses too.
+    bool server_ip_changed =
+        (details.server_ip.IsValid() &&
+         details.server_ip != policy->received_ip_address) ||
+        std::ranges::any_of(details.other_server_ips, [&](const auto& ip) {
+          return ip != policy->received_ip_address;
+        });
+    if (phase_string != kDnsPhase && server_ip_changed) {
       phase_string = kDnsPhase;
       type_string = kDnsAddressChangedType;
       details.elapsed_time = base::TimeDelta();
diff --git a/net/network_error_logging/network_error_logging_service.h b/net/network_error_logging/network_error_logging_service.h
index 913f94e..2406287 100644
--- a/net/network_error_logging/network_error_logging_service.h
+++ b/net/network_error_logging/network_error_logging_service.h
@@ -129,6 +129,13 @@
     GURL referrer;
     std::string user_agent;
     IPAddress server_ip;
+    // Addresses other than `server_ip` that were also contacted while
+    // establishing the connection (e.g., earlier addresses in the resolved
+    // address list that the socket layer attempted before falling back). Not
+    // included in the uploaded report. Used when deciding whether to downgrade
+    // the report: the report is downgraded if any of these differ from the
+    // policy's `received_ip_address`.
+    std::vector<IPAddress> other_server_ips;
     std::string protocol;
     std::string method;
     int status_code;
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 06b65d9..a8420a8 100644
--- a/net/network_error_logging/network_error_logging_service_unittest.cc
+++ b/net/network_error_logging/network_error_logging_service_unittest.cc
@@ -705,6 +705,68 @@
                    "dns.address_changed"))));
 }
 
+TEST_P(NetworkErrorLoggingServiceTest, FailureReportDowngradedOtherServerIp) {
+  service()->OnHeader(kNak_, kOrigin_, kServerIP_, kHeaderSuccessFraction1_);
+
+  // Make the rest of the test run synchronously.
+  FinishLoading(/*load_success=*/true);
+
+  // `server_ip` matches the policy's address, but the request also contacted
+  // a different address. The report should still be downgraded.
+  NetworkErrorLoggingService::RequestDetails details = MakeRequestDetails(
+      kNak_, kUrl_, ERR_CONNECTION_REFUSED, "GET", 0, kServerIP_);
+  details.other_server_ips = {kOtherServerIP_};
+  service()->OnRequest(std::move(details));
+
+  ASSERT_EQ(1u, reports().size());
+  EXPECT_EQ(kUrl_, reports()[0].url);
+  EXPECT_EQ(kNak_, reports()[0].network_anonymization_key);
+  EXPECT_EQ(kGroup_, reports()[0].group);
+  EXPECT_EQ(kType_, reports()[0].type);
+  EXPECT_EQ(0, reports()[0].depth);
+
+  EXPECT_THAT(
+      reports()[0].body,
+      Pointee(IsSupersetOfValue(
+          base::DictValue()
+              .Set(NetworkErrorLoggingService::kReferrerKey, kReferrer_.spec())
+              .Set(NetworkErrorLoggingService::kSamplingFractionKey, 1.0)
+              .Set(NetworkErrorLoggingService::kServerIpKey,
+                   kServerIP_.ToString())
+              .Set(NetworkErrorLoggingService::kProtocolKey, "")
+              .Set(NetworkErrorLoggingService::kMethodKey, "GET")
+              .Set(NetworkErrorLoggingService::kStatusCodeKey, 0)
+              .Set(NetworkErrorLoggingService::kElapsedTimeKey, 0)
+              .Set(NetworkErrorLoggingService::kPhaseKey, "dns")
+              .Set(NetworkErrorLoggingService::kTypeKey,
+                   "dns.address_changed"))));
+}
+
+TEST_P(NetworkErrorLoggingServiceTest,
+       FailureReportNotDowngradedSameOtherServerIp) {
+  service()->OnHeader(kNak_, kOrigin_, kServerIP_, kHeaderSuccessFraction1_);
+
+  // Make the rest of the test run synchronously.
+  FinishLoading(/*load_success=*/true);
+
+  // `server_ip` matches the policy's address, and the only other contacted
+  // address is the same. The report should not be downgraded.
+  NetworkErrorLoggingService::RequestDetails details = MakeRequestDetails(

diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 0ee6912..7de336b 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -24738,6 +24738,44 @@
 }
 
 TEST_P(HttpNetworkTransactionNetworkErrorLoggingTest,
+       CreateReportErrorAfterStartMultipleAddresses) {
+  const IPAddress kFirstAddress(1, 2, 3, 4);
+  const IPAddress kSecondAddress(5, 6, 7, 8);
+  session_deps_.host_resolver->rules()->AddRule(GURL(url_).GetHost(),
+                                                "1.2.3.4,5.6.7.8");
+
+  std::unique_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
+  auto trans =
+      std::make_unique<HttpNetworkTransaction>(DEFAULT_PRIORITY, session.get());
+
+  StaticSocketDataProvider data1;
+  data1.set_connect_data(MockConnect(SYNCHRONOUS, ERR_CONNECTION_REFUSED));
+  session_deps_.socket_factory->AddSocketDataProvider(&data1);
+  StaticSocketDataProvider data2;
+  data2.set_connect_data(MockConnect(SYNCHRONOUS, ERR_CONNECTION_REFUSED));
+  session_deps_.socket_factory->AddSocketDataProvider(&data2);
+
+  TestCompletionCallback callback;
+
+  int rv = trans->Start(&request_, callback.callback(), NetLogWithSource());
+  EXPECT_THAT(callback.GetResult(rv), IsError(ERR_CONNECTION_REFUSED));
+
+  trans.reset();
+
+  ASSERT_EQ(1u, network_error_logging_service()->errors().size());
+  const NetworkErrorLoggingService::RequestDetails& error =
+      network_error_logging_service()->errors()[0];
+  EXPECT_EQ(0, error.status_code);
+  EXPECT_EQ(ERR_CONNECTION_REFUSED, error.type);
+  // Both resolved addresses were attempted, so both should be reported: one as
+  // `server_ip` and the other in `other_server_ips`.
+  std::vector<IPAddress> all_ips = error.other_server_ips;
+  all_ips.push_back(error.server_ip);
+  EXPECT_THAT(all_ips,
+              testing::UnorderedElementsAre(kFirstAddress, kSecondAddress));
+}
+
+TEST_P(HttpNetworkTransactionNetworkErrorLoggingTest,
        CreateReportReadBodyError) {
   std::string extra_header_string = extra_headers_.ToString();
   MockWrite data_writes[] = {
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 06b65d9..a8420a8 100644
--- a/net/network_error_logging/network_error_logging_service_unittest.cc
+++ b/net/network_error_logging/network_error_logging_service_unittest.cc
@@ -705,6 +705,68 @@
                    "dns.address_changed"))));
 }
 
+TEST_P(NetworkErrorLoggingServiceTest, FailureReportDowngradedOtherServerIp) {
+  service()->OnHeader(kNak_, kOrigin_, kServerIP_, kHeaderSuccessFraction1_);
+
+  // Make the rest of the test run synchronously.
+  FinishLoading(/*load_success=*/true);
+
+  // `server_ip` matches the policy's address, but the request also contacted
+  // a different address. The report should still be downgraded.
+  NetworkErrorLoggingService::RequestDetails details = MakeRequestDetails(
+      kNak_, kUrl_, ERR_CONNECTION_REFUSED, "GET", 0, kServerIP_);
+  details.other_server_ips = {kOtherServerIP_};
+  service()->OnRequest(std::move(details));
+
+  ASSERT_EQ(1u, reports().size());
+  EXPECT_EQ(kUrl_, reports()[0].url);
+  EXPECT_EQ(kNak_, reports()[0].network_anonymization_key);
+  EXPECT_EQ(kGroup_, reports()[0].group);
+  EXPECT_EQ(kType_, reports()[0].type);
+  EXPECT_EQ(0, reports()[0].depth);
+
+  EXPECT_THAT(
+      reports()[0].body,
+      Pointee(IsSupersetOfValue(
+          base::DictValue()
+              .Set(NetworkErrorLoggingService::kReferrerKey, kReferrer_.spec())
+              .Set(NetworkErrorLoggingService::kSamplingFractionKey, 1.0)
+              .Set(NetworkErrorLoggingService::kServerIpKey,
+                   kServerIP_.ToString())
+              .Set(NetworkErrorLoggingService::kProtocolKey, "")
+              .Set(NetworkErrorLoggingService::kMethodKey, "GET")
+              .Set(NetworkErrorLoggingService::kStatusCodeKey, 0)
+              .Set(NetworkErrorLoggingService::kElapsedTimeKey, 0)
+              .Set(NetworkErrorLoggingService::kPhaseKey, "dns")
+              .Set(NetworkErrorLoggingService::kTypeKey,
+                   "dns.address_changed"))));
+}
+
+TEST_P(NetworkErrorLoggingServiceTest,
+       FailureReportNotDowngradedSameOtherServerIp) {
+  service()->OnHeader(kNak_, kOrigin_, kServerIP_, kHeaderSuccessFraction1_);
+
+  // Make the rest of the test run synchronously.
+  FinishLoading(/*load_success=*/true);
+
+  // `server_ip` matches the policy's address, and the only other contacted
+  // address is the same. The report should not be downgraded.
+  NetworkErrorLoggingService::RequestDetails details = MakeRequestDetails(
+      kNak_, kUrl_, ERR_CONNECTION_REFUSED, "GET", 0, kServerIP_);
+  details.other_server_ips = {kServerIP_};
+  service()->OnRequest(std::move(details));
+
+  ASSERT_EQ(1u, reports().size());
+  EXPECT_THAT(
+      reports()[0].body,
+      Pointee(IsSupersetOfValue(
+          base::DictValue()
+              .Set(NetworkErrorLoggingService::kStatusCodeKey, 0)
+              .Set(NetworkErrorLoggingService::kElapsedTimeKey, 1000)
+              .Set(NetworkErrorLoggingService::kPhaseKey, "connection")
+              .Set(NetworkErrorLoggingService::kTypeKey, "tcp.refused"))));
+}
+
 TEST_P(NetworkErrorLoggingServiceTest, HttpErrorReportDowngraded) {
   service()->OnHeader(kNak_, kOrigin_, kServerIP_, kHeaderSuccessFraction1_);