Save TTL to cache if in SOA of NXDOMAIN or NODATA results

When we get an NXDOMAIN results, this will look at the last result
processed and extract the TTL from the SOA record if it exists (possilbe
TODO: look at all the results and get the shortest TTL). For NODATA
results with an rcode of NOERROR and zero answers, again record a cache
entry with the TTL of the SOA if present.

R=mgersh@chromium.org

Bug: 115051
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I6fe029760f4ecfd3b3a91667e95b403a9f37a228
Reviewed-on: https://chromium-review.googlesource.com/562037
Commit-Queue: Brad Lassey <lassey@chromium.org>
Reviewed-by: Miriam Gershenson <mgersh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518461}

net/dns/dns_protocol.h

@@ -119,6 +119,7 @@ static const uint16_t kClassIN = 1;
 // https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-4
 static const uint16_t kTypeA = 1;
 static const uint16_t kTypeCNAME = 5;
+static const uint16_t kTypeSOA = 6;
 static const uint16_t kTypePTR = 12;
 static const uint16_t kTypeTXT = 16;
 static const uint16_t kTypeAAAA = 28;

net/dns/dns_response.cc

@@ -301,6 +301,14 @@ DnsResponse::Result DnsResponse::ParseToAddressList(
   DnsRecordParser parser = Parser();
   DnsResourceRecord record;
   unsigned ancount = answer_count();
+  if (rcode() == dns_protocol::kRcodeNXDOMAIN ||
+      (ancount == 0 && rcode() == dns_protocol::kRcodeNOERROR)) {
+    // NXDOMAIN or NODATA case respecively.
+    if (parser.ReadRecord(&record) && record.type == dns_protocol::kTypeSOA) {
+      ttl_sec = std::min(ttl_sec, record.ttl);
+    }
+  }
+
   for (unsigned i = 0; i < ancount; ++i) {
     if (!parser.ReadRecord(&record))
       return DNS_MALFORMED_RESPONSE;
@@ -332,8 +340,6 @@ DnsResponse::Result DnsResponse::ParseToAddressList(
     }
   }
 
-  // TODO(szym): Extract TTL for NODATA results. http://crbug.com/115051
-
   // getcanonname in eglibc returns the first owner name of an A or AAAA RR.
   // If the response passed all the checks so far, then |expected_name| is it.
   *addr_list = AddressList::CreateFromIPAddressList(ip_addresses,

net/dns/dns_test_util.cc

@@ -94,6 +94,7 @@ class MockTransaction : public DnsTransaction,
  private:
   void Finish() {
     switch (result_.type) {
+      case MockDnsClientRule::NODOMAIN:
       case MockDnsClientRule::EMPTY:
       case MockDnsClientRule::OK: {
         std::string qname;
@@ -107,14 +108,17 @@ class MockTransaction : public DnsTransaction,
         dns_protocol::Header* header =
             reinterpret_cast<dns_protocol::Header*>(buffer);
         header->flags |= dns_protocol::kFlagResponse;
+        if (MockDnsClientRule::NODOMAIN == result_.type) {
+          header->flags |= base::HostToNet16(dns_protocol::kRcodeNXDOMAIN);
+        }
 
-        if (MockDnsClientRule::OK == result_.type) {
         const uint16_t kPointerToQueryName =
             static_cast<uint16_t>(0xc000 | sizeof(*header));
 
         const uint32_t kTTL = 86400;  // One day.
 
         // Size of RDATA which is a IPv4 or IPv6 address.
+        if (MockDnsClientRule::OK == result_.type)
           EXPECT_TRUE(result_.ip.IsValid());
         size_t rdata_size = result_.ip.size();
 
@@ -123,18 +127,25 @@ class MockTransaction : public DnsTransaction,
         size_t answer_size = 12 + rdata_size;
 
         // Write the answer using the expected IP address.
-          header->ancount = base::HostToNet16(1);
+        header->ancount =
+            base::HostToNet16(MockDnsClientRule::OK == result_.type ? 1 : 0);
         base::BigEndianWriter writer(buffer + nbytes, answer_size);
         writer.WriteU16(kPointerToQueryName);
-          writer.WriteU16(qtype_);
+        writer.WriteU16(MockDnsClientRule::OK == result_.type
+                            ? qtype_
+                            : dns_protocol::kTypeSOA);
         writer.WriteU16(dns_protocol::kClassIN);
         writer.WriteU32(kTTL);
         writer.WriteU16(static_cast<uint16_t>(rdata_size));
         writer.WriteBytes(result_.ip.bytes().data(), rdata_size);
         nbytes += answer_size;
-        }
+
         EXPECT_TRUE(response.InitParse(nbytes, query));
+        if (MockDnsClientRule::NODOMAIN == result_.type) {
+          callback_.Run(this, ERR_NAME_NOT_RESOLVED, &response);
+        } else {
           callback_.Run(this, OK, &response);
+        }
       } break;
       case MockDnsClientRule::FAIL:
         callback_.Run(this, ERR_NAME_NOT_RESOLVED, NULL);

net/dns/dns_test_util.h

@@ -159,6 +159,7 @@ class MockTransactionFactory;
 
 struct MockDnsClientRule {
   enum ResultType {
+    NODOMAIN,  // Fail asynchronously with ERR_NAME_NOT_RESOLVED and NXDOMAIN.
     FAIL,      // Fail asynchronously with ERR_NAME_NOT_RESOLVED.
     TIMEOUT,   // Fail asynchronously with ERR_DNS_TIMEOUT.
     EMPTY,     // Return an empty response.

net/dns/dns_transaction.cc

@@ -268,7 +268,6 @@ class DnsUDPAttempt : public DnsAttempt {
     }
     if (response_->flags() & dns_protocol::kFlagTC)
       return ERR_DNS_SERVER_REQUIRES_TCP;
-    // TODO(szym): Extract TTL for NXDOMAIN results. http://crbug.com/115051
     if (response_->rcode() == dns_protocol::kRcodeNXDOMAIN)
       return ERR_NAME_NOT_RESOLVED;
     if (response_->rcode() != dns_protocol::kRcodeNOERROR)
@@ -895,7 +894,7 @@ class DnsTransactionImpl : public DnsTransaction,
           if (!qnames_.empty())
             qnames_.pop_front();
           if (qnames_.empty()) {
-            return AttemptResult(ERR_NAME_NOT_RESOLVED, NULL);
+            return result;
           } else {
             result = StartQuery();
           }

net/dns/host_resolver_impl.cc

@@ -1024,6 +1024,8 @@ class HostResolverImpl::DnsTask : public base::SupportsWeakPtr<DnsTask> {
     StartAAAA();
   }
 
+  base::TimeDelta ttl() { return ttl_; }
+
  private:
   void StartA() {
     DCHECK(!transaction_a_);
@@ -1056,7 +1058,9 @@ class HostResolverImpl::DnsTask : public base::SupportsWeakPtr<DnsTask> {
                              const DnsResponse* response) {
     DCHECK(transaction);
     base::TimeDelta duration = base::TimeTicks::Now() - start_time;
-    if (net_error != OK) {
+
+    if (net_error != OK && !(net_error == ERR_NAME_NOT_RESOLVED && response &&
+                             response->IsValid())) {
       UMA_HISTOGRAM_LONG_TIMES_100("AsyncDNS.TransactionFailure", duration);
       OnFailure(net_error, DnsResponse::DNS_PARSE_OK);
       return;
@@ -1159,8 +1163,9 @@ class HostResolverImpl::DnsTask : public base::SupportsWeakPtr<DnsTask> {
     net_log_.EndEvent(
         NetLogEventType::HOST_RESOLVER_IMPL_DNS_TASK,
         base::Bind(&NetLogDnsTaskFailedCallback, net_error, result));
-    delegate_->OnDnsTaskComplete(task_start_time_, net_error, AddressList(),
-                                 base::TimeDelta());
+    delegate_->OnDnsTaskComplete(
+        task_start_time_, net_error, AddressList(),
+        num_completed_transactions_ > 0 ? ttl_ : base::TimeDelta());
   }
 
   void OnSuccess(const AddressList& addr_list) {
@@ -1588,7 +1593,16 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job,
       StartProcTask();
     } else {
       UmaAsyncDnsResolveStatus(RESOLVE_STATUS_FAIL);
-      CompleteRequestsWithError(net_error);
+      // If the ttl is max, we didn't get one from the record, so set it to 0
+      base::TimeDelta ttl =
+          dns_task->ttl() < base::TimeDelta::FromSeconds(
+                                std::numeric_limits<uint32_t>::max())
+              ? dns_task->ttl()
+              : base::TimeDelta::FromSeconds(0);
+      CompleteRequests(
+          HostCache::Entry(net_error, AddressList(),
+                           HostCache::Entry::Source::SOURCE_UNKNOWN, ttl),
+          ttl);
     }
   }
 

net/dns/host_resolver_impl_unittest.cc

@@ -1652,6 +1652,10 @@ class HostResolverImplDnsTest : public HostResolverImplTest {
  protected:
   // testing::Test implementation:
   void SetUp() override {
+    AddDnsRule("nodomain", dns_protocol::kTypeA, MockDnsClientRule::NODOMAIN,
+               false);
+    AddDnsRule("nodomain", dns_protocol::kTypeAAAA, MockDnsClientRule::NODOMAIN,
+               false);
     AddDnsRule("nx", dns_protocol::kTypeA, MockDnsClientRule::FAIL, false);
     AddDnsRule("nx", dns_protocol::kTypeAAAA, MockDnsClientRule::FAIL, false);
     AddDnsRule("ok", dns_protocol::kTypeA, MockDnsClientRule::OK, false);
@@ -2688,6 +2692,37 @@ TEST_F(HostResolverImplDnsTest, NoIPv6OnWifi) {
   EXPECT_TRUE(requests_[5]->HasOneAddress("::2", 80));
 }
 
+TEST_F(HostResolverImplDnsTest, NotFoundTTL) {
+  CreateResolver();
+  set_fallback_to_proctask(false);
+  ChangeDnsConfig(CreateValidDnsConfig());
+  // NODATA
+  Request* request = CreateRequest("empty");
+  EXPECT_THAT(request->Resolve(), IsError(ERR_IO_PENDING));
+  EXPECT_THAT(request->WaitForResult(), IsError(ERR_NAME_NOT_RESOLVED));
+  EXPECT_THAT(request->NumberOfAddresses(), 0);
+  HostCache::Key key(request->info().hostname(), ADDRESS_FAMILY_UNSPECIFIED, 0);
+  HostCache::EntryStaleness staleness;
+  const HostCache::Entry* cache_entry =
+      resolver_->GetHostCache()->Lookup(key, base::TimeTicks::Now());
+  EXPECT_TRUE(!!cache_entry);
+  EXPECT_TRUE(cache_entry->has_ttl());
+  EXPECT_THAT(cache_entry->ttl(), base::TimeDelta::FromSeconds(86400));
+
+  // NXDOMAIN
+  request = CreateRequest("nodomain");
+  EXPECT_THAT(request->Resolve(), IsError(ERR_IO_PENDING));
+  EXPECT_THAT(request->WaitForResult(), IsError(ERR_NAME_NOT_RESOLVED));
+  EXPECT_THAT(request->NumberOfAddresses(), 0);
+  HostCache::Key nxkey(request->info().hostname(), ADDRESS_FAMILY_UNSPECIFIED,
+                       0);
+  cache_entry =
+      resolver_->GetHostCache()->Lookup(nxkey, base::TimeTicks::Now());
+  EXPECT_TRUE(!!cache_entry);
+  EXPECT_TRUE(cache_entry->has_ttl());
+  EXPECT_THAT(cache_entry->ttl(), base::TimeDelta::FromSeconds(86400));
+}
+
 TEST_F(HostResolverImplTest, ResolveLocalHostname) {
   AddressList addresses;