[net/dns] Serve requests from HOSTS file if possible.

net/base/host_resolver_impl_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/base/host_resolver_impl.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 10 matching lines @@
 #include "base/time.h"
 #include "net/base/address_list.h"
 #include "net/base/completion_callback.h"
 #include "net/base/host_cache.h"
 #include "net/base/mock_host_resolver.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log_unittest.h"
 #include "net/base/net_util.h"
 #include "net/base/sys_addrinfo.h"
 #include "net/base/test_completion_callback.h"
+#include "net/dns/dns_client.h"
+#include "net/dns/dns_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
+namespace {
 
 using base::TimeDelta;
 using base::TimeTicks;
 
-static const size_t kMaxJobs = 10u;
-static const size_t kMaxRetryAttempts = 4u;
+const size_t kMaxJobs = 10u;
+const size_t kMaxRetryAttempts = 4u;
 
 PrioritizedDispatcher::Limits DefaultLimits() {
   PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, kMaxJobs);
   return limits;
 }
 
 HostResolverImpl::ProcTaskParams DefaultParams(
     HostResolverProc* resolver_proc) {
   return HostResolverImpl::ProcTaskParams(resolver_proc,
                                           kMaxRetryAttempts);
 }
 
 HostResolverImpl* CreateHostResolverImpl(HostResolverProc* resolver_proc) {
   return new HostResolverImpl(
       HostCache::CreateDefaultCache(),
       DefaultLimits(),
       DefaultParams(resolver_proc),
       scoped_ptr<DnsConfigService>(NULL),
       NULL);
 }
 
+HostResolverImpl* CreateHostResolverImplWithDns(

[mmenke, 2012/03/13 15:56:27]

nit:  Suggest "CreateHostResolverImplWithDnsConfig" or
"CreateHostResolverImplWithDirectDns".

+    HostResolverProc* resolver_proc,
+    scoped_ptr<DnsConfigService> config_service) {
+  return new HostResolverImpl(
+      HostCache::CreateDefaultCache(),
+      DefaultLimits(),
+      DefaultParams(resolver_proc),
+      config_service.Pass(),
+      NULL);
+}
+
 // This HostResolverImpl will only allow 1 outstanding resolve at a time.
 HostResolverImpl* CreateSerialHostResolverImpl(
     HostResolverProc* resolver_proc) {
   HostResolverImpl::ProcTaskParams params = DefaultParams(resolver_proc);
   params.max_retry_attempts = 0u;
 
   PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, 1);
 
   return new HostResolverImpl(HostCache::CreateDefaultCache(),
                               limits,
@@ skipping 343 matching lines @@
 
   ResolveRequest(HostResolver* resolver,
                  const HostResolver::RequestInfo& info,
                  Delegate* delegate)
       : info_(info), resolver_(resolver), delegate_(delegate) {
     // Start the request.
     int err = resolver->Resolve(
         info, &addrlist_,
         base::Bind(&ResolveRequest::OnLookupFinished,
                    base::Unretained(this)),
        &req_, BoundNetLog());

[mmenke, 2012/03/13 15:56:27]

nit:  While you're here, indent of the above line is off by one.

     EXPECT_EQ(ERR_IO_PENDING, err);
   }
 
+  ResolveRequest(HostResolver* resolver,

[mmenke, 2012/03/13 15:56:27]

Think you should comment that this is only for synchronous requests that should
be served from the cache or HOSTS file.

+                 const std::string& hostname,
+                 int port)
+      : info_(HostPortPair(hostname, port)),
+        resolver_(resolver),
+        delegate_(NULL)  {
+    // Start the request.
+    result_ = resolver->Resolve(
+        info_, &addrlist_,
+        base::Bind(&ResolveRequest::OnLookupFinished, base::Unretained(this)),
+        &req_, BoundNetLog());
+    EXPECT_NE(ERR_IO_PENDING, result_);
+  }
+
   void Cancel() {
     resolver_->CancelRequest(req_);
   }
 
   const std::string& hostname() const {
     return info_.hostname();
   }
 
   int port() const {
     return info_.port();
   }
 
   int result() const {
     return result_;
   }
 
   const AddressList& addrlist() const {
     return addrlist_;
   }
 
   HostResolver* resolver() const {
     return resolver_;
   }
 
  private:
   void OnLookupFinished(int result) {
+    EXPECT_TRUE(delegate_ != NULL);
+    if (delegate_ == NULL)
+      return;
     result_ = result;
     delegate_->OnCompleted(this);
   }
 
   // The request details.
   HostResolver::RequestInfo info_;
   HostResolver::RequestHandle req_;
 
   // The result of the resolve.
   int result_;
@@ skipping 399 matching lines @@
   req3.Cancel();
   req5.Cancel();
 
   // Ready, Set, GO!!!
   resolver_proc->Signal();
 
   // |verifier| will send quit message once all the requests have finished.
   MessageLoop::current()->Run();
 }
 
-// Helper class used by HostResolverImplTest.CanceledRequestsReleaseJobSlots.
+// Delegate which allows to wait for specific number of requests to complete.
+// Used by HostResolverImplTest.CanceledRequestsReleaseJobSlots and .DnsTask.
 class CountingDelegate : public ResolveRequest::Delegate {
  public:
   CountingDelegate() : num_completions_(0) {}
 
   virtual void OnCompleted(ResolveRequest* resolve) OVERRIDE {
     ++num_completions_;
     MessageLoop::current()->Quit();
   }
 
   unsigned num_completions() const { return num_completions_; }
 
+  void WaitUntil(unsigned completions) {

[mmenke, 2012/03/13 15:56:27]

optional nit:  Suggest "WaitFor" or "WaitForCompletion".

+    while (num_completions_ < completions)
+      MessageLoop::current()->Run();

[mmenke, 2012/03/13 15:56:27]

Rather than repeatedly entering message loops again, it's preferred just to quit
the message loop once when num_completions_ == completions.  It tends to make
for less flaky tests, or so the theory goes.

[szym, 2012/03/13 18:14:29]

I think it's the other way around: because anyone can Quit the MessageLoop, I
will have to have while statement around Run() anyway. It's true that
OnCompleted could be smarter and not Quit the loop until there's enough
completions, although that would require adding new fields and arguments.

[mmenke, 2012/03/13 18:27:50]

You don't want anyone else exiting the message loop.  That's actually one of the
reasons for having a single call to quit.  If someone else exits the loop, you
know there's a bug somewhere, assuming you add an EXPECT statement here as well.

+  }
+
  private:
   unsigned num_completions_;
 };
 
 TEST_F(HostResolverImplTest, CanceledRequestsReleaseJobSlots) {
   scoped_refptr<CountingHostResolverProc> resolver_proc(
       new CountingHostResolverProc(NULL));
 
   scoped_ptr<HostResolverImpl> host_resolver(
       CreateHostResolverImpl(resolver_proc));
@@ skipping 16 matching lines @@
   // Cancel all but last two.
   for (unsigned i = 0; i < requests.size() - 2; ++i) {
     requests[i]->Cancel();
   }
 
   EXPECT_TRUE(resolver_proc->WaitFor(kMaxJobs + 1));
   EXPECT_EQ(0u, delegate.num_completions());
 
   resolver_proc->SignalAll();
 
-  while (delegate.num_completions() < 2)
-    MessageLoop::current()->Run();
+  delegate.WaitUntil(2);
 
   EXPECT_EQ(0u, host_resolver->num_running_jobs_for_tests());
 }
 
 // Helper class used by HostResolverImplTest.CancelWithinCallback.
 class CancelWithinCallbackVerifier : public ResolveRequest::Delegate {
  public:
   CancelWithinCallbackVerifier()
       : req_to_cancel1_(NULL), req_to_cancel2_(NULL), num_completions_(0) {
   }
@@ skipping 810 matching lines @@
   // Resolve returns -4 to indicate that 3rd attempt has resolved the host.
   EXPECT_EQ(-4, callback.WaitForResult());
 
   resolver_proc->WaitForAllAttemptsToFinish(TimeDelta::FromMilliseconds(60000));
   MessageLoop::current()->RunAllPending();
 
   EXPECT_EQ(resolver_proc->total_attempts_resolved(), kTotalAttempts);
   EXPECT_EQ(resolver_proc->resolved_attempt_number(), kAttemptNumberToResolve);
 }
 
+DnsConfig CreateValidDnsConfig() {
+  IPAddressNumber dns_ip;
+  bool rv = ParseIPLiteralToNumber("192.168.1.0", &dns_ip);
+  EXPECT_TRUE(rv);
+
+  DnsConfig config;
+  config.nameservers.push_back(IPEndPoint(dns_ip,
+                                          dns_protocol::kDefaultPort));
+  EXPECT_TRUE(config.IsValid());
+  return config;
+}
+
+// Test successful and fallback resolutions in HostResolverImpl::DnsTask.
+TEST_F(HostResolverImplTest, DnsTask) {
+  scoped_refptr<RuleBasedHostResolverProc> resolver_proc(
+      new RuleBasedHostResolverProc(NULL));
+  scoped_ptr<HostResolverImpl> host_resolver(CreateHostResolverImpl(
+      resolver_proc));
+
+  resolver_proc->AddRule("er_succeed", "192.168.1.101");
+  resolver_proc->AddRule("nx_succeed", "192.168.1.102");
+  resolver_proc->AddSimulatedFailure("ok_fail");
+  resolver_proc->AddSimulatedFailure("er_fail");
+  resolver_proc->AddSimulatedFailure("nx_fail");
+
+  CountingDelegate delegate;
+
+  // Initially there is no config, so client should not be invoked.
+  ResolveRequest req1(host_resolver.get(), "ok_fail", 80, &delegate);
+
+  delegate.WaitUntil(1);
+  EXPECT_EQ(ERR_NAME_NOT_RESOLVED, req1.result());
+
+  host_resolver->set_dns_client_for_tests(
+      CreateMockDnsClient(CreateValidDnsConfig()));
+
+  ResolveRequest req2(host_resolver.get(), "ok_fail", 80, &delegate);
+  ResolveRequest req3(host_resolver.get(), "er_fail", 80, &delegate);
+  ResolveRequest req4(host_resolver.get(), "nx_fail", 80, &delegate);
+  ResolveRequest req5(host_resolver.get(), "er_succeed", 80, &delegate);
+  ResolveRequest req6(host_resolver.get(), "nx_succeed", 80, &delegate);
+
+  delegate.WaitUntil(6);
+  EXPECT_EQ(OK, req2.result());

[mmenke, 2012/03/13 15:56:27]

Should the OK checks be ASSERTs to prevent crashing on regressions?

+  // Resolved by MockDnsClient.
+  EXPECT_EQ("127.0.0.1", NetAddressToString(req2.addrlist().head()));
+  EXPECT_EQ(ERR_NAME_NOT_RESOLVED, req3.result());
+  EXPECT_EQ(ERR_NAME_NOT_RESOLVED, req4.result());
+  EXPECT_EQ(OK, req5.result());
+  EXPECT_EQ("192.168.1.101", NetAddressToString(req5.addrlist().head()));
+  EXPECT_EQ(OK, req6.result());
+  EXPECT_EQ("192.168.1.102", NetAddressToString(req6.addrlist().head()));
+}
+
+TEST_F(HostResolverImplTest, ServeFromHosts) {
+  scoped_refptr<RuleBasedHostResolverProc> resolver_proc(
+      new RuleBasedHostResolverProc(NULL));
+  MockDnsConfigService* config_service = new MockDnsConfigService();
+  scoped_ptr<HostResolverImpl> host_resolver(CreateHostResolverImplWithDns(
+      resolver_proc,
+      scoped_ptr<DnsConfigService>(config_service)));
+
+  resolver_proc->AddSimulatedFailure("*");
+
+  DnsConfig config = CreateValidDnsConfig();
+  host_resolver->set_dns_client_for_tests(CreateMockDnsClient(config));
+
+  CountingDelegate delegate;
+
+  ResolveRequest req1(host_resolver.get(), "er_ipv4", 80, &delegate);
+  delegate.WaitUntil(1);
+  EXPECT_EQ(ERR_NAME_NOT_RESOLVED, req1.result());
+
+  IPAddressNumber local_ipv4, local_ipv6;
+  ASSERT_TRUE(ParseIPLiteralToNumber("127.0.0.1", &local_ipv4));
+  ASSERT_TRUE(ParseIPLiteralToNumber("::1", &local_ipv6));
+
+  DnsHosts hosts;
+  hosts[DnsHostsKey("er_ipv4", ADDRESS_FAMILY_IPV4)] = local_ipv4;
+  hosts[DnsHostsKey("er_ipv6", ADDRESS_FAMILY_IPV6)] = local_ipv6;

[mmenke, 2012/03/13 15:56:27]

Suggest you add two or three ADDRESS_FAMILY_UNSPEC here as well - one with ipv6
address only (like "er_ipv6"), and one with ipv4 and ipv6 addresses.  Maybe also
one with ipv4 only.

[szym, 2012/03/13 18:14:29]

DnsHosts will not have keys with ADDRESS_FAMILY_UNSPEC, unless we update
dns_hosts.cc to do that (which would increase the size of DnsHosts between 1.5x
and 2x).

[mmenke, 2012/03/13 18:27:50]

Sorry, I was a little confused...  You should have a hostname with ipv6 and ipv4
IPs, and make sure you only get the IPv4 one, as per
HostResolverImpl::ServeFromHosts.

May also be a good idea to run a request for a ADDRESS_FAMILY_IPV4 address only
(Instead of UNSPECIFIED), just for the sake of completeness.

[szym, 2012/03/13 19:04:00]

I don't want to test the IPv4 preference unless we specify that explicitly (and
therefore close the respective bug). A test that requires such behavior would
indicate that this is "how it's supposed to be" which would be detrimental in
future development.

+
+  config_service->ChangeConfig(config);
+  config_service->ChangeHosts(hosts);
+
+  ResolveRequest req2(host_resolver.get(), "er_ipv4", 80);
+  EXPECT_EQ(OK, req2.result());
+  EXPECT_EQ("127.0.0.1", NetAddressToString(req2.addrlist().head()));
+  ResolveRequest req3(host_resolver.get(), "er_ipv6", 80);
+  EXPECT_EQ(OK, req3.result());
+  EXPECT_EQ("::1", NetAddressToString(req3.addrlist().head()));
+}
+
+// TODO(szym): Test AbortAllInProgressJobs due to DnsConfig change.
+
 // TODO(cbentzel): Test a mix of requests with different HostResolverFlags.
 
+}  // namespace
 }  // namespace net