[net/dns] Don't abandon a DnsUDPAttempt when the response does not match the query.

net/dns/dns_transaction_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/dns/dns_transaction.h"
 
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/rand_util.h"
@@ skipping 13 matching lines @@
 namespace net {
 
 namespace {
 
 std::string DomainFromDot(const base::StringPiece& dotted) {
   std::string out;
   EXPECT_TRUE(DNSDomainFromDot(dotted, &out));
   return out;
 }
 
+// A SocketDataProvider builder for MockUDPClientSocket. Each socket used by a
+// DnsTransaction expects only one write and zero or more reads.
+class DnsSocketData {
+ public:
+  // The ctor takes parameters for the DnsQuery.
+  DnsSocketData(const char* dotted_name, uint16 qtype, uint16 id, IoMode mode)
+      : query_(new DnsQuery(id, DomainFromDot(dotted_name), qtype)),
+        write_(mode, query_->io_buffer()->data(), query_->io_buffer()->size()) {
+  }
+  ~DnsSocketData() {}
+
+  // All responses must be added before GetProvider.
+  void AddResponseData(const char* data, size_t length, IoMode mode) {
+    CHECK(!provider_.get());
+    AddResponse(make_scoped_ptr(new DnsResponse(data, length, 0)), mode);
+  }
+
+  void AddRcode(int rcode, IoMode mode) {

[mmenke, 2012/08/29 16:23:57]

This and AddResponse should have comments - particularly this one.

+    scoped_ptr<DnsResponse> response(
+        new DnsResponse(query_->io_buffer()->data(),
+                        query_->io_buffer()->size(),
+                        0));
+    dns_protocol::Header* header =
+        reinterpret_cast<dns_protocol::Header*>(response->io_buffer()->data());
+    header->flags |= base::HostToNet16(dns_protocol::kFlagResponse | rcode);
+    AddResponse(response.Pass(), mode);
+  }
+
+  void AddResponse(scoped_ptr<DnsResponse> response, IoMode mode) {

[mmenke, 2012/08/29 16:23:57]

nit:  Suggest you put this before AddRcode, since that's the same order the
corresponding functions appear in below.

+    CHECK(!provider_.get());
+    reads_.push_back(MockRead(mode,
+                              response->io_buffer()->data(),
+                              response->io_buffer()->size()));
+    responses_.push_back(response.release());
+  }
+
+  SocketDataProvider* GetProvider() {
+    if (provider_.get())
+      return provider_.get();
+    if (reads_.empty()) {
+      // Timeout.
+      provider_.reset(new DelayedSocketData(2, NULL, 0, &write_, 1));
+    } else {
+      provider_.reset(new DelayedSocketData(1, &reads_[0], reads_.size(),
+                                            &write_, 1));
+    }
+    return provider_.get();
+  }
+
+  uint16 query_id() const {
+    return query_->id();
+  }
+
+  bool got_written() const {

[mmenke, 2012/08/29 16:23:57]

nit:  Suggest was_written(), or maybe query_was_written().

+    CHECK(provider_.get());
+    return provider_->write_index() > 0;
+  }
+
+ private:
+  scoped_ptr<DnsQuery> query_;
+  ScopedVector<DnsResponse> responses_;
+  MockWrite write_;
+  std::vector<MockRead> reads_;
+  scoped_ptr<DelayedSocketData> provider_;
+
+  DISALLOW_COPY_AND_ASSIGN(DnsSocketData);
+};
+
 class TestSocketFactory;
 
 // A variant of MockUDPClientSocket which notifies the factory OnConnect.
 class TestUDPClientSocket : public MockUDPClientSocket {
  public:
   TestUDPClientSocket(TestSocketFactory* factory,
                       SocketDataProvider* data,
                       net::NetLog* net_log)
       : MockUDPClientSocket(data, net_log), factory_(factory) {
   }
   virtual ~TestUDPClientSocket() {}
   virtual int Connect(const IPEndPoint& endpoint) OVERRIDE;
  private:

[mmenke, 2012/08/29 16:23:57]

While you're here, there should be a blank line above private.

   TestSocketFactory* factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestUDPClientSocket);
 };
 
 // Creates TestUDPClientSockets and keeps endpoints reported via OnConnect.
 class TestSocketFactory : public MockClientSocketFactory {
  public:
   TestSocketFactory() {}
   virtual ~TestSocketFactory() {}
 
   virtual DatagramClientSocket* CreateDatagramClientSocket(
       DatagramSocket::BindType bind_type,
       const RandIntCallback& rand_int_cb,
       net::NetLog* net_log,
       const net::NetLog::Source& source) OVERRIDE {
     SocketDataProvider* data_provider = mock_data().GetNext();
     TestUDPClientSocket* socket = new TestUDPClientSocket(this,
                                                           data_provider,
                                                           net_log);
     data_provider->set_socket(socket);
     return socket;
   }
 
   void OnConnect(const IPEndPoint& endpoint) {
     remote_endpoints.push_back(endpoint);
   }
 
   std::vector<IPEndPoint> remote_endpoints;
+
+  DISALLOW_COPY_AND_ASSIGN(TestSocketFactory);
 };
 
 int TestUDPClientSocket::Connect(const IPEndPoint& endpoint) {
   factory_->OnConnect(endpoint);
   return MockUDPClientSocket::Connect(endpoint);
 }
 
 // Helper class that holds a DnsTransaction and handles OnTransactionComplete.
 class TransactionHelper {
  public:
@@ skipping 48 matching lines @@
     EXPECT_FALSE(completed_);
     EXPECT_EQ(transaction_.get(), t);
 
     completed_ = true;
 
     if (cancel_in_callback_) {
       Cancel();
       return;
     }
 
+    if (quit_in_callback_)

[mmenke, 2012/08/29 16:23:57]

Please add a comment that this needs to be done before the asserts below.

+      MessageLoop::current()->Quit();
+
     if (expected_answer_count_ >= 0) {
-      EXPECT_EQ(OK, rv);
+      ASSERT_EQ(OK, rv);
+      ASSERT_TRUE(response != NULL);
       EXPECT_EQ(static_cast<unsigned>(expected_answer_count_),
                 response->answer_count());
       EXPECT_EQ(qtype_, response->qtype());
 
       DnsRecordParser parser = response->Parser();
       DnsResourceRecord record;
       for (int i = 0; i < expected_answer_count_; ++i) {
         EXPECT_TRUE(parser.ReadRecord(&record));
       }
     } else {
       EXPECT_EQ(expected_answer_count_, rv);
     }
-
-    if (quit_in_callback_)
-      MessageLoop::current()->Quit();
   }
 
   bool has_completed() const {
     return completed_;
   }
 
   // Shorthands for commonly used commands.
 
   bool Run(DnsTransactionFactory* factory) {
     StartTransaction(factory);
@@ skipping 44 matching lines @@
   void ConfigureFactory() {
     socket_factory_.reset(new TestSocketFactory());
     session_ = new DnsSession(
         config_,
         socket_factory_.get(),
         base::Bind(&DnsTransactionTest::GetNextId, base::Unretained(this)),
         NULL /* NetLog */);
     transaction_factory_ = DnsTransactionFactory::CreateFactory(session_.get());
   }
 
-  // Each socket used by a DnsTransaction expects only one write and zero or one
-  // reads.
+  void AddSocketData(scoped_ptr<DnsSocketData> data) {
+    transaction_ids_.push_back(data->query_id());
+    socket_factory_->AddSocketDataProvider(data->GetProvider());
+    socket_data_.push_back(data.release());
+  }
 
   // Add expected query for |dotted_name| and |qtype| with |id| and response
   // taken verbatim from |data| of |data_length| bytes. The transaction id in
   // |data| should equal |id|, unless testing mismatched response.
-  void AddResponse(const std::string& dotted_name,
+  void AddResponse(const char* dotted_name,

[mmenke, 2012/08/29 16:23:57]

While you're here, could you rename this?  The comment is accurate, but the name
is misleading.  AddQueryAndResponse, maybe?

                    uint16 qtype,
                    uint16 id,
-                   const char* data,
-                   size_t data_length,
+                   const char* response_data,
+                   size_t response_length,
                    IoMode mode) {
     CHECK(socket_factory_.get());
-    DnsQuery* query = new DnsQuery(id, DomainFromDot(dotted_name), qtype);
-    queries_.push_back(query);
-
-    // The response is only used to hold the IOBuffer.
-    DnsResponse* response = new DnsResponse(data, data_length, 0);
-    responses_.push_back(response);
-
-    writes_.push_back(MockWrite(mode,
-                                query->io_buffer()->data(),
-                                query->io_buffer()->size()));
-    reads_.push_back(MockRead(mode,
-                              response->io_buffer()->data(),
-                              data_length));
-
-    transaction_ids_.push_back(id);
+    scoped_ptr<DnsSocketData> data(
+        new DnsSocketData(dotted_name, qtype, id, mode));
+    data->AddResponseData(response_data, response_length, mode);
+    AddSocketData(data.Pass());
   }
 
-  void AddAsyncResponse(const std::string& dotted_name,
+  void AddAsyncResponse(const char* dotted_name,
                         uint16 qtype,
                         uint16 id,
                         const char* data,
                         size_t data_length) {
     AddResponse(dotted_name, qtype, id, data, data_length, ASYNC);
   }
 
+  void AddSyncResponse(const char* dotted_name,
+                       uint16 qtype,
+                       uint16 id,
+                       const char* data,
+                       size_t data_length) {
+    AddResponse(dotted_name, qtype, id, data, data_length, SYNCHRONOUS);
+  }
+
   // Add expected query of |dotted_name| and |qtype| and no response.
   void AddTimeout(const char* dotted_name, uint16 qtype) {
     CHECK(socket_factory_.get());
     uint16 id = base::RandInt(0, kuint16max);
-    DnsQuery* query = new DnsQuery(id, DomainFromDot(dotted_name), qtype);
-    queries_.push_back(query);
-
-    writes_.push_back(MockWrite(ASYNC,
-                                query->io_buffer()->data(),
-                                query->io_buffer()->size()));
-    // Empty MockRead indicates no data.
-    reads_.push_back(MockRead());
-    transaction_ids_.push_back(id);
+    scoped_ptr<DnsSocketData> data(
+        new DnsSocketData(dotted_name, qtype, id, ASYNC));
+    AddSocketData(data.Pass());
   }
 
   // Add expected query of |dotted_name| and |qtype| and response with no answer
   // and rcode set to |rcode|.
   void AddRcode(const char* dotted_name, uint16 qtype, int rcode, IoMode mode) {

[mmenke, 2012/08/29 16:23:57]

Same comment as above.  AddQueryAndRcodeResponse, maybe?

     CHECK(socket_factory_.get());
     CHECK_NE(dns_protocol::kRcodeNOERROR, rcode);
     uint16 id = base::RandInt(0, kuint16max);
-    DnsQuery* query = new DnsQuery(id, DomainFromDot(dotted_name), qtype);
-    queries_.push_back(query);
-
-    DnsResponse* response = new DnsResponse(query->io_buffer()->data(),
-                                            query->io_buffer()->size(),
-                                            0);
-    dns_protocol::Header* header =
-        reinterpret_cast<dns_protocol::Header*>(response->io_buffer()->data());
-    header->flags |= base::HostToNet16(dns_protocol::kFlagResponse | rcode);
-    responses_.push_back(response);
-
-    writes_.push_back(MockWrite(mode,
-                                query->io_buffer()->data(),
-                                query->io_buffer()->size()));
-    reads_.push_back(MockRead(mode,
-                              response->io_buffer()->data(),
-                              query->io_buffer()->size()));
-    transaction_ids_.push_back(id);
+    scoped_ptr<DnsSocketData> data(
+        new DnsSocketData(dotted_name, qtype, id, mode));
+    data->AddRcode(rcode, mode);
+    AddSocketData(data.Pass());
   }
 
   void AddAsyncRcode(const char* dotted_name, uint16 qtype, int rcode) {
     AddRcode(dotted_name, qtype, rcode, ASYNC);
   }
 
-  // Call after all Add* calls to prepare data for |socket_factory_|.
-  // This separation is necessary because the |reads_| and |writes_| vectors
-  // could reallocate their data during those calls.
-  void PrepareSockets() {
-    CHECK_EQ(writes_.size(), reads_.size());
-    for (size_t i = 0; i < writes_.size(); ++i) {
-      DelayedSocketData* data;
-      if (reads_[i].data) {
-        data = new DelayedSocketData(1, &reads_[i], 1, &writes_[i], 1);
-      } else {
-        // Timeout.
-        data = new DelayedSocketData(2, NULL, 0, &writes_[i], 1);
-      }
-      socket_data_.push_back(data);
-      socket_factory_->AddSocketDataProvider(data);
-    }
+  void AddSyncRcode(const char* dotted_name, uint16 qtype, int rcode) {
+    AddRcode(dotted_name, qtype, rcode, SYNCHRONOUS);
   }
 
   // Checks if the sockets were connected in the order matching the indices in
   // |servers|.
   void CheckServerOrder(const unsigned* servers, size_t num_attempts) {
     ASSERT_EQ(num_attempts, socket_factory_->remote_endpoints.size());
     for (size_t i = 0; i < num_attempts; ++i) {
       EXPECT_EQ(socket_factory_->remote_endpoints[i],
                 session_->config().nameservers[servers[i]]);
     }
   }
 
   void SetUp() OVERRIDE {
     // By default set one server,
     ConfigureNumServers(1);
     // and no retransmissions,
     config_.attempts = 1;
     // but long enough timeout for memory tests.
     config_.timeout = TestTimeouts::action_timeout();
     ConfigureFactory();
   }
 
   void TearDown() OVERRIDE {
     // Check that all socket data was at least written to.
     for (size_t i = 0; i < socket_data_.size(); ++i) {
-      EXPECT_GT(socket_data_[i]->write_index(), 0u);
+      EXPECT_TRUE(socket_data_[i]->got_written()) << i;
     }
   }
 
  protected:
   int GetNextId(int min, int max) {
     EXPECT_FALSE(transaction_ids_.empty());
     int id = transaction_ids_.front();
     transaction_ids_.pop_front();
     EXPECT_GE(id, min);
     EXPECT_LE(id, max);
     return id;
   }
 
   DnsConfig config_;
 
-  // Holders for the buffers behind MockRead/MockWrites (they do not own them).
-  ScopedVector<DnsQuery> queries_;
-  ScopedVector<DnsResponse> responses_;
-
-  // Holders for MockRead/MockWrites (SocketDataProvider does not own it).
-  std::vector<MockRead> reads_;
-  std::vector<MockWrite> writes_;
-
-  // Holder for the socket data (MockClientSocketFactory does not own it).
-  ScopedVector<DelayedSocketData> socket_data_;
+  ScopedVector<DnsSocketData> socket_data_;
 
   std::deque<int> transaction_ids_;
   scoped_ptr<TestSocketFactory> socket_factory_;
   scoped_refptr<DnsSession> session_;
   scoped_ptr<DnsTransactionFactory> transaction_factory_;
 };
 
 TEST_F(DnsTransactionTest, Lookup) {
   AddAsyncResponse(kT0HostName,
                    kT0Qtype,
                    0 /* id */,
                    reinterpret_cast<const char*>(kT0ResponseDatagram),
                    arraysize(kT0ResponseDatagram));
-  PrepareSockets();
 
   TransactionHelper helper0(kT0HostName,
                             kT0Qtype,
                             kT0RecordCount);
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 }
 
 // Concurrent lookup tests assume that DnsTransaction::Start immediately
 // consumes a socket from ClientSocketFactory.
 TEST_F(DnsTransactionTest, ConcurrentLookup) {
   AddAsyncResponse(kT0HostName,
                    kT0Qtype,
                    0 /* id */,
                    reinterpret_cast<const char*>(kT0ResponseDatagram),
                    arraysize(kT0ResponseDatagram));
   AddAsyncResponse(kT1HostName,
                    kT1Qtype,
                    1 /* id */,
                    reinterpret_cast<const char*>(kT1ResponseDatagram),
                    arraysize(kT1ResponseDatagram));
-  PrepareSockets();
 
   TransactionHelper helper0(kT0HostName,
                             kT0Qtype,
                             kT0RecordCount);
   helper0.StartTransaction(transaction_factory_.get());
   TransactionHelper helper1(kT1HostName,
                             kT1Qtype,
                             kT1RecordCount);
   helper1.StartTransaction(transaction_factory_.get());
 
   MessageLoop::current()->RunAllPending();
 
   EXPECT_TRUE(helper0.has_completed());
   EXPECT_TRUE(helper1.has_completed());
 }
 
 TEST_F(DnsTransactionTest, CancelLookup) {
   AddAsyncResponse(kT0HostName,
                    kT0Qtype,
                    0 /* id */,
                    reinterpret_cast<const char*>(kT0ResponseDatagram),
                    arraysize(kT0ResponseDatagram));
   AddAsyncResponse(kT1HostName,
                    kT1Qtype,
                    1 /* id */,
                    reinterpret_cast<const char*>(kT1ResponseDatagram),
                    arraysize(kT1ResponseDatagram));
-  PrepareSockets();
 
   TransactionHelper helper0(kT0HostName,
                             kT0Qtype,
                             kT0RecordCount);
   helper0.StartTransaction(transaction_factory_.get());
   TransactionHelper helper1(kT1HostName,
                             kT1Qtype,
                             kT1RecordCount);
   helper1.StartTransaction(transaction_factory_.get());
 
   helper0.Cancel();
 
   MessageLoop::current()->RunAllPending();
 
   EXPECT_FALSE(helper0.has_completed());
   EXPECT_TRUE(helper1.has_completed());
 }
 
 TEST_F(DnsTransactionTest, DestroyFactory) {
   AddAsyncResponse(kT0HostName,
                    kT0Qtype,
                    0 /* id */,
                    reinterpret_cast<const char*>(kT0ResponseDatagram),
                    arraysize(kT0ResponseDatagram));
-  PrepareSockets();
 
   TransactionHelper helper0(kT0HostName,
                             kT0Qtype,
                             kT0RecordCount);
   helper0.StartTransaction(transaction_factory_.get());
 
   // Destroying the client does not affect running requests.
   transaction_factory_.reset(NULL);
 
   MessageLoop::current()->RunAllPending();
 
   EXPECT_TRUE(helper0.has_completed());
 }
 
 TEST_F(DnsTransactionTest, CancelFromCallback) {
   AddAsyncResponse(kT0HostName,
                    kT0Qtype,
                    0 /* id */,
                    reinterpret_cast<const char*>(kT0ResponseDatagram),
                    arraysize(kT0ResponseDatagram));
-  PrepareSockets();
 
   TransactionHelper helper0(kT0HostName,
                             kT0Qtype,
                             kT0RecordCount);
   helper0.set_cancel_in_callback();
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 }
 
+TEST_F(DnsTransactionTest, MismatchedResponseSync) {

[mmenke, 2012/08/29 16:23:57]

I'd also like a test where we get an ERR_DNS_MALFORMED_RESPONSE

[szym, 2012/08/29 21:37:26]

That was not possible in this patch. If the transaction could't make any further
attempts it would wait until the last attempt timed out. See
dns_transaction.cc:520

If we fail early, we will still be vulnerable to unrelated packets, just
slightly less if retries are allowed. If we wait until timeout, we might be
fruitlessly stalling the whole HostResolver::Request. I'd rather err on the
optimistic site for now.

+  config_.attempts = 2;
+  config_.timeout = TestTimeouts::tiny_timeout();
+  ConfigureFactory();
+
+  // Attempt receives mismatched response followed by valid response.
+  scoped_ptr<DnsSocketData> data(
+      new DnsSocketData(kT0HostName, kT0Qtype, 0, SYNCHRONOUS));
+  data->AddResponseData(reinterpret_cast<const char*>(kT1ResponseDatagram),
+                        arraysize(kT1ResponseDatagram), SYNCHRONOUS);
+  data->AddResponseData(reinterpret_cast<const char*>(kT0ResponseDatagram),
+                        arraysize(kT0ResponseDatagram), SYNCHRONOUS);
+  AddSocketData(data.Pass());
+
+  TransactionHelper helper0(kT0HostName,
+                            kT0Qtype,
+                            kT0RecordCount);
+  EXPECT_TRUE(helper0.RunUntilDone(transaction_factory_.get()));
+}
+
+TEST_F(DnsTransactionTest, MismatchedResponseAsync) {
+  config_.attempts = 2;
+  config_.timeout = TestTimeouts::tiny_timeout();
+  ConfigureFactory();
+
+  // First attempt receives mismatched response followed by valid response.
+  // Second attempt times out.
+  scoped_ptr<DnsSocketData> data(
+      new DnsSocketData(kT0HostName, kT0Qtype, 0, ASYNC));
+  data->AddResponseData(reinterpret_cast<const char*>(kT1ResponseDatagram),
+                        arraysize(kT1ResponseDatagram), ASYNC);
+  data->AddResponseData(reinterpret_cast<const char*>(kT0ResponseDatagram),
+                        arraysize(kT0ResponseDatagram), ASYNC);
+  AddSocketData(data.Pass());
+  AddTimeout(kT0HostName, kT0Qtype);
+
+  TransactionHelper helper0(kT0HostName,
+                            kT0Qtype,
+                            kT0RecordCount);
+  EXPECT_TRUE(helper0.RunUntilDone(transaction_factory_.get()));
+}
+
 TEST_F(DnsTransactionTest, ServerFail) {
   AddAsyncRcode(kT0HostName, kT0Qtype, dns_protocol::kRcodeSERVFAIL);
-  PrepareSockets();
 
   TransactionHelper helper0(kT0HostName,
                             kT0Qtype,
                             ERR_DNS_SERVER_FAILED);
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 }
 
 TEST_F(DnsTransactionTest, NoDomain) {
   AddAsyncRcode(kT0HostName, kT0Qtype, dns_protocol::kRcodeNXDOMAIN);
-  PrepareSockets();
 
   TransactionHelper helper0(kT0HostName,
                             kT0Qtype,
                             ERR_NAME_NOT_RESOLVED);
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 }
 
 TEST_F(DnsTransactionTest, Timeout) {
   config_.attempts = 3;
   // Use short timeout to speed up the test.
   config_.timeout = TestTimeouts::tiny_timeout();
   ConfigureFactory();
 
   AddTimeout(kT0HostName, kT0Qtype);
   AddTimeout(kT0HostName, kT0Qtype);
   AddTimeout(kT0HostName, kT0Qtype);
-  PrepareSockets();
 
   TransactionHelper helper0(kT0HostName,
                             kT0Qtype,
                             ERR_DNS_TIMED_OUT);
   EXPECT_TRUE(helper0.RunUntilDone(transaction_factory_.get()));
   MessageLoop::current()->AssertIdle();
 }
 
 TEST_F(DnsTransactionTest, ServerFallbackAndRotate) {
   // Test that we fallback on both server failure and timeout.
   config_.attempts = 2;
   // The next request should start from the next server.
   config_.rotate = true;
   ConfigureNumServers(3);
   // Use short timeout to speed up the test.
   config_.timeout = TestTimeouts::tiny_timeout();
   ConfigureFactory();
 
   // Responses for first request.
   AddTimeout(kT0HostName, kT0Qtype);
   AddAsyncRcode(kT0HostName, kT0Qtype, dns_protocol::kRcodeSERVFAIL);
   AddTimeout(kT0HostName, kT0Qtype);
   AddAsyncRcode(kT0HostName, kT0Qtype, dns_protocol::kRcodeSERVFAIL);
   AddAsyncRcode(kT0HostName, kT0Qtype, dns_protocol::kRcodeNXDOMAIN);
   // Responses for second request.
   AddAsyncRcode(kT1HostName, kT1Qtype, dns_protocol::kRcodeSERVFAIL);
   AddAsyncRcode(kT1HostName, kT1Qtype, dns_protocol::kRcodeNXDOMAIN);
-  PrepareSockets();
 
   TransactionHelper helper0(kT0HostName,
                             kT0Qtype,
                             ERR_NAME_NOT_RESOLVED);
   TransactionHelper helper1(kT1HostName,
                             kT1Qtype,
                             ERR_NAME_NOT_RESOLVED);
 
   EXPECT_TRUE(helper0.RunUntilDone(transaction_factory_.get()));
   EXPECT_TRUE(helper1.Run(transaction_factory_.get()));
@@ skipping 11 matching lines @@
   config_.search.push_back("b");
   config_.search.push_back("c");
   config_.rotate = true;
   ConfigureNumServers(2);
   ConfigureFactory();
 
   AddAsyncRcode("x.y.z", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   AddAsyncRcode("x.y.z.a", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   AddAsyncRcode("x.y.z.b", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   AddAsyncRcode("x.y.z.c", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
-  PrepareSockets();
 
   TransactionHelper helper0("x.y.z",
                             dns_protocol::kTypeA,
                             ERR_NAME_NOT_RESOLVED);
 
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 
   // Also check if suffix search causes server rotation.
   unsigned kOrder0[] = { 0, 1, 0, 1 };
   CheckServerOrder(kOrder0, arraysize(kOrder0));
@@ skipping 10 matching lines @@
   AddAsyncRcode("x.y.a", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   AddAsyncRcode("x.y.b", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   AddAsyncRcode("x.y.c", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   AddAsyncRcode("x.y", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   // Responses for second transaction.
   AddAsyncRcode("x.a", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   AddAsyncRcode("x.b", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   AddAsyncRcode("x.c", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   // Responses for third transaction.
   AddAsyncRcode("x", dns_protocol::kTypeAAAA, dns_protocol::kRcodeNXDOMAIN);
-  PrepareSockets();
 
   TransactionHelper helper0("x.y",
                             dns_protocol::kTypeA,
                             ERR_NAME_NOT_RESOLVED);
 
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 
   // A single-label name.
   TransactionHelper helper1("x",
                             dns_protocol::kTypeA,
                             ERR_NAME_NOT_RESOLVED);
 
   EXPECT_TRUE(helper1.Run(transaction_factory_.get()));
 
   // A fully-qualified name.
   TransactionHelper helper2("x.",
                             dns_protocol::kTypeAAAA,
                             ERR_NAME_NOT_RESOLVED);
 
   EXPECT_TRUE(helper2.Run(transaction_factory_.get()));
 }
 
 TEST_F(DnsTransactionTest, EmptySuffixSearch) {
   // Responses for first transaction.
   AddAsyncRcode("x", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
-  PrepareSockets();
 
   // A fully-qualified name.
   TransactionHelper helper0("x.",
                             dns_protocol::kTypeA,
                             ERR_NAME_NOT_RESOLVED);
 
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 
   // A single label name is not even attempted.
   TransactionHelper helper1("singlelabel",
@@ skipping 12 matching lines @@
   ConfigureFactory();
 
   // Responses for first transaction.
   AddAsyncRcode("x.y.z", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   // Responses for second transaction.
   AddAsyncRcode("x.y", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   // Responses for third transaction.
   AddAsyncRcode("x.a", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   AddAsyncRcode("x.b", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   AddAsyncRcode("x.c", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
-  PrepareSockets();
 
   TransactionHelper helper0("x.y.z",
                             dns_protocol::kTypeA,
                             ERR_NAME_NOT_RESOLVED);
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 
   TransactionHelper helper1("x.y",
                             dns_protocol::kTypeA,
                             ERR_NAME_NOT_RESOLVED);
   EXPECT_TRUE(helper1.Run(transaction_factory_.get()));
@@ skipping 26 matching lines @@
   config_.search.push_back("c");
   ConfigureFactory();
 
   AddAsyncRcode("x.y.z", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   AddAsyncRcode("x.y.z.a", dns_protocol::kTypeA, dns_protocol::kRcodeNXDOMAIN);
   AddAsyncResponse("x.y.z.b",
                    dns_protocol::kTypeA,
                    0 /* id */,
                    reinterpret_cast<const char*>(kResponseNoData),
                    arraysize(kResponseNoData));
-  PrepareSockets();
 
   TransactionHelper helper0("x.y.z", dns_protocol::kTypeA, 0 /* answers */);
 
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 }
 
 TEST_F(DnsTransactionTest, SyncFirstQuery) {
   config_.search.push_back("lab.ccs.neu.edu");
   config_.search.push_back("ccs.neu.edu");
   ConfigureFactory();
 
-  AddResponse(kT0HostName,
-              kT0Qtype,
-              0 /* id */,
-              reinterpret_cast<const char*>(kT0ResponseDatagram),
-              arraysize(kT0ResponseDatagram),
-              SYNCHRONOUS);
-  PrepareSockets();
+  AddSyncResponse(kT0HostName,
+                  kT0Qtype,
+                  0 /* id */,
+                  reinterpret_cast<const char*>(kT0ResponseDatagram),
+                  arraysize(kT0ResponseDatagram));
 
   TransactionHelper helper0(kT0HostName,
                             kT0Qtype,
                             kT0RecordCount);
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 }
 
 TEST_F(DnsTransactionTest, SyncFirstQueryWithSearch) {
   config_.search.push_back("lab.ccs.neu.edu");
   config_.search.push_back("ccs.neu.edu");
   ConfigureFactory();
 
-  AddRcode("www.lab.ccs.neu.edu",
-           kT2Qtype,
-           dns_protocol::kRcodeNXDOMAIN,
-           SYNCHRONOUS);
-  AddResponse(kT2HostName,  // "www.ccs.neu.edu"
-              kT2Qtype,
-              2 /* id */,
-              reinterpret_cast<const char*>(kT2ResponseDatagram),
-              arraysize(kT2ResponseDatagram),
-              ASYNC);
-  PrepareSockets();
+  AddSyncRcode("www.lab.ccs.neu.edu",
+               kT2Qtype,
+               dns_protocol::kRcodeNXDOMAIN);
+  AddAsyncResponse(kT2HostName,  // "www.ccs.neu.edu"
+                   kT2Qtype,
+                   2 /* id */,
+                   reinterpret_cast<const char*>(kT2ResponseDatagram),
+                   arraysize(kT2ResponseDatagram));
 
   TransactionHelper helper0("www",
                             kT2Qtype,
                             kT2RecordCount);
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 }
 
 TEST_F(DnsTransactionTest, SyncSearchQuery) {
   config_.search.push_back("lab.ccs.neu.edu");
   config_.search.push_back("ccs.neu.edu");
   ConfigureFactory();
 
-  AddRcode("www.lab.ccs.neu.edu",
-           dns_protocol::kTypeA,
-           dns_protocol::kRcodeNXDOMAIN,
-           ASYNC);
-  AddResponse(kT2HostName,
-              kT2Qtype,
-              2 /* id */,
-              reinterpret_cast<const char*>(kT2ResponseDatagram),
-              arraysize(kT2ResponseDatagram),
-              SYNCHRONOUS);
-  PrepareSockets();
+  AddAsyncRcode("www.lab.ccs.neu.edu",
+                dns_protocol::kTypeA,
+               dns_protocol::kRcodeNXDOMAIN);
+  AddSyncResponse(kT2HostName,
+                  kT2Qtype,
+                  2 /* id */,
+                  reinterpret_cast<const char*>(kT2ResponseDatagram),
+                  arraysize(kT2ResponseDatagram));
 
   TransactionHelper helper0("www",
                             kT2Qtype,
                             kT2RecordCount);
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 }
 
 }  // namespace
 
 }  // namespace net