[net] HostResolverImpl + DnsTransaction + DnsConfigService = Asynchronous DNS ready for experiments.

net/dns/dns_response.cc

Index: net/dns/dns_response.cc
diff --git a/net/dns/dns_response.cc b/net/dns/dns_response.cc
index 760bd5b68064dbc60c6a0f5b14b63e3e9d9b6f71..554e09d1cfce98f716f7bd57daeab09979f7ae0f 100644
--- a/net/dns/dns_response.cc
+++ b/net/dns/dns_response.cc
@@ -4,7 +4,9 @@
 
 #include "net/dns/dns_response.h"
 
+#include "base/string_util.h"
 #include "base/sys_byteorder.h"
+#include "net/base/address_list.h"
 #include "net/base/big_endian.h"
 #include "net/base/dns_util.h"
 #include "net/base/io_buffer.h"
@@ -32,7 +34,7 @@ DnsRecordParser::DnsRecordParser(const void* packet,
   DCHECK_LE(offset, length);
 }
 
-int DnsRecordParser::ParseName(const void* const vpos, std::string* out) const {
+int DnsRecordParser::ReadName(const void* const vpos, std::string* out) const {
   const char* const pos = reinterpret_cast<const char*>(vpos);
   DCHECK(packet_);
   DCHECK_LE(packet_, pos);
@@ -54,7 +56,7 @@ int DnsRecordParser::ParseName(const void* const vpos, std::string* out) const {
   }
 
   for (;;) {
-    // The two couple of bits of the length give the type of the length. It's
+    // The first two bits of the length give the type of the length. It's
     // either a direct length or a pointer to the remainder of the name.
     switch (*p & dns_protocol::kLabelMask) {
       case dns_protocol::kLabelPointer: {
@@ -105,9 +107,9 @@ int DnsRecordParser::ParseName(const void* const vpos, std::string* out) const {
   }
 }
 
-bool DnsRecordParser::ParseRecord(DnsResourceRecord* out) {
+bool DnsRecordParser::ReadRecord(DnsResourceRecord* out) {
   DCHECK(packet_);
-  size_t consumed = ParseName(cur_, &out->name);
+  size_t consumed = ReadName(cur_, &out->name);
   if (!consumed)
     return false;
   BigEndianReader reader(cur_ + consumed,
@@ -220,4 +222,73 @@ const dns_protocol::Header* DnsResponse::header() const {
   return reinterpret_cast<const dns_protocol::Header*>(io_buffer_->data());
 }
 
+DnsResponse::Result DnsResponse::ParseToAddressList(
+    AddressList* addr_list,
+    base::TimeDelta* ttl) const {
+  DCHECK(IsValid());
+  // DnsTransaction already verified that |response| matches the issued query.
+  // We still need to determine if there is a valid chain of CNAMEs from the
+  // query name to the RR owner name.
+
+  // Expected owner of record. No trailing dot.
+  std::string expected_name = GetDottedName();
+
+  uint16 expected_type = qtype();
+  DCHECK(expected_type == dns_protocol::kTypeA ||
+         expected_type == dns_protocol::kTypeAAAA);
+
+  size_t expected_size = (expected_type == dns_protocol::kTypeAAAA)
+      ? kIPv6AddressSize : kIPv4AddressSize;
+
+  // getcanonname in eglibc returns the first owner name of an A or AAAA RR.
+  std::string canon_name;
+
+  uint32 cname_ttl_sec = kuint32max;
+  uint32 addr_ttl_sec = kuint32max;
+  IPAddressList ip_addresses;
+  DnsRecordParser parser = Parser();
+  DnsResourceRecord record;
+  while (parser.ReadRecord(&record)) {
+    if (base::strcasecmp(record.name.c_str(), expected_name.c_str()) != 0) {
+      if (record.type == expected_type)

[cbentzel, 2012/02/15 19:48:35]

Are there any cases where DNS servers will append RRs of related domains to
prevent a round trip? I suppose we'll see the DNS_NAME_MISMATCH then, but might
be something to support in the future. 

[szym, 2012/02/15 20:28:05]

It might. I'll add a comment clarifying that for now we err on the cautious side
with the assumption that getaddrinfo will save our butt if we go too far. We'll
need a more flexible API that both records what kind of weirdness we've seen in
the DNS responses and allows to proceed despite such weirdness.

+        return DNS_NAME_MISMATCH;
+      continue;

[cbentzel, 2012/02/15 19:48:35]

Why do you want to continue in this case? 

[szym, 2012/02/15 20:28:05]

Hmmm. I'll make it unconditionally reject.

+    }
+    if ((record.type == dns_protocol::kTypeCNAME)) {

[cbentzel, 2012/02/15 19:48:35]

Nit: double parents not needed.

[szym, 2012/02/15 20:28:05]

clang barfed on it :-/

+      // Following the CNAME chain, only if no addresses seen.
+      if (!ip_addresses.empty()) {

[cbentzel, 2012/02/15 19:48:35]

Nit: extra braces

+        return DNS_CNAME_AFTER_ADDRESS;
+      }
+
+      if (!parser.ReadName(record.rdata.begin(), &expected_name)) {
+        return DNS_MALFORMED_RESPONSE;
+      }
+      cname_ttl_sec = std::min(cname_ttl_sec, record.ttl);
+    } else if (record.type == expected_type) {
+      if (record.rdata.size() != expected_size)
+        return DNS_SIZE_MISMATCH;
+      if (ip_addresses.empty()) {
+        addr_ttl_sec = record.ttl;
+      } else {
+        if (addr_ttl_sec != record.ttl)
+          return DNS_ADDRESS_TTL_MISMATCH;
+      }
+      ip_addresses.push_back(IPAddressNumber(record.rdata.begin(),
+                                             record.rdata.end()));
+      if (canon_name.empty())
+        canon_name = record.name;

[cbentzel, 2012/02/15 19:48:35]

Shouldn't the canon_name always be the same? Should we check for that?

[szym, 2012/02/15 20:28:05]

All the other checks will already catch that. This logic might be more clear if
we simply used |expected_name| as |canon_name| at the end.

+    }
+  }
+  if (!parser.AtEnd())
+    return DNS_MALFORMED_RESPONSE;
+
+  if (ip_addresses.empty())
+    return DNS_NO_ADDRESSES;
+
+  *addr_list = AddressList::CreateFromIPAddressList(ip_addresses,
+                                                    canon_name);
+  *ttl = base::TimeDelta::FromSeconds(std::min(cname_ttl_sec, addr_ttl_sec));
+  return DNS_SUCCESS;
+}
+
 }  // namespace net