[Backport] Don't clear DnsAttempts that have received a response

When a DnsTransaction finishes synchronously, it posts a task to run its
callback. In the meantime, DnsAttempts can keep running, and if a TCP
attempt starts, it will delete all the previous attempts. Then the
callback will run and use an attempt which was just deleted.

This fix is designed to be easy to merge to branches.

Bug: 788131, 793099
Change-Id: I768418dfdff70f88454b6daa9c6f4b8b0639619a
Reviewed-on: https://chromium-review.googlesource.com/817681
Reviewed-by: Julia Tuttle <juliatuttle@chromium.org>
Commit-Queue: Miriam Gershenson <mgersh@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#522883}(cherry picked from commit 9069772b10e2796e4a09d6248a81b3c4ea4506d5)
Reviewed-on: https://chromium-review.googlesource.com/822891
Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#664}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

2 files changed, 57 insertions, 2 deletions

diff --git a/chromium/net/dns/dns_transaction.cc b/chromium/net/dns/dns_transaction.cc
index 1bc1f29c99e..828f4e98efa 100644
--- a/chromium/net/dns/dns_transaction.cc
+++ b/chromium/net/dns/dns_transaction.cc
@@ -771,8 +771,15 @@ class DnsTransactionImpl : public DnsTransaction,
         previous_attempt->GetQuery()->CloneWithNewId(id);
 
     RecordLostPacketsIfAny();
-    // Cancel all other attempts, no point waiting on them.
-    attempts_.clear();
+
+    // Cancel all other attempts that have not received a response, no point
+    // waiting on them.
+    for (auto it = attempts_.begin(); it != attempts_.end();) {
+      if (!(*it)->is_completed())
+        it = attempts_.erase(it);
+      else
+        ++it;
+    }
 
     unsigned attempt_number = attempts_.size();
 
diff --git a/chromium/net/dns/dns_transaction_unittest.cc b/chromium/net/dns/dns_transaction_unittest.cc
index c1a7b681c3b..9750121959c 100644
--- a/chromium/net/dns/dns_transaction_unittest.cc
+++ b/chromium/net/dns/dns_transaction_unittest.cc
@@ -995,6 +995,54 @@ TEST_F(DnsTransactionTest, TCPConnectionClosedSynchronous) {
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 }
 
+TEST_F(DnsTransactionTest, MismatchedThenNxdomainThenTCP) {
+  config_.attempts = 2;
+  config_.timeout = TestTimeouts::tiny_timeout();
+  ConfigureFactory();
+  std::unique_ptr<DnsSocketData> data(
+      new DnsSocketData(0 /* id */, kT0HostName, kT0Qtype, SYNCHRONOUS, false));
+  // First attempt gets a mismatched response.
+  data->AddResponseData(kT1ResponseDatagram, arraysize(kT1ResponseDatagram),
+                        SYNCHRONOUS);
+  // Second read from first attempt gets TCP required.
+  data->AddRcode(dns_protocol::kFlagTC, ASYNC);
+  AddSocketData(std::move(data));
+  // Second attempt gets NXDOMAIN, which happens before the TCP required.
+  AddSyncQueryAndRcode(kT0HostName, kT0Qtype, dns_protocol::kRcodeNXDOMAIN);
+  std::unique_ptr<DnsSocketData> tcp_data(
+      new DnsSocketData(0 /* id */, kT0HostName, kT0Qtype, ASYNC, true));
+  tcp_data->AddReadError(ERR_CONNECTION_CLOSED, SYNCHRONOUS);
+  AddSocketData(std::move(tcp_data));
+
+  TransactionHelper helper0(kT0HostName, kT0Qtype, ERR_NAME_NOT_RESOLVED);
+  EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
+}
+
+TEST_F(DnsTransactionTest, MismatchedThenOkThenTCP) {
+  config_.attempts = 2;
+  config_.timeout = TestTimeouts::tiny_timeout();
+  ConfigureFactory();
+  std::unique_ptr<DnsSocketData> data(
+      new DnsSocketData(0 /* id */, kT0HostName, kT0Qtype, SYNCHRONOUS, false));
+  // First attempt gets a mismatched response.
+  data->AddResponseData(kT1ResponseDatagram, arraysize(kT1ResponseDatagram),
+                        SYNCHRONOUS);
+  // Second read from first attempt gets TCP required.
+  data->AddRcode(dns_protocol::kFlagTC, ASYNC);
+  AddSocketData(std::move(data));
+  // Second attempt gets a valid response, which happens before the TCP
+  // required.
+  AddSyncQueryAndResponse(0 /* id */, kT0HostName, kT0Qtype,
+                          kT0ResponseDatagram, arraysize(kT0ResponseDatagram));
+  std::unique_ptr<DnsSocketData> tcp_data(
+      new DnsSocketData(0 /* id */, kT0HostName, kT0Qtype, ASYNC, true));
+  tcp_data->AddReadError(ERR_CONNECTION_CLOSED, SYNCHRONOUS);
+  AddSocketData(std::move(tcp_data));
+
+  TransactionHelper helper0(kT0HostName, kT0Qtype, kT0RecordCount);
+  EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
+}
+
 TEST_F(DnsTransactionTest, InvalidQuery) {
   config_.timeout = TestTimeouts::tiny_timeout();
   ConfigureFactory();