Break the dependency of DnsQuery and DnsResponse from AddressList

BUG=None
TEST=net_unittests --gtest_filter="DnsQuery*:DnsResponse*"

Review URL: http://codereview.chromium.org/6995088

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@88552 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 115 insertions, 163 deletions

diff --git a/net/base/dns_query.cc b/net/base/dns_query.cc
index ba79150..cbc1e82 100644
--- a/net/base/dns_query.cc
+++ b/net/base/dns_query.cc
@@ -7,7 +7,6 @@
 #include <string>
 
 #include "base/rand_util.h"
-#include "net/base/address_family.h"
 #include "net/base/dns_util.h"
 
 namespace net {
@@ -19,16 +18,8 @@ void PackUint16BE(char buf[2], uint16 v) {
   buf[1] = v & 0xff;
 }
 
-uint16 QTypeFromAddressFamily(AddressFamily address_family) {
-  switch (address_family) {
-    case ADDRESS_FAMILY_IPV4:
-      return kDNS_A;
-    case ADDRESS_FAMILY_IPV6:
-      return kDNS_AAAA;
-    default:
-      NOTREACHED() << "Bad address family";
-      return kDNS_A;
-  }
+uint16 UnpackUint16BE(char buf[2]) {
+  return static_cast<uint8>(buf[0]) << 8 | static_cast<uint8>(buf[1]);
 }
 
 }  // namespace
@@ -42,70 +33,61 @@ uint16 QTypeFromAddressFamily(AddressFamily address_family) {
 // DnsQuery construction.
 static const char kHeader[] = {0x00, 0x00, 0x01, 0x00, 0x00, 0x01,
                                0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
-static const size_t kHeaderLen = arraysize(kHeader);
+static const size_t kHeaderSize = arraysize(kHeader);
 
-DnsQuery::DnsQuery(const std::string& hostname,
-                   AddressFamily address_family,
-                   int port)
-    : port_(port),
-      id_(0),
-      qtype_(QTypeFromAddressFamily(address_family)),
-      hostname_(hostname) {
-  std::string qname;
-  if (!net::DNSDomainFromDot(hostname, &qname))
-    return;
+DnsQuery::DnsQuery(const std::string& dns_name, uint16 qtype)
+    : dns_name_size_(dns_name.size()) {
+  DCHECK(DnsResponseBuffer(reinterpret_cast<const uint8*>(dns_name.c_str()),
+                           dns_name.size()).DNSName(NULL));
+  DCHECK(qtype == kDNS_A || qtype == kDNS_AAAA);
 
-  size_t query_size = kHeaderLen + qname.size() +
-    sizeof(qtype_) + sizeof(kClassIN);
-
-  io_buffer_ = new IOBufferWithSize(query_size);
+  io_buffer_ = new IOBufferWithSize(kHeaderSize + question_size());
 
   int byte_offset = 0;
   char* buffer_head = io_buffer_->data();
-  memcpy(&buffer_head[byte_offset], kHeader, kHeaderLen);
-  byte_offset += kHeaderLen;
-  memcpy(&buffer_head[byte_offset], &qname[0], qname.size());
-  byte_offset += qname.size();
-  PackUint16BE(&buffer_head[byte_offset], qtype_);
-  byte_offset += sizeof(qtype_);
+  memcpy(&buffer_head[byte_offset], kHeader, kHeaderSize);
+  byte_offset += kHeaderSize;
+  memcpy(&buffer_head[byte_offset], &dns_name[0], dns_name_size_);
+  byte_offset += dns_name_size_;
+  PackUint16BE(&buffer_head[byte_offset], qtype);
+  byte_offset += sizeof(qtype);
   PackUint16BE(&buffer_head[byte_offset], kClassIN);
-
-  // Randomize ID, first two bytes.
-  id_ = base::RandUint64() & 0xffff;
-  PackUint16BE(&buffer_head[0], id_);
+  RandomizeId();
 }
 
-DnsQuery::~DnsQuery() {
+DnsQuery::DnsQuery(const DnsQuery& rhs) : dns_name_size_(rhs.dns_name_size_) {
+  io_buffer_ = new IOBufferWithSize(rhs.io_buffer()->size());
+  memcpy(io_buffer_->data(), rhs.io_buffer()->data(), rhs.io_buffer()->size());
+  RandomizeId();
 }
 
-int DnsQuery::port() const {
-  DCHECK(IsValid());
-  return port_;
+DnsQuery::~DnsQuery() {
 }
 
 uint16 DnsQuery::id() const {
-  DCHECK(IsValid());
-  return id_;
+  return UnpackUint16BE(&io_buffer_->data()[0]);
 }
 
 uint16 DnsQuery::qtype() const {
-  DCHECK(IsValid());
-  return qtype_;
+  return UnpackUint16BE(&io_buffer_->data()[kHeaderSize + dns_name_size_]);
+}
+
+DnsQuery* DnsQuery::CloneWithNewId() const {
+  return new DnsQuery(*this);
 }
 
-AddressFamily DnsQuery::address_family() const {
-  DCHECK(IsValid());
-  return address_family_;
+size_t DnsQuery::question_size() const {
+  return dns_name_size_       // QNAME
+    + sizeof(uint16)          // QTYPE
+    + sizeof(uint16);         // QCLASS
 }
 
-const std::string& DnsQuery::hostname() const {
-  DCHECK(IsValid());
-  return hostname_;
+const char* DnsQuery::question_data() const {
+  return &io_buffer_->data()[kHeaderSize];
 }
 
-IOBufferWithSize* DnsQuery::io_buffer() const {
-  DCHECK(IsValid());
-  return io_buffer_.get();
+void DnsQuery::RandomizeId() {
+  PackUint16BE(&io_buffer_->data()[0], base::RandUint64() & 0xffff);
 }
 
 }  // namespace net
diff --git a/net/base/dns_query.h b/net/base/dns_query.h
index bc25997..71b260c 100644
--- a/net/base/dns_query.h
+++ b/net/base/dns_query.h
@@ -8,63 +8,59 @@
 
 #include <string>
 
-#include "net/base/address_family.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_util.h"
 
 namespace net{
 
-// A class that encapsulates bits and pieces related to DNS request processing.
+// Represents on-the-wire DNS query message as an object.
 class DnsQuery {
  public:
-  // Constructs an object containing an IOBuffer with raw DNS query string
-  // for |hostname| with the given |address_family|; |port| is here due to
-  // legacy -- getaddrinfo() takes service name, which is mapped to some
-  // port and returns sockaddr_in structures with ports filled in, so do we
-  // -- look at DnsResponse::Parse() to see where it is used.
+  // Constructs a query message from |dns_name| which *MUST* be in a valid
+  // DNS name format, and |qtype| which must be either kDNS_A or kDNS_AAA.
 
   // Every generated object has a random ID, hence two objects generated
   // with the same set of constructor arguments are generally not equal;
   // there is a 1/2^16 chance of them being equal due to size of |id_|.
-  DnsQuery(const std::string& hostname, AddressFamily address_family, int port);
+  DnsQuery(const std::string& dns_name, uint16 qtype);
   ~DnsQuery();
 
-  // Returns true if the constructed object was valid.
-  bool IsValid() const { return io_buffer_.get() != NULL; }
+  // Clones |this| verbatim with ID field of the header regenerated.
+  DnsQuery* CloneWithNewId() const;
 
-  // DnsQuery field accessors.  These should only be called on an object
-  // for whom |IsValid| is true.
-  int port() const;
+  // DnsQuery field accessors.
   uint16 id() const;
   uint16 qtype() const;
-  AddressFamily address_family() const;
-  const std::string& hostname() const;
+
+  // Returns the size of the Question section of the query.  Used when
+  // matching the response.
+  size_t question_size() const;
+
+  // Returns pointer to the Question section of the query.  Used when
+  // matching the response.
+  const char* question_data() const;
 
   // IOBuffer accessor to be used for writing out the query.
-  IOBufferWithSize* io_buffer() const;
+  IOBufferWithSize* io_buffer() const { return io_buffer_; }
 
  private:
-  // Port to be used by corresponding DnsResponse when filling sockaddr_ins
-  // to be returned.
-  int port_;
+  // Copy constructor to be used only by CloneWithNewId; it's not public
+  // since there is a semantic difference -- this does not construct an
+  // exact copy!
+  DnsQuery(const DnsQuery& rhs);
 
-  // ID of the query.
-  uint16 id_;
+  // Not implemented; just to prevent the assignment.
+  void operator=(const DnsQuery&);
 
-  // Type of query, currently, either A or AAAA.
-  uint16 qtype_;
+  // Randomizes ID field of the query message.
+  void RandomizeId();
 
-  // Address family of the query; used when constructing new object from
-  // this one.
-  AddressFamily address_family_;
-
-  // Hostname that we are trying to resolve.
-  std::string hostname_;
+  // Size of the DNS name (*NOT* hostname) we are trying to resolve; used
+  // to calculate offsets.
+  size_t dns_name_size_;
 
   // Contains query bytes to be consumed by higher level Write() call.
   scoped_refptr<IOBufferWithSize> io_buffer_;
-
-  DISALLOW_COPY_AND_ASSIGN(DnsQuery);
 };
 
 }  // namespace net
diff --git a/net/base/dns_query_unittest.cc b/net/base/dns_query_unittest.cc
index ba21c4e..7e5e75e 100644
--- a/net/base/dns_query_unittest.cc
+++ b/net/base/dns_query_unittest.cc
@@ -42,35 +42,11 @@ namespace net {
 //  |                     QCLASS                    |
 //  +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
 
-TEST(DnsQueryTest, RandomIdTest) {
-  const std::string kHostname = "www.google.com";
-  const uint16 kPort = 80;
-
-  DnsQuery q1(kHostname, ADDRESS_FAMILY_IPV4, kPort);
-  EXPECT_TRUE(q1.IsValid());
-  EXPECT_EQ(kPort, q1.port());
-  EXPECT_EQ(kDNS_A, q1.qtype());
-  EXPECT_EQ(kHostname, q1.hostname());
-
-  DnsQuery q2(kHostname, ADDRESS_FAMILY_IPV4, kPort);
-  EXPECT_TRUE(q2.IsValid());
-  EXPECT_EQ(kPort, q2.port());
-  EXPECT_EQ(kDNS_A, q2.qtype());
-  EXPECT_EQ(kHostname, q2.hostname());
-
-  // This has a 1/2^16 probability of failure.
-  EXPECT_FALSE(q1.id() == q2.id());
-}
-
 TEST(DnsQueryTest, ConstructorTest) {
-  const std::string kHostname = "www.google.com";
-  const uint16 kPort = 80;
+  std::string kHostnameDns("\003www\006google\003com", 16);
 
-  DnsQuery q1(kHostname, ADDRESS_FAMILY_IPV4, kPort);
-  EXPECT_TRUE(q1.IsValid());
-  EXPECT_EQ(kPort, q1.port());
+  DnsQuery q1(kHostnameDns, kDNS_A);
   EXPECT_EQ(kDNS_A, q1.qtype());
-  EXPECT_EQ(kHostname, q1.hostname());
 
   uint8 id_hi = q1.id() >> 8, id_lo = q1.id() & 0xff;
 
@@ -96,14 +72,36 @@ TEST(DnsQueryTest, ConstructorTest) {
 
   int expected_size = arraysize(query_data);
   EXPECT_EQ(expected_size, q1.io_buffer()->size());
-  EXPECT_EQ(0, memcmp(q1.io_buffer()->data(), query_data,
-                      q1.io_buffer()->size()));
+  EXPECT_EQ(0, memcmp(q1.io_buffer()->data(), query_data, expected_size));
+}
 
-  // Query with a long hostname.
-  const char hostname_too_long[] = "123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.1234";
+TEST(DnsQueryTest, CloneTest) {
+  std::string kHostnameDns("\003www\006google\003com", 16);
 
-  DnsQuery q2(hostname_too_long, ADDRESS_FAMILY_IPV4, kPort);
-  EXPECT_FALSE(q2.IsValid());
+  DnsQuery q1(kHostnameDns, kDNS_A);
+  scoped_ptr<DnsQuery> q2(q1.CloneWithNewId());
+  EXPECT_EQ(q1.io_buffer()->size(), q2->io_buffer()->size());
+  EXPECT_EQ(q1.qtype(), q2->qtype());
+  EXPECT_EQ(q1.question_size(), q2->question_size());
+  EXPECT_EQ(0, memcmp(q1.question_data(), q2->question_data(),
+                      q1.question_size()));
+}
+
+TEST(DnsQueryTest, RandomIdTest) {
+  std::string kHostnameDns("\003www\006google\003com", 16);
+
+  // Since id fields are 16-bit values, we iterate to reduce the
+  // probability of collision, to avoid a flaky test.
+  bool ids_are_random = false;
+  for (int i = 0; i < 1000; ++i) {
+    DnsQuery q1(kHostnameDns, kDNS_A);
+    DnsQuery q2(kHostnameDns, kDNS_A);
+    scoped_ptr<DnsQuery> q3(q1.CloneWithNewId());
+    ids_are_random = q1.id () != q2.id() && q1.id() != q3->id();
+    if (ids_are_random)
+      break;
+  }
+  EXPECT_TRUE(ids_are_random);
 }
 
 }  // namespace net
diff --git a/net/base/dns_response.cc b/net/base/dns_response.cc
index 9cfbae2..31dfac3 100644
--- a/net/base/dns_response.cc
+++ b/net/base/dns_response.cc
@@ -4,7 +4,6 @@
 
 #include "net/base/dns_response.h"
 
-#include "net/base/address_list.h"
 #include "net/base/dns_util.h"
 #include "net/base/net_errors.h"
 
@@ -18,15 +17,12 @@ DnsResponse::DnsResponse(DnsQuery* query)
     : query_(query),
       io_buffer_(new IOBufferWithSize(kMaxResponseSize + 1)) {
   DCHECK(query_);
-  DCHECK(query_->IsValid());
 }
 
 DnsResponse::~DnsResponse() {
 }
 
-int DnsResponse::Parse(int nbytes, AddressList* results) {
-  DCHECK(query_->IsValid());
-
+int DnsResponse::Parse(int nbytes, std::vector<IPAddressNumber>* ip_addresses) {
   // Response includes query, it should be at least that size.
   if (nbytes < query_->io_buffer()->size() || nbytes > kMaxResponseSize)
     return ERR_DNS_MALFORMED_RESPONSE;
@@ -59,14 +55,10 @@ int DnsResponse::Parse(int nbytes, AddressList* results) {
   if (query_count != 1) // Sent a single question, shouldn't have changed.
     return ERR_DNS_MALFORMED_RESPONSE;
 
-  std::string hostname;
-  uint16 qtype, qclass;
-  if (!response.DNSName(&hostname) ||
-      !response.U16(&qtype) ||
-      !response.U16(&qclass) ||
-      hostname != query_->hostname() || // Make sure Question section
-      qtype != query_->qtype() ||       // echoed back.
-      qclass != kClassIN) {
+  base::StringPiece question; // Make sure question section is echoed back.
+  if (!response.Block(&question, query_->question_size()) ||
+      memcmp(question.data(), query_->question_data(),
+             query_->question_size())) {
     return ERR_DNS_MALFORMED_RESPONSE;
   }
 
@@ -76,7 +68,7 @@ int DnsResponse::Parse(int nbytes, AddressList* results) {
   std::vector<IPAddressNumber> rdatas;
   while (answer_count--) {
     uint32 ttl;
-    uint16 rdlength;
+    uint16 rdlength, qtype, qclass;
     if (!response.DNSName(NULL) ||
         !response.U16(&qtype) ||
         !response.U16(&qclass) ||
@@ -84,7 +76,6 @@ int DnsResponse::Parse(int nbytes, AddressList* results) {
         !response.U16(&rdlength)) {
       return ERR_DNS_MALFORMED_RESPONSE;
     }
-
     if (qtype == query_->qtype() &&
         qclass == kClassIN &&
         (rdlength == kIPv4AddressSize || rdlength == kIPv6AddressSize)) {
@@ -99,7 +90,8 @@ int DnsResponse::Parse(int nbytes, AddressList* results) {
   if (rdatas.empty())
     return ERR_NAME_NOT_RESOLVED;
 
-  *results = AddressList::CreateFromIPAddressList(rdatas, query_->port());
+  if (ip_addresses)
+    ip_addresses->swap(rdatas);
   return OK;
 }
 
diff --git a/net/base/dns_response.h b/net/base/dns_response.h
index 39ee47d..b0704c9 100644
--- a/net/base/dns_response.h
+++ b/net/base/dns_response.h
@@ -10,10 +10,8 @@
 
 namespace net{
 
-class AddressList;
-
-// A class that encapsulates bits and pieces related to DNS response
-// processing.
+// Represents on-the-wire DNS response as an object; allows extracting
+// records.
 class DnsResponse {
  public:
   // Constructs an object with an IOBuffer large enough to read
@@ -27,9 +25,9 @@ class DnsResponse {
   // read.
   IOBufferWithSize* io_buffer() { return io_buffer_.get(); }
 
-  // Parses response of size nbytes and puts address into |results|,
+  // Parses response of size nbytes and puts address into |ip_addresses|,
   // returns net_error code in case of failure.
-  int Parse(int nbytes, AddressList* results);
+  int Parse(int nbytes, std::vector<IPAddressNumber>* ip_addresses);
 
  private:
   // The matching query; |this| is the response for |query_|.  We do not
diff --git a/net/base/dns_response_unittest.cc b/net/base/dns_response_unittest.cc
index df56d2f..6dd790e 100644
--- a/net/base/dns_response_unittest.cc
+++ b/net/base/dns_response_unittest.cc
@@ -2,10 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "net/base/address_list.h"
 #include "net/base/dns_response.h"
+#include "net/base/dns_util.h"
 #include "net/base/net_errors.h"
-#include "net/base/sys_addrinfo.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
@@ -67,17 +66,20 @@ namespace net {
 //  /                                               /
 //  +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
 
+// TODO(agayev): add more thorough tests.
 TEST(DnsResponseTest, ResponseWithCnameA) {
-  const std::string kHostname = "codereview.chromium.org";
-  const uint16 kPort = 80;
+  const std::string kHostnameDns("\012codereview\010chromium\003org", 25);
 
-  DnsQuery q1(kHostname, ADDRESS_FAMILY_IPV4, kPort);
+  DnsQuery q1(kHostnameDns, kDNS_A);
   uint8 id_hi = q1.id() >> 8, id_lo = q1.id() & 0xff;
 
   uint8 ip[] = {              // codereview.chromium.org resolves to
     0x4a, 0x7d, 0x5f, 0x79    // 74.125.95.121
   };
 
+  std::vector<IPAddressNumber> expected_ips;
+  expected_ips.push_back(IPAddressNumber(ip, ip + arraysize(ip)));
+
   uint8 response_data[] = {
     // Header
     id_hi, id_lo,             // ID
@@ -126,27 +128,11 @@ TEST(DnsResponseTest, ResponseWithCnameA) {
   memcpy(r1.io_buffer()->data(), &response_data[0],
          r1.io_buffer()->size());
 
+  // Verify resolved IPs.
   int response_size = arraysize(response_data);
-  AddressList address_list;
-  EXPECT_EQ(OK, r1.Parse(response_size, &address_list));
-
-  // Verify AddressList content.
-  size_t sockaddr_size = sizeof(struct sockaddr_in);
-  const struct addrinfo* ai = address_list.head();
-  EXPECT_EQ(kPort, address_list.GetPort());
-
-  // addrinfo part.
-  EXPECT_TRUE(ai != NULL);
-  EXPECT_EQ(AF_INET, ai->ai_family);
-  EXPECT_EQ(SOCK_STREAM, ai->ai_socktype);
-  EXPECT_EQ(sockaddr_size, ai->ai_addrlen);
-
-  // sockaddr_in part.
-  struct sockaddr_in* sa = reinterpret_cast<sockaddr_in*>(ai->ai_addr);
-  ASSERT_TRUE(sa != NULL);
-  EXPECT_EQ(AF_INET, sa->sin_family);
-  EXPECT_EQ(kPort, ntohs(sa->sin_port));
-  EXPECT_EQ(0, memcmp(&sa->sin_addr, &ip[0], kIPv4AddressSize));
+  std::vector<IPAddressNumber> actual_ips;
+  EXPECT_EQ(OK, r1.Parse(response_size, &actual_ips));
+  EXPECT_EQ(expected_ips, actual_ips);
 }
 
 }  // namespace net