diff options
31 files changed, 974 insertions, 1673 deletions
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc index d2f6006..0288173 100644 --- a/chrome/browser/io_thread.cc +++ b/chrome/browser/io_thread.cc @@ -39,12 +39,10 @@ #include "net/base/default_origin_bound_cert_store.h" #include "net/base/host_cache.h" #include "net/base/host_resolver.h" -#include "net/base/host_resolver_impl.h" #include "net/base/mapped_host_resolver.h" #include "net/base/net_util.h" #include "net/base/origin_bound_cert_service.h" #include "net/base/sdch_manager.h" -#include "net/dns/async_host_resolver.h" #include "net/ftp/ftp_network_layer.h" #include "net/http/http_auth_filter.h" #include "net/http/http_auth_handler_factory.h" @@ -137,17 +135,9 @@ net::HostResolver* CreateGlobalHostResolver(net::NetLog* net_log) { } net::HostResolver* global_host_resolver = NULL; - if (command_line.HasSwitch(switches::kDnsServer)) { - std::string dns_ip_string = - command_line.GetSwitchValueASCII(switches::kDnsServer); - net::IPAddressNumber dns_ip_number; - if (net::ParseIPLiteralToNumber(dns_ip_string, &dns_ip_number)) { - global_host_resolver = - net::CreateAsyncHostResolver(parallelism, dns_ip_number, net_log); - } else { - LOG(ERROR) << "Invalid IP address specified for --dns-server: " - << dns_ip_string; - } + if (command_line.HasSwitch(switches::kEnableAsyncDns)) { + global_host_resolver = + net::CreateAsyncHostResolver(parallelism, retry_attempts, net_log); } if (!global_host_resolver) { diff --git a/chrome/browser/resources/net_internals/events_view.css b/chrome/browser/resources/net_internals/events_view.css index 4047a82..b60812b 100644 --- a/chrome/browser/resources/net_internals/events_view.css +++ b/chrome/browser/resources/net_internals/events_view.css @@ -73,10 +73,7 @@ } #events-view-source-list-tbody .source_HOST_RESOLVER_IMPL_JOB, -#events-view-source-list-tbody .source_HOST_RESOLVER_IMPL_REQUEST, -#events-view-source-list-tbody .source_HOST_RESOLVER_IMPL_PROC_TASK, -#events-view-source-list-tbody .source_ASYNC_HOST_RESOLVER_REQUEST, -#events-view-source-list-tbody .source_DNS_TRANSACTION { +#events-view-source-list-tbody .source_HOST_RESOLVER_IMPL_REQUEST { color: #206060; } diff --git a/chrome/browser/resources/net_internals/source_entry.js b/chrome/browser/resources/net_internals/source_entry.js index dcab8d0..f4d3ce0 100644 --- a/chrome/browser/resources/net_internals/source_entry.js +++ b/chrome/browser/resources/net_internals/source_entry.js @@ -114,14 +114,15 @@ var SourceEntry = (function() { case LogSourceType.UDP_SOCKET: if (e.params.address != undefined) { this.description_ = e.params.address; - // If the parent of |this| is a DNS_TRANSACTION, use - // '<DNS Server IP> [<DNS we're resolving>]'. + // If the parent of |this| is a HOST_RESOLVER_IMPL_JOB, use + // '<DNS Server IP> [<host we're resolving>]'. if (this.entries_[0].type == LogEventType.SOCKET_ALIVE && this.entries_[0].params.source_dependency != undefined) { var parentId = this.entries_[0].params.source_dependency.id; var parent = g_browser.sourceTracker.getSourceEntry(parentId); if (parent && - parent.getSourceType() == LogSourceType.DNS_TRANSACTION && + parent.getSourceType() == + LogSourceType.HOST_RESOLVER_IMPL_JOB && parent.getDescription().length > 0) { this.description_ += ' [' + parent.getDescription() + ']'; } diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc index 16f1018..8797b1e 100644 --- a/chrome/common/chrome_switches.cc +++ b/chrome/common/chrome_switches.cc @@ -408,9 +408,6 @@ const char kDnsLogDetails[] = "dns-log-details"; // Disables prefetching of DNS information. const char kDnsPrefetchDisable[] = "dns-prefetch-disable"; -// Use the specified DNS server for raw DNS resolution. -const char kDnsServer[] = "dns-server"; - // Replaces the download shelf with a new experimental UI. const char kDownloadsNewUI[] = "downloads-new-ui"; @@ -422,6 +419,9 @@ const char kDumpHistogramsOnExit[] = "dump-histograms-on-exit"; // Enables AeroPeek for each tab. (This switch only works on Windows 7). const char kEnableAeroPeekTabs[] = "enable-aero-peek-tabs"; +// Enables the experimental asynchronous DNS client. +const char kEnableAsyncDns[] = "enable-async-dns"; + // Enables asynchronous spellchecking features for all time. // Enabling this feature also enables unified spellchecking. const char kEnableAsynchronousSpellChecking[] = diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h index 8ce11b8..d140670 100644 --- a/chrome/common/chrome_switches.h +++ b/chrome/common/chrome_switches.h @@ -120,10 +120,10 @@ extern const char kDiskCacheDir[]; extern const char kDiskCacheSize[]; extern const char kDnsLogDetails[]; extern const char kDnsPrefetchDisable[]; -extern const char kDnsServer[]; extern const char kDownloadsNewUI[]; extern const char kDumpHistogramsOnExit[]; extern const char kEnableAeroPeekTabs[]; +extern const char kEnableAsyncDns[]; extern const char kEnableAsynchronousSpellChecking[]; extern const char kEnableAuthNegotiatePort[]; extern const char kEnableAutofillFeedback[]; diff --git a/net/base/address_list.cc b/net/base/address_list.cc index 5442687..3c74f5d 100644 --- a/net/base/address_list.cc +++ b/net/base/address_list.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// 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. @@ -112,7 +112,7 @@ AddressList& AddressList::operator=(const AddressList& addresslist) { // static AddressList AddressList::CreateFromIPAddressList( const IPAddressList& addresses, - uint16 port) { + const std::string& canonical_name) { DCHECK(!addresses.empty()); struct addrinfo* head = NULL; struct addrinfo* next = NULL; @@ -121,13 +121,15 @@ AddressList AddressList::CreateFromIPAddressList( it != addresses.end(); ++it) { if (head == NULL) { head = next = CreateAddrInfo(*it, false); + if (!canonical_name.empty()) { + head->ai_canonname = do_strdup(canonical_name.c_str()); + } } else { next->ai_next = CreateAddrInfo(*it, false); next = next->ai_next; } } - SetPortForAllAddrinfos(head, port); return AddressList(new Data(head, false)); } diff --git a/net/base/address_list.h b/net/base/address_list.h index e03e5c5..edcdf09 100644 --- a/net/base/address_list.h +++ b/net/base/address_list.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// 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. @@ -31,7 +31,7 @@ class NET_EXPORT AddressList { // Creates an address list for a list of IP literals. static AddressList CreateFromIPAddressList( const IPAddressList& addresses, - uint16 port); + const std::string& canonical_name); // Creates an address list for a single IP literal. static AddressList CreateFromIPAddress( diff --git a/net/base/address_list_unittest.cc b/net/base/address_list_unittest.cc index 3dda87a..7eda7df 100644 --- a/net/base/address_list_unittest.cc +++ b/net/base/address_list_unittest.cc @@ -216,7 +216,7 @@ TEST(AddressListTest, IPLiteralConstructor) { const struct addrinfo* good_ai = expected_list.head(); IPAddressNumber ip_number; - ParseIPLiteralToNumber(tests[i].ip_address, &ip_number); + ASSERT_TRUE(ParseIPLiteralToNumber(tests[i].ip_address, &ip_number)); AddressList test_list = AddressList::CreateFromIPAddressWithCname( ip_number, 80, true); const struct addrinfo* test_ai = test_list.head(); @@ -305,18 +305,21 @@ TEST(AddressListTest, CreateFromIPAddressList) { sizeof(struct in_addr), }, }; - const uint16 kPort = 80; + const std::string kCanonicalName = "canonical.example.com"; // Construct a list of ip addresses. IPAddressList ip_list; for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) { IPAddressNumber ip_number; - ParseIPLiteralToNumber(tests[i].ip_address, &ip_number); + ASSERT_TRUE(ParseIPLiteralToNumber(tests[i].ip_address, &ip_number)); ip_list.push_back(ip_number); } - AddressList test_list = AddressList::CreateFromIPAddressList(ip_list, kPort); - EXPECT_EQ(kPort, test_list.GetPort()); + AddressList test_list = AddressList::CreateFromIPAddressList(ip_list, + kCanonicalName); + std::string canonical_name; + EXPECT_TRUE(test_list.GetCanonicalName(&canonical_name)); + EXPECT_EQ(kCanonicalName, canonical_name); // Make sure that CreateFromIPAddressList has created an addrinfo // chain of exactly the same length as the |tests| with correct content. diff --git a/net/base/host_resolver.h b/net/base/host_resolver.h index 02b61af..e375961 100644 --- a/net/base/host_resolver.h +++ b/net/base/host_resolver.h @@ -188,10 +188,12 @@ NET_EXPORT HostResolver* CreateNonCachingSystemHostResolver( size_t max_retry_attempts, NetLog* net_log); -// Creates a HostResolver implementation that sends actual DNS queries to -// the specified DNS server and parses response and returns results. +// As above, but the HostResolver will use the asynchronous DNS client in +// DnsTransaction, which will be configured using DnsConfigService to match +// the system DNS settings. If the client fails, the resolver falls back to +// the global HostResolverProc. NET_EXPORT HostResolver* CreateAsyncHostResolver(size_t max_concurrent_resolves, - const IPAddressNumber& dns_ip, + size_t max_retry_attempts, NetLog* net_log); } // namespace net diff --git a/net/base/host_resolver_impl.cc b/net/base/host_resolver_impl.cc index d9330e2..02d2061 100644 --- a/net/base/host_resolver_impl.cc +++ b/net/base/host_resolver_impl.cc @@ -24,12 +24,14 @@ #include "base/message_loop_proxy.h" #include "base/metrics/field_trial.h" #include "base/metrics/histogram.h" +#include "base/rand_util.h" #include "base/stl_util.h" #include "base/string_util.h" #include "base/threading/worker_pool.h" #include "base/time.h" #include "base/utf_string_conversions.h" #include "base/values.h" +#include "net/base/address_family.h" #include "net/base/address_list.h" #include "net/base/address_list_net_log_param.h" #include "net/base/dns_reloader.h" @@ -38,6 +40,12 @@ #include "net/base/net_errors.h" #include "net/base/net_log.h" #include "net/base/net_util.h" +#include "net/dns/dns_config_service.h" +#include "net/dns/dns_protocol.h" +#include "net/dns/dns_response.h" +#include "net/dns/dns_session.h" +#include "net/dns/dns_transaction.h" +#include "net/socket/client_socket_factory.h" #if defined(OS_WIN) #include "net/base/winsock_init.h" @@ -54,6 +62,9 @@ const size_t kMaxHostLength = 4096; // Default TTL for successful resolutions with ProcTask. const unsigned kCacheEntryTTLSeconds = 60; +// Default TTL for unsuccessful resolutions with ProcTask. +const unsigned kNegativeCacheEntryTTLSeconds = 0; + // Maximum of 8 concurrent resolver threads (excluding retries). // Some routers (or resolvers) appear to start to provide host-not-found if // too many simultaneous resolutions are pending. This number needs to be @@ -66,9 +77,9 @@ static const size_t kDefaultMaxProcTasks = 8u; // // However since we allocated the AddressList ourselves we can safely // do this optimization and avoid reallocating the list. -void MutableSetPort(int port, AddressList* addrlist) { +void MutableSetPort(int port, AddressList* addr_list) { struct addrinfo* mutable_head = - const_cast<struct addrinfo*>(addrlist->head()); + const_cast<struct addrinfo*>(addr_list->head()); SetPortForAllAddrinfos(mutable_head, port); } @@ -144,22 +155,20 @@ class CallSystemHostResolverProc : public HostResolverProc { virtual int Resolve(const std::string& hostname, AddressFamily address_family, HostResolverFlags host_resolver_flags, - AddressList* addrlist, + AddressList* addr_list, int* os_error) OVERRIDE { return SystemHostResolverProc(hostname, address_family, host_resolver_flags, - addrlist, + addr_list, os_error); } }; // Extra parameters to attach to the NetLog when the resolve failed. -class HostResolveFailedParams : public NetLog::EventParameters { +class ProcTaskFailedParams : public NetLog::EventParameters { public: - HostResolveFailedParams(uint32 attempt_number, - int net_error, - int os_error) + ProcTaskFailedParams(uint32 attempt_number, int net_error, int os_error) : attempt_number_(attempt_number), net_error_(net_error), os_error_(os_error) { @@ -201,6 +210,26 @@ class HostResolveFailedParams : public NetLog::EventParameters { const int os_error_; }; +// Extra parameters to attach to the NetLog when the DnsTask failed. +class DnsTaskFailedParams : public NetLog::EventParameters { + public: + DnsTaskFailedParams(int net_error, int dns_error) + : net_error_(net_error), dns_error_(dns_error) { + } + + virtual Value* ToValue() const OVERRIDE { + DictionaryValue* dict = new DictionaryValue(); + dict->SetInteger("net_error", net_error_); + if (dns_error_) + dict->SetInteger("dns_error", dns_error_); + return dict; + } + + private: + const int net_error_; + const int dns_error_; +}; + // Parameters representing the information in a RequestInfo object, along with // the associated NetLog::Source. class RequestInfoParameters : public NetLog::EventParameters { @@ -229,8 +258,7 @@ class RequestInfoParameters : public NetLog::EventParameters { const NetLog::Source source_; }; -// Parameters associated with the creation of a HostResolverImpl::Job -// or a HostResolverImpl::ProcTask. +// Parameters associated with the creation of a HostResolverImpl::Job. class JobCreationParameters : public NetLog::EventParameters { public: JobCreationParameters(const std::string& host, @@ -290,14 +318,9 @@ void LogStartRequest(const BoundNetLog& source_net_log, void LogFinishRequest(const BoundNetLog& source_net_log, const BoundNetLog& request_net_log, const HostResolver::RequestInfo& info, - int net_error, - int os_error) { - scoped_refptr<NetLog::EventParameters> params; - if (net_error != OK) { - params = new HostResolveFailedParams(0, net_error, os_error); - } - - request_net_log.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST, params); + int net_error) { + request_net_log.EndEventWithNetErrorCode( + NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST, net_error); source_net_log.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL, NULL); } @@ -315,8 +338,8 @@ void LogCancelRequest(const BoundNetLog& source_net_log, // Keeps track of the highest priority. class PriorityTracker { public: - PriorityTracker() - : highest_priority_(IDLE), total_count_(0) { + explicit PriorityTracker(RequestPriority priority) + : highest_priority_(priority), total_count_(0) { memset(counts_, 0, sizeof(counts_)); } @@ -361,7 +384,8 @@ class PriorityTracker { HostResolver* CreateHostResolver(size_t max_concurrent_resolves, size_t max_retry_attempts, - bool use_cache, + HostCache* cache, + scoped_ptr<DnsConfigService> config_service, NetLog* net_log) { if (max_concurrent_resolves == HostResolver::kDefaultParallelism) max_concurrent_resolves = kDefaultMaxProcTasks; @@ -372,9 +396,10 @@ HostResolver* CreateHostResolver(size_t max_concurrent_resolves, PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, max_concurrent_resolves); HostResolverImpl* resolver = new HostResolverImpl( - use_cache ? HostCache::CreateDefaultCache() : NULL, + cache, limits, HostResolverImpl::ProcTaskParams(NULL, max_retry_attempts), + config_service.Pass(), net_log); return resolver; @@ -389,7 +414,8 @@ HostResolver* CreateSystemHostResolver(size_t max_concurrent_resolves, NetLog* net_log) { return CreateHostResolver(max_concurrent_resolves, max_retry_attempts, - true /* use_cache */, + HostCache::CreateDefaultCache(), + scoped_ptr<DnsConfigService>(NULL), net_log); } @@ -398,7 +424,21 @@ HostResolver* CreateNonCachingSystemHostResolver(size_t max_concurrent_resolves, NetLog* net_log) { return CreateHostResolver(max_concurrent_resolves, max_retry_attempts, - false /* use_cache */, + NULL, + scoped_ptr<DnsConfigService>(NULL), + net_log); +} + +HostResolver* CreateAsyncHostResolver(size_t max_concurrent_resolves, + size_t max_retry_attempts, + NetLog* net_log) { + scoped_ptr<DnsConfigService> config_service = + DnsConfigService::CreateSystemService(); + config_service->Watch(); + return CreateHostResolver(max_concurrent_resolves, + max_retry_attempts, + HostCache::CreateDefaultCache(), + config_service.Pass(), net_log); } @@ -440,9 +480,9 @@ class HostResolverImpl::Request { } // Prepare final AddressList and call completion callback. - void OnComplete(int error, const AddressList& addrlist) { + void OnComplete(int error, const AddressList& addr_list) { if (error == OK) - *addresses_ = CreateAddressListUsingPort(addrlist, info_.port()); + *addresses_ = CreateAddressListUsingPort(addr_list, info_.port()); CompletionCallback callback = callback_; MarkAsCanceled(); callback.Run(error); @@ -505,7 +545,8 @@ class HostResolverImpl::Request { class HostResolverImpl::ProcTask : public base::RefCountedThreadSafe<HostResolverImpl::ProcTask> { public: - typedef base::Callback<void(int, int, const AddressList&)> Callback; + typedef base::Callback<void(int net_error, + const AddressList& addr_list)> Callback; ProcTask(const Key& key, const ProcTaskParams& params, @@ -519,22 +560,14 @@ class HostResolverImpl::ProcTask completed_attempt_number_(0), completed_attempt_error_(ERR_UNEXPECTED), had_non_speculative_request_(false), - net_log_(BoundNetLog::Make( - job_net_log.net_log(), - NetLog::SOURCE_HOST_RESOLVER_IMPL_PROC_TASK)) { + net_log_(job_net_log) { if (!params_.resolver_proc) params_.resolver_proc = HostResolverProc::GetDefault(); // If default is unset, use the system proc. if (!params_.resolver_proc) params_.resolver_proc = new CallSystemHostResolverProc(); - job_net_log.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_CREATE_PROC_TASK, - new NetLogSourceParameter("source_dependency", - net_log_.source())); - - net_log_.BeginEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK, - new JobCreationParameters(key_.hostname, - job_net_log.source())); + net_log_.BeginEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK, NULL); } void Start() { @@ -551,8 +584,6 @@ class HostResolverImpl::ProcTask if (was_canceled()) return; - net_log_.AddEvent(NetLog::TYPE_CANCELLED, NULL); - callback_.Reset(); net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK, NULL); @@ -621,7 +652,6 @@ class HostResolverImpl::ProcTask AddressList results; int os_error = 0; // Running on the worker thread - int error = params_.resolver_proc->Resolve(key_.hostname, key_.address_family, key_.host_resolver_flags, @@ -657,14 +687,13 @@ class HostResolverImpl::ProcTask bool was_retry_attempt = attempt_number > 1; // Ideally the following code would be part of host_resolver_proc.cc, - // however it isn't safe to call NetworkChangeNotifier from worker - // threads. So we do it here on the IO thread instead. + // however it isn't safe to call NetworkChangeNotifier from worker threads. + // So we do it here on the IO thread instead. if (error != OK && NetworkChangeNotifier::IsOffline()) error = ERR_INTERNET_DISCONNECTED; - // If this is the first attempt that is finishing later, then record - // data for the first attempt. Won't contaminate with retry attempt's - // data. + // If this is the first attempt that is finishing later, then record data + // for the first attempt. Won't contaminate with retry attempt's data. if (!was_retry_attempt) RecordPerformanceHistograms(start_time, error, os_error); @@ -675,7 +704,7 @@ class HostResolverImpl::ProcTask scoped_refptr<NetLog::EventParameters> params; if (error != OK) { - params = new HostResolveFailedParams(attempt_number, error, os_error); + params = new ProcTaskFailedParams(attempt_number, error, os_error); } else { params = new NetLogIntegerParameter("attempt_number", attempt_number); } @@ -696,13 +725,13 @@ class HostResolverImpl::ProcTask } if (error != OK) { - params = new HostResolveFailedParams(0, error, os_error); + params = new ProcTaskFailedParams(0, error, os_error); } else { params = new AddressListNetLogParam(results_); } net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK, params); - callback_.Run(error, os_error, results_); + callback_.Run(error, results_); } void RecordPerformanceHistograms(const base::TimeTicks& start_time, @@ -891,6 +920,8 @@ class HostResolverImpl::ProcTask //----------------------------------------------------------------------------- // Represents a request to the worker pool for a "probe for IPv6 support" call. +// +// TODO(szym): This could also be replaced with PostTaskAndReply and Callbacks. class HostResolverImpl::IPv6ProbeJob : public base::RefCountedThreadSafe<HostResolverImpl::IPv6ProbeJob> { public: @@ -958,61 +989,130 @@ class HostResolverImpl::IPv6ProbeJob //----------------------------------------------------------------------------- +// Resolves the hostname using DnsTransaction. +// TODO(szym): This could be moved to separate source file as well. +class HostResolverImpl::DnsTask { + public: + typedef base::Callback<void(int net_error, + const AddressList& addr_list, + base::TimeDelta ttl)> Callback; + + DnsTask(DnsTransactionFactory* factory, + const Key& key, + const Callback& callback, + const BoundNetLog& job_net_log) + : callback_(callback), net_log_(job_net_log) { + DCHECK(factory); + DCHECK(!callback.is_null()); + + // For now we treat ADDRESS_FAMILY_UNSPEC as if it was IPV4. + uint16 qtype = (key.address_family == ADDRESS_FAMILY_IPV6) + ? dns_protocol::kTypeAAAA + : dns_protocol::kTypeA; + // TODO(szym): Implement "happy eyeballs". + transaction_ = factory->CreateTransaction( + key.hostname, + qtype, + base::Bind(&DnsTask::OnTransactionComplete, base::Unretained(this)), + net_log_); + DCHECK(transaction_.get()); + } + + int Start() { + net_log_.BeginEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK, NULL); + return transaction_->Start(); + } + + void OnTransactionComplete(DnsTransaction* transaction, + int net_error, + const DnsResponse* response) { + // TODO(szym): Record performance histograms. + // Run |callback_| last since the owning Job will then delete this DnsTask. + DnsResponse::Result result = DnsResponse::DNS_SUCCESS; + if (net_error == OK) { + AddressList addr_list; + base::TimeDelta ttl; + result = response->ParseToAddressList(&addr_list, &ttl); + if (result == DnsResponse::DNS_SUCCESS) { + net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK, + new AddressListNetLogParam(addr_list)); + callback_.Run(net_error, addr_list, ttl); + return; + } + net_error = ERR_DNS_MALFORMED_RESPONSE; + } + net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK, + new DnsTaskFailedParams(net_error, result)); + callback_.Run(net_error, AddressList(), base::TimeDelta()); + } + + private: + // The listener to the results of this DnsTask. + Callback callback_; + + const BoundNetLog net_log_; + + scoped_ptr<DnsTransaction> transaction_; +}; + +//----------------------------------------------------------------------------- + // Aggregates all Requests for the same Key. Dispatched via PriorityDispatch. -// Spawns ProcTask when started. class HostResolverImpl::Job : public PrioritizedDispatcher::Job { public: // Creates new job for |key| where |request_net_log| is bound to the - // request that spawned it. + // request that spawned it and |priority| is the initial priority. Job(HostResolverImpl* resolver, const Key& key, - const BoundNetLog& request_net_log) + const BoundNetLog& request_net_log, + RequestPriority priority) : resolver_(resolver->AsWeakPtr()), key_(key), + priority_tracker_(priority), had_non_speculative_request_(false), net_log_(BoundNetLog::Make(request_net_log.net_log(), - NetLog::SOURCE_HOST_RESOLVER_IMPL_JOB)), - net_error_(ERR_IO_PENDING), - os_error_(0) { + NetLog::SOURCE_HOST_RESOLVER_IMPL_JOB)) { request_net_log.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_CREATE_JOB, NULL); net_log_.BeginEvent( NetLog::TYPE_HOST_RESOLVER_IMPL_JOB, make_scoped_refptr(new JobCreationParameters( key_.hostname, request_net_log.source()))); + + handle_ = resolver_->dispatcher_.Add(this, priority); } virtual ~Job() { - if (net_error_ == ERR_IO_PENDING) { - if (is_running()) { - DCHECK_EQ(ERR_IO_PENDING, net_error_); + if (is_running()) { + // |resolver_| was destroyed with this Job still in flight. + // Clean-up, record in the log, but don't run any callbacks. + if (is_proc_running()) { proc_task_->Cancel(); proc_task_ = NULL; - net_error_ = ERR_ABORTED; - } else { - net_log_.AddEvent(NetLog::TYPE_CANCELLED, NULL); - net_error_ = OK; // For NetLog. } + net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB, + ERR_ABORTED); + } else if (is_queued()) { + // This Job was cancelled without running. + // TODO(szym): is there's any benefit in having this distinction? + net_log_.AddEvent(NetLog::TYPE_CANCELLED, NULL); + net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB, NULL); + } + // else CompleteRequests logged EndEvent. - for (RequestsList::const_iterator it = requests_.begin(); - it != requests_.end(); ++it) { - Request* req = *it; - if (req->was_canceled()) - continue; - DCHECK_EQ(this, req->job()); - LogCancelRequest(req->source_net_log(), req->request_net_log(), - req->info()); - } + // Log any remaining Requests as cancelled. + for (RequestsList::const_iterator it = requests_.begin(); + it != requests_.end(); ++it) { + Request* req = *it; + if (req->was_canceled()) + continue; + DCHECK_EQ(this, req->job()); + LogCancelRequest(req->source_net_log(), req->request_net_log(), + req->info()); } - net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB, - net_error_); STLDeleteElements(&requests_); } - HostResolverImpl* resolver() const { - return resolver_; - } - RequestPriority priority() const { return priority_tracker_.highest_priority(); } @@ -1026,26 +1126,14 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job { return key_; } - int net_error() const { - return net_error_; - } - - // Used by HostResolverImpl with |dispatcher_|. - const PrioritizedDispatcher::Handle& handle() const { - return handle_; - } - - void set_handle(const PrioritizedDispatcher::Handle& handle) { - handle_ = handle; + bool is_queued() const { + return !handle_.is_null(); } - // The Job will own |req| and destroy it in ~Job. - void AddRequest(Request* req) { + void AddRequest(scoped_ptr<Request> req) { DCHECK_EQ(key_.hostname, req->info().hostname()); req->set_job(this); - requests_.push_back(req); - priority_tracker_.Add(req->info().priority()); req->request_net_log().AddEvent( @@ -1064,6 +1152,11 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job { if (proc_task_) proc_task_->set_had_non_speculative_request(); } + + requests_.push_back(req.release()); + + if (!handle_.is_null()) + handle_ = resolver_->dispatcher_.ChangePriority(handle_, priority()); } void CancelRequest(Request* req) { @@ -1078,44 +1171,80 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job { NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_REQUEST_DETACH, make_scoped_refptr(new JobAttachParameters( req->request_net_log().source(), priority()))); + + if (!handle_.is_null()) { + if (num_active_requests() > 0) { + handle_ = resolver_->dispatcher_.ChangePriority(handle_, priority()); + } else { + resolver_->dispatcher_.Cancel(handle_); + handle_.Reset(); + } + } } // Aborts and destroys the job, completes all requests as aborted. + // The caller should clean up. void Abort() { // Job should only be aborted if it's running. DCHECK(is_running()); - proc_task_->Cancel(); - proc_task_ = NULL; - net_error_ = ERR_ABORTED; - os_error_ = 0; - CompleteRequests(AddressList()); + if (is_proc_running()) { + proc_task_->Cancel(); + proc_task_ = NULL; + } + dns_task_.reset(); + CompleteRequests(ERR_ABORTED, AddressList(), base::TimeDelta()); } - bool is_running() const { + bool is_dns_running() const { + return dns_task_.get() != NULL; + } + + bool is_proc_running() const { return proc_task_.get() != NULL; } + bool is_running() const { + return is_dns_running() || is_proc_running(); + } + // Called by HostResolverImpl when this job is evicted due to queue overflow. void OnEvicted() { // Must not be running. DCHECK(!is_running()); - handle_ = PrioritizedDispatcher::Handle(); + handle_.Reset(); net_log_.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_EVICTED, NULL); + scoped_ptr<Job> self_deleter(this); + resolver_->RemoveJob(this); + // This signals to CompleteRequests that this job never ran. - net_error_ = ERR_HOST_RESOLVER_QUEUE_TOO_LARGE; - os_error_ = 0; - CompleteRequests(AddressList()); + CompleteRequests(ERR_HOST_RESOLVER_QUEUE_TOO_LARGE, + AddressList(), + base::TimeDelta()); } // PriorityDispatch::Job interface. virtual void Start() OVERRIDE { DCHECK(!is_running()); - handle_ = PrioritizedDispatcher::Handle(); + handle_.Reset(); net_log_.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_STARTED, NULL); + if (resolver_->dns_transaction_factory_.get()) { + StartDnsTask(); + } else { + StartProcTask(); + } + } + + private: + // TODO(szym): Since DnsTransaction does not consume threads, we can increase + // the limits on |dispatcher_|. But in order to keep the number of WorkerPool + // threads low, we will need to use an "inner" PrioritizedDispatcher with + // tighter limits. + void StartProcTask() { + DCHECK(!dns_task_.get()); proc_task_ = new ProcTask( key_, resolver_->proc_params_, @@ -1129,33 +1258,82 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job { proc_task_->Start(); } - private: // Called by ProcTask when it completes. - void OnProcTaskComplete(int net_error, int os_error, - const AddressList& addrlist) { - DCHECK(is_running()); + void OnProcTaskComplete(int net_error, const AddressList& addr_list) { + DCHECK(is_proc_running()); + // |addr_list| will be destroyed once we destroy |proc_task_|. + AddressList list = addr_list; proc_task_ = NULL; - net_error_ = net_error; - os_error_ = os_error; - // We are the only consumer of |addrlist|, so we can safely change the port - // without copy-on-write. This pays off, when job has only one request. - AddressList list = addrlist; - if (net_error == OK && !requests_.empty()) - MutableSetPort(requests_.front()->info().port(), &list); - CompleteRequests(list); - } - - // Completes all Requests. Calls OnJobFinished and deletes self. - void CompleteRequests(const AddressList& addrlist) { - CHECK(resolver_); + base::TimeDelta ttl = base::TimeDelta::FromSeconds( + kNegativeCacheEntryTTLSeconds); + if (net_error == OK) + ttl = base::TimeDelta::FromSeconds(kCacheEntryTTLSeconds); // This job must be removed from resolver's |jobs_| now to make room for a // new job with the same key in case one of the OnComplete callbacks decides // to spawn one. Consequently, the job deletes itself when CompleteRequests // is done. scoped_ptr<Job> self_deleter(this); - resolver_->OnJobFinished(this, addrlist); + resolver_->OnJobFinished(this, net_error, list, ttl); + + CompleteRequests(net_error, list, ttl); + } + + void StartDnsTask() { + dns_task_.reset(new DnsTask( + resolver_->dns_transaction_factory_.get(), + key_, + base::Bind(&Job::OnDnsTaskComplete, base::Unretained(this)), + net_log_)); + + int rv = dns_task_->Start(); + if (rv != ERR_IO_PENDING) { + DCHECK_NE(OK, rv); + dns_task_.reset(); + StartProcTask(); + } + } + + // Called by DnsTask when it completes. + void OnDnsTaskComplete(int net_error, + const AddressList& addr_list, + base::TimeDelta ttl) { + DCHECK(is_dns_running()); + // |addr_list| will be destroyed once we destroy |dns_task_|. + AddressList list = addr_list; + dns_task_.reset(); + + if (net_error != OK) { + // TODO(szym): Some net errors indicate lack of connectivity. Starting + // ProcTask in that case is a waste of time. + StartProcTask(); + return; + } + + // As in OnProcTaskComplete + scoped_ptr<Job> self_deleter(this); + resolver_->OnJobFinished(this, net_error, list, ttl); + + CompleteRequests(net_error, list, ttl); + } + + // Completes all Requests. Calls OnJobFinished and deletes self. + void CompleteRequests(int net_error, + const AddressList& addr_list, + base::TimeDelta ttl) { + CHECK(resolver_); + DCHECK(!is_running()); + DCHECK(!is_queued()); + + // We are the only consumer of |addr_list|, so we can safely change the port + // without copy-on-write. This pays off, when job has only one request. + AddressList list = addr_list; + if (net_error == OK && !requests_.empty()) + MutableSetPort(requests_.front()->info().port(), &list); + + net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB, + net_error); // Complete all of the requests that were attached to the job. for (RequestsList::const_iterator it = requests_.begin(); @@ -1168,9 +1346,9 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job { DCHECK_EQ(this, req->job()); // Update the net log and notify registered observers. LogFinishRequest(req->source_net_log(), req->request_net_log(), - req->info(), net_error_, os_error_); + req->info(), net_error); - req->OnComplete(net_error_, addrlist); + req->OnComplete(net_error, list); // Check if the resolver was destroyed as a result of running the // callback. If it was, we could continue, but we choose to bail. @@ -1179,7 +1357,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job { } } - // Used to call OnJobFinished and RemoveJob. + // Used to call OnJobFinished. base::WeakPtr<HostResolverImpl> resolver_; Key key_; @@ -1191,17 +1369,16 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job { BoundNetLog net_log_; - // Store result here in case the job fails fast in Resolve(). - int net_error_; - int os_error_; - - // A ProcTask created and started when this Job is dispatched.. + // Resolves the host using a HostResolverProc. scoped_refptr<ProcTask> proc_task_; + // Resolves the host using a DnsTransaction. + scoped_ptr<DnsTask> dns_task_; + // All Requests waiting for the result of this Job. Some can be canceled. RequestsList requests_; - // A handle used by HostResolverImpl in |dispatcher_|. + // A handle used by |HostResolverImpl::dispatcher_|. PrioritizedDispatcher::Handle handle_; }; @@ -1222,12 +1399,14 @@ HostResolverImpl::HostResolverImpl( HostCache* cache, const PrioritizedDispatcher::Limits& job_limits, const ProcTaskParams& proc_params, + scoped_ptr<DnsConfigService> dns_config_service, NetLog* net_log) : cache_(cache), dispatcher_(job_limits), max_queued_jobs_(job_limits.total_jobs * 100u), proc_params_(proc_params), default_address_family_(ADDRESS_FAMILY_UNSPECIFIED), + dns_config_service_(dns_config_service.Pass()), ipv6_probe_monitoring_(false), additional_resolver_flags_(0), net_log_(net_log) { @@ -1254,6 +1433,9 @@ HostResolverImpl::HostResolverImpl( #endif NetworkChangeNotifier::AddDNSObserver(this); #endif + + if (dns_config_service_.get()) + dns_config_service_->AddObserver(this); } HostResolverImpl::~HostResolverImpl() { @@ -1295,8 +1477,7 @@ int HostResolverImpl::Resolve(const RequestInfo& info, int rv = ResolveHelper(key, info, addresses, request_net_log); if (rv != ERR_DNS_CACHE_MISS) { - LogFinishRequest(source_net_log, request_net_log, info, rv, - 0 /* os_error (unknown since from cache) */); + LogFinishRequest(source_net_log, request_net_log, info, rv); return rv; } @@ -1307,8 +1488,7 @@ int HostResolverImpl::Resolve(const RequestInfo& info, Job* job; if (jobit == jobs_.end()) { // Create new Job. - job = new Job(this, key, request_net_log); - job->set_handle(dispatcher_.Add(job, info.priority())); + job = new Job(this, key, request_net_log, info.priority()); // Check for queue overflow. if (dispatcher_.num_queued_jobs() > max_queued_jobs_) { @@ -1317,27 +1497,26 @@ int HostResolverImpl::Resolve(const RequestInfo& info, if (evicted == job) { delete job; rv = ERR_HOST_RESOLVER_QUEUE_TOO_LARGE; - LogFinishRequest(source_net_log, request_net_log, info, rv, 0); + LogFinishRequest(source_net_log, request_net_log, info, rv); return rv; } evicted->OnEvicted(); // Deletes |evicted|. } - jobs_.insert(jobit, std::make_pair(key, job)); } else { job = jobit->second; } // Can't complete synchronously. Create and attach request. - Request* req = new Request(source_net_log, request_net_log, info, callback, - addresses); - job->AddRequest(req); - if (!job->handle().is_null()) - job->set_handle(dispatcher_.ChangePriority(job->handle(), job->priority())); + scoped_ptr<Request> req(new Request(source_net_log, + request_net_log, + info, + callback, + addresses)); if (out_req) - *out_req = reinterpret_cast<RequestHandle>(req); + *out_req = reinterpret_cast<RequestHandle>(req.get()); - DCHECK_EQ(ERR_IO_PENDING, job->net_error()); + job->AddRequest(req.Pass()); // Completion happens during Job::CompleteRequests(). return ERR_IO_PENDING; } @@ -1376,8 +1555,7 @@ int HostResolverImpl::ResolveFromCache(const RequestInfo& info, Key key = GetEffectiveKeyForRequest(info); int rv = ResolveHelper(key, info, addresses, request_net_log); - LogFinishRequest(source_net_log, request_net_log, info, rv, - 0 /* os_error (unknown since from cache) */); + LogFinishRequest(source_net_log, request_net_log, info, rv); return rv; } @@ -1391,23 +1569,14 @@ void HostResolverImpl::CancelRequest(RequestHandle req_handle) { job->CancelRequest(req); - if (!job->handle().is_null()) { - // Still in queue. - if (job->num_active_requests()) { - job->set_handle(dispatcher_.ChangePriority(job->handle(), - job->priority())); - } else { - dispatcher_.Cancel(job->handle()); - RemoveJob(job); - delete job; - } - } else { - // Job is running (and could be in CompleteRequests right now). - // But to be in Request::OnComplete we would have to have a non-canceled - // request. So it is safe to Abort it if it has no more active requests. - if (!job->num_active_requests()) { + if (job->num_active_requests() == 0) { + // If we were called from a Requests' callback within Job::CompleteRequests, + // that Request could not have been cancelled, so job->num_active_requests() + // could not be 0. Therefore, we are not in Job::CompleteRequests(). + RemoveJob(job); + if (job->is_running()) job->Abort(); - } + delete job; } } @@ -1482,23 +1651,21 @@ bool HostResolverImpl::ServeFromCache(const Key& key, return true; } -void HostResolverImpl::OnJobFinished(Job* job, const AddressList& addrlist) { +void HostResolverImpl::OnJobFinished(Job* job, + int net_error, + const AddressList& addr_list, + base::TimeDelta ttl) { DCHECK(job); - DCHECK(job->handle().is_null()); - RemoveJob(job); - if (job->net_error() == ERR_HOST_RESOLVER_QUEUE_TOO_LARGE) - return; + DCHECK(!job->is_queued()); + DCHECK_NE(ERR_HOST_RESOLVER_QUEUE_TOO_LARGE, net_error); + DCHECK_NE(ERR_ABORTED, net_error); + RemoveJob(job); // Signal dispatcher that a slot has opened. dispatcher_.OnJobFinished(); - if (job->net_error() == ERR_ABORTED) - return; // Write result to the cache. if (cache_.get()) { - base::TimeDelta ttl = base::TimeDelta::FromSeconds(0); - if (job->net_error() == OK) - ttl = base::TimeDelta::FromSeconds(kCacheEntryTTLSeconds); - cache_->Set(job->key(), job->net_error(), addrlist, + cache_->Set(job->key(), net_error, addr_list, base::TimeTicks::Now(), ttl); } } @@ -1545,21 +1712,26 @@ HostResolverImpl::Key HostResolverImpl::GetEffectiveKeyForRequest( void HostResolverImpl::AbortAllInProgressJobs() { base::WeakPtr<HostResolverImpl> self = AsWeakPtr(); - // Scan |jobs_| for running jobs and abort them. + // In Abort, a Request callback could spawn new Jobs with matching keys, so + // first collect and remove all running jobs from |jobs_|. + std::vector<Job*> jobs_to_abort; for (JobMap::iterator it = jobs_.begin(); it != jobs_.end(); ) { Job* job = it->second; - // Advance the iterator before we might erase it. - ++it; if (job->is_running()) { - job->Abort(); - // Check if resolver was deleted in a request callback. - if (!self) - return; + jobs_to_abort.push_back(job); + jobs_.erase(it++); } else { - // Keep it in |dispatch_|. - DCHECK(!job->handle().is_null()); + DCHECK(job->is_queued()); + ++it; } } + + // Then Abort them and dispatch new Jobs. + for (size_t i = 0; i < jobs_to_abort.size(); ++i) { + jobs_to_abort[i]->Abort(); + dispatcher_.OnJobFinished(); + } + STLDeleteElements(&jobs_to_abort); } void HostResolverImpl::OnIPAddressChanged() { @@ -1594,4 +1766,23 @@ void HostResolverImpl::OnDNSChanged() { // |this| may be deleted inside AbortAllInProgressJobs(). } +void HostResolverImpl::OnConfigChanged(const DnsConfig& dns_config) { + // We want a new factory in place, before we Abort running Jobs, so that the + // newly started jobs use the new factory. + bool had_factory = (dns_transaction_factory_.get() != NULL); + if (dns_config.IsValid()) { + dns_transaction_factory_ = DnsTransactionFactory::CreateFactory( + new DnsSession(dns_config, + ClientSocketFactory::GetDefaultFactory(), + base::Bind(&base::RandInt), + net_log_)); + } else { + dns_transaction_factory_.reset(); + } + // Don't Abort running Jobs unless they were running on DnsTransaction. + // TODO(szym): This will change once http://crbug.com/114827 is fixed. + if (had_factory) + OnDNSChanged(); +} + } // namespace net diff --git a/net/base/host_resolver_impl.h b/net/base/host_resolver_impl.h index d5ff027..155cc83 100644 --- a/net/base/host_resolver_impl.h +++ b/net/base/host_resolver_impl.h @@ -23,11 +23,14 @@ #include "net/base/net_log.h" #include "net/base/network_change_notifier.h" #include "net/base/prioritized_dispatcher.h" +#include "net/dns/dns_config_service.h" namespace net { +class DnsTransactionFactory; + // For each hostname that is requested, HostResolver creates a -// HostResolverImpl::Job. When this job gets dispatched it creates a ProcJob +// HostResolverImpl::Job. When this job gets dispatched it creates a ProcTask // which runs the given HostResolverProc on a WorkerPool thread. If requests for // that same host are made during the job's lifetime, they are attached to the // existing job rather than creating a new one. This avoids doing parallel @@ -55,11 +58,13 @@ namespace net { // // Jobs are ordered in the queue based on their priority and order of arrival. // +// TODO(szym): Change DnsConfigService::Observer to Callback. class NET_EXPORT HostResolverImpl : public HostResolver, NON_EXPORTED_BASE(public base::NonThreadSafe), public NetworkChangeNotifier::IPAddressObserver, public NetworkChangeNotifier::DNSObserver, + NON_EXPORTED_BASE(public DnsConfigService::Observer), public base::SupportsWeakPtr<HostResolverImpl> { public: // Parameters for ProcTask which resolves hostnames using HostResolveProc. @@ -105,13 +110,18 @@ class NET_EXPORT HostResolverImpl // ownership of the |cache| pointer, and will free it during destruction. // // |job_limits| specifies the maximum number of jobs that the resolver will - // run at once (not counting potential duplicate attempts). + // run at once. This upper-bounds the total number of outstanding + // DNS transactions (not counting retransmissions and retries). + // + // |dns_config_service| will be used to obtain DnsConfig for + // DnsTransactionFactory. // // |net_log| must remain valid for the life of the HostResolverImpl. // TODO(szym): change to scoped_ptr<HostCache>. HostResolverImpl(HostCache* cache, const PrioritizedDispatcher::Limits& job_limits, const ProcTaskParams& proc_params, + scoped_ptr<DnsConfigService> dns_config_service, NetLog* net_log); // If any completion callbacks are pending when the resolver is destroyed, @@ -142,6 +152,7 @@ class NET_EXPORT HostResolverImpl class Job; class ProcTask; class IPv6ProbeJob; + class DnsTask; class Request; typedef HostCache::Key Key; typedef std::map<Key, Job*> JobMap; @@ -183,9 +194,12 @@ class NET_EXPORT HostResolverImpl // family when the request leaves it unspecified. Key GetEffectiveKeyForRequest(const RequestInfo& info) const; - // Called by |job| when it has finished running. Records the result in cache + // Called by |job| when it has completed. Records the result in cache // if necessary and dispatches another job if possible. - void OnJobFinished(Job* job, const AddressList& addrlist); + void OnJobFinished(Job* job, + int net_error, + const AddressList& addr_list, + base::TimeDelta ttl); // Removes |job| from |jobs_|. void RemoveJob(Job* job); @@ -194,12 +208,15 @@ class NET_EXPORT HostResolverImpl // Might start new jobs. void AbortAllInProgressJobs(); - // NetworkChangeNotifier::IPAddressObserver methods: + // NetworkChangeNotifier::IPAddressObserver: virtual void OnIPAddressChanged() OVERRIDE; - // NetworkChangeNotifier::OnDNSChanged methods: + // NetworkChangeNotifier::DNSObserver: virtual void OnDNSChanged() OVERRIDE; + // DnsConfigService::Observer: + virtual void OnConfigChanged(const DnsConfig& dns_config) OVERRIDE; + // Cache of host resolution results. scoped_ptr<HostCache> cache_; @@ -215,9 +232,13 @@ class NET_EXPORT HostResolverImpl // Parameters for ProcTask. ProcTaskParams proc_params_; + scoped_ptr<DnsTransactionFactory> dns_transaction_factory_; + // Address family to use when the request doesn't specify one. AddressFamily default_address_family_; + scoped_ptr<DnsConfigService> dns_config_service_; + // Indicate if probing is done after each network change event to set address // family. // When false, explicit setting of address family is used. diff --git a/net/base/host_resolver_impl_unittest.cc b/net/base/host_resolver_impl_unittest.cc index 9a299c1..88211ea 100644 --- a/net/base/host_resolver_impl_unittest.cc +++ b/net/base/host_resolver_impl_unittest.cc @@ -51,6 +51,7 @@ HostResolverImpl* CreateHostResolverImpl(HostResolverProc* resolver_proc) { HostCache::CreateDefaultCache(), DefaultLimits(), DefaultParams(resolver_proc), + scoped_ptr<DnsConfigService>(NULL), NULL); } @@ -65,6 +66,7 @@ HostResolverImpl* CreateSerialHostResolverImpl( return new HostResolverImpl(HostCache::CreateDefaultCache(), limits, params, + scoped_ptr<DnsConfigService>(NULL), NULL); } @@ -114,7 +116,7 @@ class CapturingHostResolverProc : public HostResolverProc { AddressFamily address_family, HostResolverFlags host_resolver_flags, AddressList* addrlist, - int* os_error) { + int* os_error) OVERRIDE { event_.Wait(); { base::AutoLock l(lock_); @@ -161,7 +163,7 @@ class EchoingHostResolverProc : public HostResolverProc { AddressFamily address_family, HostResolverFlags host_resolver_flags, AddressList* addrlist, - int* os_error) { + int* os_error) OVERRIDE { // Encode the request's hostname and address_family in the output address. std::string ip_literal = base::StringPrintf("192.%d.%d.%d", static_cast<int>(hostname.size()), @@ -227,7 +229,7 @@ class LookupAttemptHostResolverProc : public HostResolverProc { AddressFamily address_family, HostResolverFlags host_resolver_flags, AddressList* addrlist, - int* os_error) { + int* os_error) OVERRIDE { bool wait_for_right_attempt_to_complete = true; { base::AutoLock auto_lock(lock_); @@ -434,8 +436,7 @@ TEST_F(HostResolverImplTest, FailedAsynchronousLookup) { new RuleBasedHostResolverProc(NULL)); resolver_proc->AddSimulatedFailure("just.testing"); - scoped_ptr<HostResolver> host_resolver( - CreateHostResolverImpl(resolver_proc)); + scoped_ptr<HostResolver> host_resolver(CreateHostResolverImpl(resolver_proc)); HostResolver::RequestInfo info(HostPortPair("just.testing", kPortnum)); CapturingBoundNetLog log(CapturingNetLog::kUnbounded); @@ -470,9 +471,16 @@ TEST_F(HostResolverImplTest, FailedAsynchronousLookup) { class WaitingHostResolverProc : public HostResolverProc { public: explicit WaitingHostResolverProc(HostResolverProc* previous) - : HostResolverProc(previous), - is_waiting_(false, false), - is_signaled_(false, false) {} + : HostResolverProc(previous), + is_waiting_(false, false), + is_signaled_(false, false) {} + + // If |manual_reset| is true, once Signalled, it will let all Resolve calls + // proceed. + WaitingHostResolverProc(HostResolverProc* previous, bool manual_reset) + : HostResolverProc(previous), + is_waiting_(false, false), + is_signaled_(manual_reset, false) {} // Waits until a call to |Resolve| is blocked. It is recommended to always // |Wait| before |Signal|, and required if issuing a series of two or more @@ -492,7 +500,7 @@ class WaitingHostResolverProc : public HostResolverProc { AddressFamily address_family, HostResolverFlags host_resolver_flags, AddressList* addrlist, - int* os_error) { + int* os_error) OVERRIDE { is_waiting_.Signal(); is_signaled_.Wait(); return ResolveUsingPrevious(host, address_family, host_resolver_flags, @@ -501,7 +509,6 @@ class WaitingHostResolverProc : public HostResolverProc { private: virtual ~WaitingHostResolverProc() {} - base::WaitableEvent is_waiting_; base::WaitableEvent is_signaled_; }; @@ -518,6 +525,7 @@ TEST_F(HostResolverImplTest, AbortedAsynchronousLookup) { new HostResolverImpl(HostCache::CreateDefaultCache(), DefaultLimits(), DefaultParams(resolver_proc), + scoped_ptr<DnsConfigService>(NULL), &net_log)); AddressList addrlist; const int kPortnum = 80; @@ -553,20 +561,17 @@ TEST_F(HostResolverImplTest, AbortedAsynchronousLookup) { pos = ExpectLogContainsSomewhereAfter(net_log_entries, pos + 1, NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK, NetLog::PHASE_BEGIN); - // Both Request and ProcTask need to be cancelled. (The Job is "aborted".) - pos = ExpectLogContainsSomewhereAfter(net_log_entries, pos + 1, - NetLog::TYPE_CANCELLED, - NetLog::PHASE_NONE); - // Don't care about order in which Request, Job and ProcTask end, or when the - // other one is cancelled. + + // The Request needs to be cancelled. (The Job is "aborted".) + // Don't care about order in which Request, Job and ProcTask end. ExpectLogContainsSomewhereAfter(net_log_entries, pos + 1, NetLog::TYPE_CANCELLED, NetLog::PHASE_NONE); ExpectLogContainsSomewhereAfter(net_log_entries, pos + 1, - NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST, + NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK, NetLog::PHASE_END); ExpectLogContainsSomewhereAfter(net_log_entries, pos + 1, - NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK, + NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST, NetLog::PHASE_END); ExpectLogContainsSomewhereAfter(net_log_entries, pos + 1, NetLog::TYPE_HOST_RESOLVER_IMPL_JOB, @@ -679,7 +684,7 @@ class DeDupeRequestsVerifier : public ResolveRequest::Delegate { : count_a_(0), count_b_(0), resolver_proc_(resolver_proc) {} // The test does 5 resolves (which can complete in any order). - virtual void OnCompleted(ResolveRequest* resolve) { + virtual void OnCompleted(ResolveRequest* resolve) OVERRIDE { // Tally up how many requests we have seen. if (resolve->hostname() == "a") { count_a_++; @@ -752,7 +757,7 @@ class CancelMultipleRequestsVerifier : public ResolveRequest::Delegate { CancelMultipleRequestsVerifier() {} // The cancels kill all but one request. - virtual void OnCompleted(ResolveRequest* resolve) { + virtual void OnCompleted(ResolveRequest* resolve) OVERRIDE { EXPECT_EQ("a", resolve->hostname()); EXPECT_EQ(82, resolve->port()); @@ -810,7 +815,7 @@ class CancelWithinCallbackVerifier : public ResolveRequest::Delegate { : req_to_cancel1_(NULL), req_to_cancel2_(NULL), num_completions_(0) { } - virtual void OnCompleted(ResolveRequest* resolve) { + virtual void OnCompleted(ResolveRequest* resolve) OVERRIDE { num_completions_++; // Port 80 is the first request that the callback will be invoked for. @@ -893,7 +898,7 @@ class DeleteWithinCallbackVerifier : public ResolveRequest::Delegate { explicit DeleteWithinCallbackVerifier(HostResolver* host_resolver) : host_resolver_(host_resolver) {} - virtual void OnCompleted(ResolveRequest* resolve) { + virtual void OnCompleted(ResolveRequest* resolve) OVERRIDE { EXPECT_EQ("a", resolve->hostname()); EXPECT_EQ(80, resolve->port()); @@ -943,7 +948,7 @@ class StartWithinCallbackVerifier : public ResolveRequest::Delegate { public: StartWithinCallbackVerifier() : num_requests_(0) {} - virtual void OnCompleted(ResolveRequest* resolve) { + virtual void OnCompleted(ResolveRequest* resolve) OVERRIDE { EXPECT_EQ("a", resolve->hostname()); if (80 == resolve->port()) { @@ -973,7 +978,11 @@ TEST_F(HostResolverImplTest, StartWithinCallback) { // Turn off caching for this host resolver. scoped_ptr<HostResolver> host_resolver(new HostResolverImpl( - NULL, DefaultLimits(), DefaultParams(resolver_proc), NULL)); + NULL, + DefaultLimits(), + DefaultParams(resolver_proc), + scoped_ptr<DnsConfigService>(NULL), + NULL)); // The class will receive callbacks for when each resolve completes. It // checks that the right things happened. @@ -999,7 +1008,7 @@ class BypassCacheVerifier : public ResolveRequest::Delegate { public: BypassCacheVerifier() {} - virtual void OnCompleted(ResolveRequest* resolve) { + virtual void OnCompleted(ResolveRequest* resolve) OVERRIDE { EXPECT_EQ("a", resolve->hostname()); HostResolver* resolver = resolve->resolver(); @@ -1141,58 +1150,70 @@ TEST_F(HostResolverImplTest, ObeyPoolConstraintsAfterIPAddressChange) { EXPECT_EQ(OK, callback.WaitForResult()); } -class ResolveWithinCallback { +// Helper class used by AbortOnlyExistingRequestsOnIPAddressChange. +class StartWithinAbortedCallbackVerifier : public ResolveRequest::Delegate { public: - explicit ResolveWithinCallback(const HostResolver::RequestInfo& info) - : info_(info) {} - - const CompletionCallback& callback() const { return callback_.callback(); } - - int WaitForResult() { - int result = callback_.WaitForResult(); - // Ditch the WaitingHostResolverProc so that the subsequent request - // succeeds. - host_resolver_.reset( - CreateHostResolverImpl(CreateCatchAllHostResolverProc())); - EXPECT_EQ(ERR_IO_PENDING, - host_resolver_->Resolve(info_, &addrlist_, - nested_callback_.callback(), NULL, - BoundNetLog())); - return result; + explicit StartWithinAbortedCallbackVerifier(const std::string& next_hostname) + : next_hostname_(next_hostname) {} + + virtual void OnCompleted(ResolveRequest* resolve) OVERRIDE { + if (request_.get() == NULL) { + EXPECT_EQ(ERR_ABORTED, resolve->result()); + // Start new request for a different hostname to ensure that the order + // of jobs in HostResolverImpl is not stable. + request_.reset(new ResolveRequest(resolve->resolver(), + next_hostname_, + resolve->port(), + this)); + } else { + EXPECT_EQ(resolve, request_.get()); + callback_.callback().Run(resolve->result()); + } } - int WaitForNestedResult() { - return nested_callback_.WaitForResult(); + int WaitUntilDone() { + return callback_.WaitForResult(); } private: - const HostResolver::RequestInfo info_; - AddressList addrlist_; + std::string next_hostname_; + scoped_ptr<ResolveRequest> request_; TestCompletionCallback callback_; - TestCompletionCallback nested_callback_; - scoped_ptr<HostResolver> host_resolver_; + DISALLOW_COPY_AND_ASSIGN(StartWithinAbortedCallbackVerifier); }; -TEST_F(HostResolverImplTest, OnlyAbortExistingRequestsOnIPAddressChange) { +// Tests that a new Request made from the callback of a previously aborted one +// will not be aborted. +TEST_F(HostResolverImplTest, AbortOnlyExistingRequestsOnIPAddressChange) { + // Setting |manual_reset| to true so that Signal unblocks all calls. scoped_refptr<WaitingHostResolverProc> resolver_proc( - new WaitingHostResolverProc(CreateCatchAllHostResolverProc())); + new WaitingHostResolverProc(CreateCatchAllHostResolverProc(), true)); scoped_ptr<HostResolver> host_resolver(CreateHostResolverImpl(resolver_proc)); - // Resolve "host1". - HostResolver::RequestInfo info(HostPortPair("host1", 70)); - ResolveWithinCallback callback(info); - AddressList addrlist; - int rv = host_resolver->Resolve(info, &addrlist, callback.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(ERR_IO_PENDING, rv); + StartWithinAbortedCallbackVerifier verifier1("zzz"); + StartWithinAbortedCallbackVerifier verifier2("aaa"); + StartWithinAbortedCallbackVerifier verifier3("eee"); + ResolveRequest req1(host_resolver.get(), "bbb", 40, &verifier1); + ResolveRequest req2(host_resolver.get(), "eee", 80, &verifier2); + ResolveRequest req3(host_resolver.get(), "ccc", 90, &verifier3); + // The jobs start immediately. + // Wait until at least one is blocked. resolver_proc->Wait(); - // Triggering an IP address change. + // Trigger an IP address change. NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests(); - MessageLoop::current()->RunAllPending(); // Notification happens async. - EXPECT_EQ(ERR_ABORTED, callback.WaitForResult()); - resolver_proc->Signal(); // release the thread from WorkerPool for cleanup - EXPECT_EQ(OK, callback.WaitForNestedResult()); + // This should abort all running jobs. + MessageLoop::current()->RunAllPending(); + EXPECT_EQ(ERR_ABORTED, req1.result()); + EXPECT_EQ(ERR_ABORTED, req2.result()); + EXPECT_EQ(ERR_ABORTED, req3.result()); + // Unblock all calls to proc. + resolver_proc->Signal(); + // Run until the re-started requests finish. + EXPECT_EQ(OK, verifier1.WaitUntilDone()); + EXPECT_EQ(OK, verifier2.WaitUntilDone()); + EXPECT_EQ(OK, verifier3.WaitUntilDone()); + MessageLoop::current()->AssertIdle(); } // Tests that when the maximum threads is set to 1, requests are dequeued @@ -1575,7 +1596,7 @@ TEST_F(HostResolverImplTest, MultipleAttempts) { new LookupAttemptHostResolverProc( NULL, kAttemptNumberToResolve, kTotalAttempts)); - HostResolverImpl::ProcTaskParams params = DefaultParams(resolver_proc); + HostResolverImpl::ProcTaskParams params = DefaultParams(resolver_proc.get()); // Specify smaller interval for unresponsive_delay_ for HostResolverImpl so // that unit test runs faster. For example, this test finishes in 1.5 secs @@ -1586,6 +1607,7 @@ TEST_F(HostResolverImplTest, MultipleAttempts) { new HostResolverImpl(HostCache::CreateDefaultCache(), DefaultLimits(), params, + scoped_ptr<DnsConfigService>(NULL), NULL)); // Resolve "host1". diff --git a/net/base/net_log_event_type_list.h b/net/base/net_log_event_type_list.h index 6d54842..b6b02d3 100644 --- a/net/base/net_log_event_type_list.h +++ b/net/base/net_log_event_type_list.h @@ -56,7 +56,6 @@ EVENT_TYPE(HOST_RESOLVER_IMPL) // If an error occurred, the END phase will contain these parameters: // { // "net_error": <The net error code integer for the failure>, -// "os_error": <The exact error code integer that getaddrinfo() returned>, // } EVENT_TYPE(HOST_RESOLVER_IMPL_REQUEST) @@ -84,7 +83,6 @@ EVENT_TYPE(HOST_RESOLVER_IMPL_CREATE_JOB) // If an error occurred, the END phase will contain these parameters: // { // "net_error": <The net error code integer for the failure>, -// "os_error": <The exact error code integer that getaddrinfo() returned>, // } EVENT_TYPE(HOST_RESOLVER_IMPL_JOB) @@ -154,26 +152,16 @@ EVENT_TYPE(HOST_RESOLVER_IMPL_JOB_REQUEST_ATTACH) // } EVENT_TYPE(HOST_RESOLVER_IMPL_JOB_REQUEST_DETACH) -// Logged for a HostResolverImpl::Job when it creates a ProcTask. -// -// The event contains the following parameters: -// -// { -// "source_dependency": <Source id of parent HostResolverImpl::Job>, -// } -EVENT_TYPE(HOST_RESOLVER_IMPL_CREATE_PROC_TASK) - // The creation/completion of a HostResolverImpl::ProcTask to call getaddrinfo. // The BEGIN phase contains the following parameters: // // { // "hostname": <Hostname associated with the request>, -// "source_dependency": <Source id of parent HostResolverImpl::Job>, // } // // On success, the END phase has these parameters: // { -// "address_list": <The host name being resolved>, +// "address_list": <The resolved addresses>, // } // If an error occurred, the END phase will contain these parameters: // { @@ -182,6 +170,24 @@ EVENT_TYPE(HOST_RESOLVER_IMPL_CREATE_PROC_TASK) // } EVENT_TYPE(HOST_RESOLVER_IMPL_PROC_TASK) +// The creation/completion of a HostResolverImpl::DnsTask to manage a +// DnsTransaction. The BEGIN phase contains the following parameters: +// +// { +// "source_dependency": <Source id of DnsTransaction>, +// } +// +// On success, the END phase has these parameters: +// { +// "address_list": <The resolved addresses>, +// } +// If an error occurred, the END phase will contain these parameters: +// { +// "net_error": <The net error code integer for the failure>, +// "dns_error": <The detailed DnsResponse::Result> +// } +EVENT_TYPE(HOST_RESOLVER_IMPL_DNS_TASK) + // ------------------------------------------------------------------------ // InitProxyResolver // ------------------------------------------------------------------------ @@ -1256,8 +1262,6 @@ EVENT_TYPE(THROTTLING_GOT_CUSTOM_RETRY_AFTER) // { // "hostname": <The hostname it is trying to resolve>, // "query_type": <Type of the query>, -// "source_dependency": <Source id, if any, of what created the -// transaction>, // } // // The END phase contains the following parameters: @@ -1288,7 +1292,8 @@ EVENT_TYPE(DNS_TRANSACTION_QUERY) // It has a single parameter: // // { -// "socket_source": <Source id of the UDP socket created for the attempt>, +// "source_dependency": <Source id of the UDP socket created for the +// attempt>, // } EVENT_TYPE(DNS_TRANSACTION_ATTEMPT) @@ -1299,51 +1304,12 @@ EVENT_TYPE(DNS_TRANSACTION_ATTEMPT) // { // "rcode": <rcode in the received response>, // "answer_count": <answer_count in the received response>, -// "socket_source": <Source id of the UDP socket that received the -// response>, +// "source_dependency": <Source id of the UDP socket that received the +// response>, // } EVENT_TYPE(DNS_TRANSACTION_RESPONSE) // ------------------------------------------------------------------------ -// AsyncHostResolver -// ------------------------------------------------------------------------ - -// The start/end of waiting on a host resolve (DNS) request. -// The BEGIN phase contains the following parameters: -// -// { -// "source_dependency": <Source id of the request being waited on>, -// } -EVENT_TYPE(ASYNC_HOST_RESOLVER) - -// The start/end of a host resolve (DNS) request. -// -// The BEGIN phase contains the following parameters: -// -// { -// "hostname": <Hostname associated with the request>, -// "address_family": <Address family of the request>, -// "allow_cached_response": <Whether to allow cached response>, -// "only_use_cached_response": <Use cached results only>, -// "is_speculative": <Whether the lookup is speculative>, -// "priority": <Priority of the request>, -// "source_dependency": <Source id, if any, of what created the request>, -// } -// -// If an error occurred, the END phase will contain this parameter: -// { -// "net_error": <The net error code integer for the failure>, -// } -EVENT_TYPE(ASYNC_HOST_RESOLVER_REQUEST) - -// This event is created when a new DnsTransaction is about to be created -// for a request. -EVENT_TYPE(ASYNC_HOST_RESOLVER_CREATE_DNS_TRANSACTION) - -// This event is logged when a request is handled by a cache entry. -EVENT_TYPE(ASYNC_HOST_RESOLVER_CACHE_HIT) - -// ------------------------------------------------------------------------ // ChromeExtension // ------------------------------------------------------------------------ diff --git a/net/base/net_log_source_type_list.h b/net/base/net_log_source_type_list.h index 233f3a2..ea550bc 100644 --- a/net/base/net_log_source_type_list.h +++ b/net/base/net_log_source_type_list.h @@ -16,18 +16,15 @@ SOURCE_TYPE(SOCKET, 5) SOURCE_TYPE(SPDY_SESSION, 6) SOURCE_TYPE(HOST_RESOLVER_IMPL_REQUEST, 7) SOURCE_TYPE(HOST_RESOLVER_IMPL_JOB, 8) -SOURCE_TYPE(HOST_RESOLVER_IMPL_PROC_TASK, 9) -SOURCE_TYPE(DISK_CACHE_ENTRY, 10) -SOURCE_TYPE(MEMORY_CACHE_ENTRY, 11) -SOURCE_TYPE(HTTP_STREAM_JOB, 12) -SOURCE_TYPE(EXPONENTIAL_BACKOFF_THROTTLING, 13) -SOURCE_TYPE(DNS_TRANSACTION, 14) -SOURCE_TYPE(ASYNC_HOST_RESOLVER_REQUEST, 15) -SOURCE_TYPE(UDP_SOCKET, 16) -SOURCE_TYPE(CERT_VERIFIER_JOB, 17) -SOURCE_TYPE(HTTP_PIPELINED_CONNECTION, 18) -SOURCE_TYPE(DOWNLOAD, 19) -SOURCE_TYPE(FILESTREAM, 20) +SOURCE_TYPE(DISK_CACHE_ENTRY, 9) +SOURCE_TYPE(MEMORY_CACHE_ENTRY, 10) +SOURCE_TYPE(HTTP_STREAM_JOB, 11) +SOURCE_TYPE(EXPONENTIAL_BACKOFF_THROTTLING, 12) +SOURCE_TYPE(UDP_SOCKET, 13) +SOURCE_TYPE(CERT_VERIFIER_JOB, 14) +SOURCE_TYPE(HTTP_PIPELINED_CONNECTION, 15) +SOURCE_TYPE(DOWNLOAD, 16) +SOURCE_TYPE(FILESTREAM, 17) -SOURCE_TYPE(COUNT, 21) // Always keep this as the last entry. +SOURCE_TYPE(COUNT, 18) // Always keep this as the last entry. diff --git a/net/base/priority_queue.h b/net/base/priority_queue.h index 037faed..f60b88c 100644 --- a/net/base/priority_queue.h +++ b/net/base/priority_queue.h @@ -47,7 +47,11 @@ class PriorityQueue : public base::NonThreadSafe { class Pointer { public: // Constructs a null pointer. - Pointer() : priority_(kNullPriority) {} + Pointer() : priority_(kNullPriority) { +#if !defined(NDEBUG) + id_ = static_cast<size_t>(-1); +#endif + } bool is_null() const { return priority_ == kNullPriority; } @@ -64,6 +68,10 @@ class PriorityQueue : public base::NonThreadSafe { return (priority_ == other.priority_) && (iterator_ == other.iterator_); } + void Reset() { + *this = Pointer(); + } + private: friend class PriorityQueue; diff --git a/net/dns/async_host_resolver.cc b/net/dns/async_host_resolver.cc deleted file mode 100644 index 72e5472..0000000 --- a/net/dns/async_host_resolver.cc +++ /dev/null @@ -1,524 +0,0 @@ -// 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/async_host_resolver.h" - -#include <algorithm> - -#include "base/bind.h" -#include "base/logging.h" -#include "base/message_loop.h" -#include "base/rand_util.h" -#include "base/stl_util.h" -#include "base/values.h" -#include "net/base/address_list.h" -#include "net/base/dns_util.h" -#include "net/base/net_errors.h" -#include "net/dns/dns_protocol.h" -#include "net/dns/dns_response.h" -#include "net/dns/dns_session.h" -#include "net/socket/client_socket_factory.h" - -namespace net { - -namespace { - -// TODO(agayev): fix this when IPv6 support is added. -uint16 QueryTypeFromAddressFamily(AddressFamily address_family) { - return dns_protocol::kTypeA; -} - -class RequestParameters : public NetLog::EventParameters { - public: - RequestParameters(const HostResolver::RequestInfo& info, - const NetLog::Source& source) - : info_(info), source_(source) {} - - virtual Value* ToValue() const { - DictionaryValue* dict = new DictionaryValue(); - dict->SetString("hostname", info_.host_port_pair().ToString()); - dict->SetInteger("address_family", - static_cast<int>(info_.address_family())); - dict->SetBoolean("allow_cached_response", info_.allow_cached_response()); - dict->SetBoolean("is_speculative", info_.is_speculative()); - dict->SetInteger("priority", info_.priority()); - - if (source_.is_valid()) - dict->Set("source_dependency", source_.ToValue()); - - return dict; - } - - private: - const HostResolver::RequestInfo info_; - const NetLog::Source source_; -}; - -} // namespace - -HostResolver* CreateAsyncHostResolver(size_t max_concurrent_resolves, - const IPAddressNumber& dns_ip, - NetLog* net_log) { - size_t max_dns_requests = max_concurrent_resolves; - if (max_dns_requests == 0) - max_dns_requests = 20; - size_t max_pending_requests = max_dns_requests * 100; - DnsConfig config; - config.nameservers.push_back(IPEndPoint(dns_ip, 53)); - DnsSession* session = new DnsSession( - config, - ClientSocketFactory::GetDefaultFactory(), - base::Bind(&base::RandInt), - net_log); - HostResolver* resolver = new AsyncHostResolver( - max_dns_requests, - max_pending_requests, - HostCache::CreateDefaultCache(), - DnsTransactionFactory::CreateFactory(session), - net_log); - return resolver; -} - -//----------------------------------------------------------------------------- -// Every call to Resolve() results in Request object being created. Such a -// call may complete either synchronously or asynchronously or it may get -// cancelled, which can be either through specific CancelRequest call or by -// the destruction of AsyncHostResolver, which would destruct pending or -// in-progress requests, causing them to be cancelled. Synchronous -// resolution sets |callback_| to NULL, thus, if in the destructor we still -// have a non-NULL |callback_|, we are being cancelled. -class AsyncHostResolver::Request { - public: - Request(AsyncHostResolver* resolver, - const BoundNetLog& source_net_log, - const BoundNetLog& request_net_log, - const HostResolver::RequestInfo& info, - const CompletionCallback& callback, - AddressList* addresses) - : resolver_(resolver), - source_net_log_(source_net_log), - request_net_log_(request_net_log), - info_(info), - callback_(callback), - addresses_(addresses), - result_(ERR_UNEXPECTED) { - DCHECK(addresses_); - DCHECK(resolver_); - resolver_->OnStart(this); - key_ = Key(info.hostname(), - QueryTypeFromAddressFamily(info.address_family())); - } - - ~Request() { - if (!callback_.is_null()) - resolver_->OnCancel(this); - } - - int result() const { return result_; } - const Key& key() const { - DCHECK(IsValid()); - return key_; - } - const HostResolver::RequestInfo& info() const { return info_; } - RequestPriority priority() const { return info_.priority(); } - const BoundNetLog& source_net_log() const { return source_net_log_; } - const BoundNetLog& request_net_log() const { return request_net_log_; } - - bool ResolveAsIp() { - IPAddressNumber ip_number; - if (!ParseIPLiteralToNumber(info_.hostname(), &ip_number)) - return false; - - if (ip_number.size() != kIPv4AddressSize) { - result_ = ERR_NAME_NOT_RESOLVED; - } else { - *addresses_ = AddressList::CreateFromIPAddressWithCname( - ip_number, - info_.port(), - info_.host_resolver_flags() & HOST_RESOLVER_CANONNAME); - result_ = OK; - } - return true; - } - - bool ServeFromCache() { - HostCache* cache = resolver_->cache_.get(); - if (!cache || !info_.allow_cached_response()) - return false; - - HostCache::Key key(info_.hostname(), info_.address_family(), - info_.host_resolver_flags()); - const HostCache::Entry* cache_entry = cache->Lookup( - key, base::TimeTicks::Now()); - if (cache_entry) { - request_net_log_.AddEvent( - NetLog::TYPE_ASYNC_HOST_RESOLVER_CACHE_HIT, NULL); - DCHECK_EQ(OK, cache_entry->error); - result_ = cache_entry->error; - *addresses_ = - CreateAddressListUsingPort(cache_entry->addrlist, info_.port()); - return true; - } - return false; - } - - // Called when a request completes synchronously; we do not have an - // AddressList argument, since in case of a successful synchronous - // completion, either ResolveAsIp or ServerFromCache would set the - // |addresses_| and in case of an unsuccessful synchronous completion, we - // do not touch |addresses_|. - void OnSyncComplete(int result) { - callback_.Reset(); - resolver_->OnFinish(this, result); - } - - // Called when a request completes asynchronously. - void OnAsyncComplete(int result, const AddressList& addresses) { - if (result == OK) - *addresses_ = CreateAddressListUsingPort(addresses, info_.port()); - DCHECK_EQ(false, callback_.is_null()); - CompletionCallback callback = callback_; - callback_.Reset(); - resolver_->OnFinish(this, result); - callback.Run(result); - } - - // Returns true if request has a validly formed hostname. - bool IsValid() const { - return !info_.hostname().empty() && !key_.first.empty(); - } - - private: - AsyncHostResolver* resolver_; - BoundNetLog source_net_log_; - BoundNetLog request_net_log_; - const HostResolver::RequestInfo info_; - Key key_; - CompletionCallback callback_; - AddressList* addresses_; - int result_; -}; - -//----------------------------------------------------------------------------- -AsyncHostResolver::AsyncHostResolver(size_t max_dns_requests, - size_t max_pending_requests, - HostCache* cache, - scoped_ptr<DnsTransactionFactory> client, - NetLog* net_log) - : max_dns_transactions_(max_dns_requests), - max_pending_requests_(max_pending_requests), - cache_(cache), - client_(client.Pass()), - net_log_(net_log) { -} - -AsyncHostResolver::~AsyncHostResolver() { - // Destroy request lists. - for (KeyRequestListMap::iterator it = requestlist_map_.begin(); - it != requestlist_map_.end(); ++it) - STLDeleteElements(&it->second); - - // Destroy DNS transactions. - STLDeleteElements(&dns_transactions_); - - // Destroy pending requests. - for (size_t i = 0; i < arraysize(pending_requests_); ++i) - STLDeleteElements(&pending_requests_[i]); -} - -int AsyncHostResolver::Resolve(const RequestInfo& info, - AddressList* addresses, - const CompletionCallback& callback, - RequestHandle* out_req, - const BoundNetLog& source_net_log) { - DCHECK(addresses); - DCHECK_EQ(false, callback.is_null()); - scoped_ptr<Request> request( - CreateNewRequest(info, callback, addresses, source_net_log)); - - int rv = ERR_UNEXPECTED; - if (!request->IsValid()) - rv = ERR_NAME_NOT_RESOLVED; - else if (request->ResolveAsIp() || request->ServeFromCache()) - rv = request->result(); - else if (AttachToRequestList(request.get())) - rv = ERR_IO_PENDING; - else if (dns_transactions_.size() < max_dns_transactions_) - rv = StartNewDnsRequestFor(request.get()); - else - rv = Enqueue(request.get()); - - if (rv != ERR_IO_PENDING) { - request->OnSyncComplete(rv); - } else { - Request* req = request.release(); - if (out_req) - *out_req = reinterpret_cast<RequestHandle>(req); - } - return rv; -} - -int AsyncHostResolver::ResolveFromCache(const RequestInfo& info, - AddressList* addresses, - const BoundNetLog& source_net_log) { - scoped_ptr<Request> request( - CreateNewRequest(info, CompletionCallback(), addresses, source_net_log)); - int rv = ERR_UNEXPECTED; - if (!request->IsValid()) - rv = ERR_NAME_NOT_RESOLVED; - else if (request->ResolveAsIp() || request->ServeFromCache()) - rv = request->result(); - else - rv = ERR_DNS_CACHE_MISS; - request->OnSyncComplete(rv); - return rv; -} - -void AsyncHostResolver::OnStart(Request* request) { - DCHECK(request); - - request->source_net_log().BeginEvent( - NetLog::TYPE_ASYNC_HOST_RESOLVER, - make_scoped_refptr(new NetLogSourceParameter( - "source_dependency", request->request_net_log().source()))); - request->request_net_log().BeginEvent( - NetLog::TYPE_ASYNC_HOST_RESOLVER_REQUEST, - make_scoped_refptr(new RequestParameters( - request->info(), request->source_net_log().source()))); -} - -void AsyncHostResolver::OnFinish(Request* request, int result) { - DCHECK(request); - request->request_net_log().EndEventWithNetErrorCode( - NetLog::TYPE_ASYNC_HOST_RESOLVER_REQUEST, result); - request->source_net_log().EndEvent( - NetLog::TYPE_ASYNC_HOST_RESOLVER, NULL); -} - -void AsyncHostResolver::OnCancel(Request* request) { - DCHECK(request); - - request->request_net_log().AddEvent( - NetLog::TYPE_CANCELLED, NULL); - request->request_net_log().EndEvent( - NetLog::TYPE_ASYNC_HOST_RESOLVER_REQUEST, NULL); - request->source_net_log().EndEvent( - NetLog::TYPE_ASYNC_HOST_RESOLVER, NULL); -} - -void AsyncHostResolver::CancelRequest(RequestHandle req_handle) { - scoped_ptr<Request> request(reinterpret_cast<Request*>(req_handle)); - DCHECK(request.get()); - - KeyRequestListMap::iterator it = requestlist_map_.find(request->key()); - if (it != requestlist_map_.end()) - it->second.remove(request.get()); - else - pending_requests_[request->priority()].remove(request.get()); -} - -void AsyncHostResolver::SetDefaultAddressFamily( - AddressFamily address_family) { - NOTIMPLEMENTED(); -} - -AddressFamily AsyncHostResolver::GetDefaultAddressFamily() const { - return ADDRESS_FAMILY_IPV4; -} - -HostCache* AsyncHostResolver::GetHostCache() { - return cache_.get(); -} - -void AsyncHostResolver::OnDnsTransactionComplete( - DnsTransaction* transaction, - int result, - const DnsResponse* response) { - DCHECK(std::find(dns_transactions_.begin(), - dns_transactions_.end(), - transaction) != dns_transactions_.end()); - - // If by the time requests that caused |transaction| are cancelled, we do - // not have a port number to associate with the result, therefore, we - // assume the most common port, otherwise we use the port number of the - // first request. - KeyRequestListMap::iterator rit = requestlist_map_.find( - std::make_pair(transaction->GetHostname(), transaction->GetType())); - DCHECK(rit != requestlist_map_.end()); - RequestList& requests = rit->second; - int port = requests.empty() ? 80 : requests.front()->info().port(); - - // Extract AddressList and TTL out of DnsResponse. - AddressList addr_list; - uint32 ttl = kuint32max; - if (result == OK) { - IPAddressList ip_addresses; - DnsRecordParser parser = response->Parser(); - DnsResourceRecord record; - // TODO(szym): Add stricter checking of names, aliases and address lengths. - while (parser.ParseRecord(&record)) { - if (record.type == transaction->GetType() && - (record.rdata.size() == kIPv4AddressSize || - record.rdata.size() == kIPv6AddressSize)) { - ip_addresses.push_back(IPAddressNumber(record.rdata.begin(), - record.rdata.end())); - ttl = std::min(ttl, record.ttl); - } - } - if (!ip_addresses.empty()) - addr_list = AddressList::CreateFromIPAddressList(ip_addresses, port); - else - result = ERR_NAME_NOT_RESOLVED; - } - - // Run callback of every request that was depending on this DNS request, - // also notify observers. - for (RequestList::iterator it = requests.begin(); it != requests.end(); ++it) - (*it)->OnAsyncComplete(result, addr_list); - - // It is possible that the requests that caused |transaction| to be - // created are cancelled by the time |transaction| completes. In that - // case |requests| would be empty. We are knowingly throwing away the - // result of a DNS resolution in that case, because (a) if there are no - // requests, we do not have info to obtain a key from, (b) DnsTransaction - // does not have info(). - // TODO(szym): Should DnsTransaction ignore HostResolverFlags or use defaults? - if ((result == OK || result == ERR_NAME_NOT_RESOLVED) && cache_.get() && - !requests.empty()) { - Request* request = requests.front(); - HostResolver::RequestInfo info = request->info(); - HostCache::Key key( - info.hostname(), info.address_family(), info.host_resolver_flags()); - // Store negative results with TTL 0 to flush out the old entry. - cache_->Set(key, - result, - addr_list, - base::TimeTicks::Now(), - (result == OK) ? base::TimeDelta::FromSeconds(ttl) - : base::TimeDelta()); - } - - // Cleanup requests. - STLDeleteElements(&requests); - requestlist_map_.erase(rit); - - // Cleanup |transaction| and start a new one if there are pending requests. - dns_transactions_.remove(transaction); - delete transaction; - ProcessPending(); -} - -AsyncHostResolver::Request* AsyncHostResolver::CreateNewRequest( - const RequestInfo& info, - const CompletionCallback& callback, - AddressList* addresses, - const BoundNetLog& source_net_log) { - BoundNetLog request_net_log = BoundNetLog::Make(net_log_, - NetLog::SOURCE_ASYNC_HOST_RESOLVER_REQUEST); - return new Request( - this, source_net_log, request_net_log, info, callback, addresses); -} - -bool AsyncHostResolver::AttachToRequestList(Request* request) { - KeyRequestListMap::iterator it = requestlist_map_.find(request->key()); - if (it == requestlist_map_.end()) - return false; - it->second.push_back(request); - return true; -} - -int AsyncHostResolver::StartNewDnsRequestFor(Request* request) { - DCHECK(requestlist_map_.find(request->key()) == requestlist_map_.end()); - DCHECK(dns_transactions_.size() < max_dns_transactions_); - - request->request_net_log().AddEvent( - NetLog::TYPE_ASYNC_HOST_RESOLVER_CREATE_DNS_TRANSACTION, NULL); - - requestlist_map_[request->key()].push_back(request); - scoped_ptr<DnsTransaction> transaction(client_->CreateTransaction( - request->key().first, - request->key().second, - base::Bind(&AsyncHostResolver::OnDnsTransactionComplete, - base::Unretained(this)), - request->request_net_log())); - int rv = transaction->Start(); - if (rv == ERR_IO_PENDING) - dns_transactions_.push_back(transaction.release()); - return rv; -} - -int AsyncHostResolver::Enqueue(Request* request) { - Request* evicted_request = Insert(request); - int rv = ERR_HOST_RESOLVER_QUEUE_TOO_LARGE; - if (evicted_request == request) - return rv; - if (evicted_request != NULL) { - evicted_request->OnAsyncComplete(rv, AddressList()); - delete evicted_request; - } - return ERR_IO_PENDING; -} - -AsyncHostResolver::Request* AsyncHostResolver::Insert(Request* request) { - pending_requests_[request->priority()].push_back(request); - if (GetNumPending() > max_pending_requests_) { - Request* req = RemoveLowest(); - DCHECK(req); - return req; - } - return NULL; -} - -size_t AsyncHostResolver::GetNumPending() const { - size_t num_pending = 0; - for (size_t i = 0; i < arraysize(pending_requests_); ++i) - num_pending += pending_requests_[i].size(); - return num_pending; -} - -AsyncHostResolver::Request* AsyncHostResolver::RemoveLowest() { - for (int i = static_cast<int>(arraysize(pending_requests_)) - 1; - i >= 0; --i) { - RequestList& requests = pending_requests_[i]; - if (!requests.empty()) { - Request* request = requests.front(); - requests.pop_front(); - return request; - } - } - return NULL; -} - -AsyncHostResolver::Request* AsyncHostResolver::RemoveHighest() { - for (size_t i = 0; i < arraysize(pending_requests_) - 1; ++i) { - RequestList& requests = pending_requests_[i]; - if (!requests.empty()) { - Request* request = requests.front(); - requests.pop_front(); - return request; - } - } - return NULL; -} - -void AsyncHostResolver::ProcessPending() { - Request* request = RemoveHighest(); - if (!request) - return; - for (size_t i = 0; i < arraysize(pending_requests_); ++i) { - RequestList& requests = pending_requests_[i]; - RequestList::iterator it = requests.begin(); - while (it != requests.end()) { - if (request->key() == (*it)->key()) { - requestlist_map_[request->key()].push_back(*it); - it = requests.erase(it); - } else { - ++it; - } - } - } - StartNewDnsRequestFor(request); -} - -} // namespace net diff --git a/net/dns/async_host_resolver.h b/net/dns/async_host_resolver.h deleted file mode 100644 index 23d9808..0000000 --- a/net/dns/async_host_resolver.h +++ /dev/null @@ -1,138 +0,0 @@ -// 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. - -#ifndef NET_DNS_ASYNC_HOST_RESOLVER_H_ -#define NET_DNS_ASYNC_HOST_RESOLVER_H_ -#pragma once - -#include <list> -#include <map> -#include <string> -#include <utility> - -#include "base/threading/non_thread_safe.h" -#include "net/base/address_family.h" -#include "net/base/host_cache.h" -#include "net/base/host_resolver.h" -#include "net/base/ip_endpoint.h" -#include "net/base/net_log.h" -#include "net/dns/dns_transaction.h" - -namespace net { - -class NET_EXPORT AsyncHostResolver - : public HostResolver, - NON_EXPORTED_BASE(public base::NonThreadSafe) { - public: - AsyncHostResolver(size_t max_dns_requests, - size_t max_pending_requests, - HostCache* cache, - scoped_ptr<DnsTransactionFactory> client, - NetLog* net_log); - virtual ~AsyncHostResolver(); - - // HostResolver interface - virtual int Resolve(const RequestInfo& info, - AddressList* addresses, - const CompletionCallback& callback, - RequestHandle* out_req, - const BoundNetLog& source_net_log) OVERRIDE; - virtual int ResolveFromCache(const RequestInfo& info, - AddressList* addresses, - const BoundNetLog& source_net_log) OVERRIDE; - virtual void CancelRequest(RequestHandle req_handle) OVERRIDE; - virtual void SetDefaultAddressFamily(AddressFamily address_family) OVERRIDE; - virtual AddressFamily GetDefaultAddressFamily() const OVERRIDE; - virtual HostCache* GetHostCache() OVERRIDE; - - void OnDnsTransactionComplete(DnsTransaction* transaction, - int result, - const DnsResponse* response); - - private: - FRIEND_TEST_ALL_PREFIXES(AsyncHostResolverTest, QueuedLookup); - FRIEND_TEST_ALL_PREFIXES(AsyncHostResolverTest, CancelPendingLookup); - FRIEND_TEST_ALL_PREFIXES(AsyncHostResolverTest, - ResolverDestructionCancelsLookups); - FRIEND_TEST_ALL_PREFIXES(AsyncHostResolverTest, - OverflowQueueWithLowPriorityLookup); - FRIEND_TEST_ALL_PREFIXES(AsyncHostResolverTest, - OverflowQueueWithHighPriorityLookup); - - class Request; - - typedef std::pair<std::string, uint16> Key; - typedef std::list<Request*> RequestList; - typedef std::list<const DnsTransaction*> DnsTransactionList; - typedef std::map<Key, RequestList> KeyRequestListMap; - - // Create a new request for the incoming Resolve() call. - Request* CreateNewRequest(const RequestInfo& info, - const CompletionCallback& callback, - AddressList* addresses, - const BoundNetLog& source_net_log); - - // Called when a request has just been started. - void OnStart(Request* request); - - // Called when a request has just completed (before its callback is run). - void OnFinish(Request* request, int result); - - // Called when a request has been cancelled. - void OnCancel(Request* request); - - // If there is an in-progress transaction for Request->key(), this will - // attach |request| to the respective list. - bool AttachToRequestList(Request* request); - - // Will start a new DNS request for |request|, will insert a new key in - // |requestlist_map_| and append |request| to the respective list. - int StartNewDnsRequestFor(Request* request); - - // Will enqueue |request| in |pending_requests_|. - int Enqueue(Request* request); - - // A helper used by Enqueue to insert |request| into |pending_requests_|. - Request* Insert(Request* request); - - // Returns the number of pending requests. - size_t GetNumPending() const; - - // Removes and returns a pointer to the lowest/highest priority request - // from |pending_requests_|. - Request* RemoveLowest(); - Request* RemoveHighest(); - - // Once a transaction has completed, called to start a new transaction if - // there are pending requests. - void ProcessPending(); - - // Maximum number of concurrent DNS transactions. - size_t max_dns_transactions_; - - // List of current DNS transactions. - DnsTransactionList dns_transactions_; - - // A map from Key to a list of requests waiting for the Key to resolve. - KeyRequestListMap requestlist_map_; - - // Maximum number of pending requests. - size_t max_pending_requests_; - - // Queues based on priority for putting pending requests. - RequestList pending_requests_[NUM_PRIORITIES]; - - // Cache of host resolution results. - scoped_ptr<HostCache> cache_; - - scoped_ptr<DnsTransactionFactory> client_; - - NetLog* net_log_; - - DISALLOW_COPY_AND_ASSIGN(AsyncHostResolver); -}; - -} // namespace net - -#endif // NET_DNS_ASYNC_HOST_RESOLVER_H_ diff --git a/net/dns/async_host_resolver_unittest.cc b/net/dns/async_host_resolver_unittest.cc deleted file mode 100644 index be57d49..0000000 --- a/net/dns/async_host_resolver_unittest.cc +++ /dev/null @@ -1,544 +0,0 @@ -// 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/async_host_resolver.h" - -#include "base/bind.h" -#include "base/memory/scoped_ptr.h" -#include "base/message_loop.h" -#include "base/stl_util.h" -#include "net/base/host_cache.h" -#include "net/base/net_errors.h" -#include "net/base/net_log.h" -#include "net/base/sys_addrinfo.h" -#include "net/base/test_completion_callback.h" -#include "net/dns/dns_query.h" -#include "net/dns/dns_response.h" -#include "net/dns/dns_test_util.h" -#include "net/dns/dns_transaction.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace net { - -namespace { - -const int kPortNum = 80; -const size_t kMaxTransactions = 2; -const size_t kMaxPendingRequests = 1; - -void VerifyAddressList(const std::vector<const char*>& ip_addresses, - int port, - const AddressList& addrlist) { - ASSERT_LT(0u, ip_addresses.size()); - ASSERT_NE(static_cast<addrinfo*>(NULL), addrlist.head()); - - IPAddressNumber ip_number; - const struct addrinfo* ainfo = addrlist.head(); - for (std::vector<const char*>::const_iterator i = ip_addresses.begin(); - i != ip_addresses.end(); ++i, ainfo = ainfo->ai_next) { - ASSERT_NE(static_cast<addrinfo*>(NULL), ainfo); - EXPECT_EQ(sizeof(struct sockaddr_in), - static_cast<size_t>(ainfo->ai_addrlen)); - - const struct sockaddr* sa = ainfo->ai_addr; - const struct sockaddr_in* sa_in = (const struct sockaddr_in*) sa; - EXPECT_TRUE(htons(port) == sa_in->sin_port); - EXPECT_STREQ(*i, NetAddressToString(sa, ainfo->ai_addrlen).c_str()); - } - ASSERT_EQ(static_cast<addrinfo*>(NULL), ainfo); -} - -class MockTransactionFactory : public DnsTransactionFactory, - public base::SupportsWeakPtr<MockTransactionFactory> { - public: - // Using WeakPtr to support cancellation. All MockTransactions succeed unless - // cancelled or MockTransactionFactory is destroyed. - class MockTransaction : public DnsTransaction, - public base::SupportsWeakPtr<MockTransaction> { - public: - MockTransaction(const std::string& hostname, - uint16 qtype, - const DnsTransactionFactory::CallbackType& callback, - const base::WeakPtr<MockTransactionFactory>& factory) - : hostname_(hostname), - qtype_(qtype), - callback_(callback), - started_(false), - factory_(factory) { - EXPECT_FALSE(started_); - started_ = true; - MessageLoop::current()->PostTask( - FROM_HERE, - base::Bind(&MockTransaction::Finish, AsWeakPtr())); - } - - virtual const std::string& GetHostname() const OVERRIDE { - return hostname_; - } - - virtual uint16 GetType() const OVERRIDE { - return qtype_; - } - - virtual int Start() OVERRIDE { - return ERR_IO_PENDING; - } - - private: - void Finish() { - if (!factory_) { - callback_.Run(this, ERR_DNS_SERVER_FAILED, NULL); - return; - } - callback_.Run(this, - OK, - factory_->responses_[Key(GetHostname(), GetType())]); - } - - const std::string hostname_; - const uint16 qtype_; - DnsTransactionFactory::CallbackType callback_; - bool started_; - const base::WeakPtr<MockTransactionFactory> factory_; - }; - - typedef std::pair<std::string, uint16> Key; - - MockTransactionFactory() : num_requests_(0) {} - ~MockTransactionFactory() { - STLDeleteValues(&responses_); - } - - scoped_ptr<DnsTransaction> CreateTransaction( - const std::string& qname, - uint16 qtype, - const DnsTransactionFactory::CallbackType& callback, - const BoundNetLog&) { - ++num_requests_; - return scoped_ptr<DnsTransaction>( - new MockTransaction(qname, qtype, callback, AsWeakPtr())); - } - - void AddResponse(const std::string& name, uint8 type, DnsResponse* response) { - responses_[MockTransactionFactory::Key(name, type)] = response; - } - - int num_requests() const { return num_requests_; } - - private: - int num_requests_; - std::map<Key, DnsResponse*> responses_; -}; - -} // namespace - - -// The following fixture sets up an environment for four different lookups -// with their data defined in dns_test_util.h. All tests make use of these -// predefined variables instead of each defining their own, to avoid -// boilerplate code in every test. Assuming every coming query is for a -// distinct hostname, as |kMaxTransactions| is set to 2 and -// |kMaxPendingRequests| is set to 1, first two queries start immediately -// and the next one is sent to pending queue; as a result, the next query -// should either fail itself or cause the pending query to fail depending -// on its priority. -class AsyncHostResolverTest : public testing::Test { - public: - AsyncHostResolverTest() - : info0_(HostPortPair(kT0HostName, kPortNum)), - info1_(HostPortPair(kT1HostName, kPortNum)), - info2_(HostPortPair(kT2HostName, kPortNum)), - info3_(HostPortPair(kT3HostName, kPortNum)), - ip_addresses0_(kT0IpAddresses, - kT0IpAddresses + arraysize(kT0IpAddresses)), - ip_addresses1_(kT1IpAddresses, - kT1IpAddresses + arraysize(kT1IpAddresses)), - ip_addresses2_(kT2IpAddresses, - kT2IpAddresses + arraysize(kT2IpAddresses)), - ip_addresses3_(kT3IpAddresses, - kT3IpAddresses + arraysize(kT3IpAddresses)) { - // AF_INET only for now. - info0_.set_address_family(ADDRESS_FAMILY_IPV4); - info1_.set_address_family(ADDRESS_FAMILY_IPV4); - info2_.set_address_family(ADDRESS_FAMILY_IPV4); - info3_.set_address_family(ADDRESS_FAMILY_IPV4); - - client_ = new MockTransactionFactory(); - - client_->AddResponse(kT0HostName, kT0Qtype, - new DnsResponse(reinterpret_cast<const char*>(kT0ResponseDatagram), - arraysize(kT0ResponseDatagram), - arraysize(kT0QueryDatagram))); - - client_->AddResponse(kT1HostName, kT1Qtype, - new DnsResponse(reinterpret_cast<const char*>(kT1ResponseDatagram), - arraysize(kT1ResponseDatagram), - arraysize(kT1QueryDatagram))); - - client_->AddResponse(kT2HostName, kT2Qtype, - new DnsResponse(reinterpret_cast<const char*>(kT2ResponseDatagram), - arraysize(kT2ResponseDatagram), - arraysize(kT2QueryDatagram))); - - client_->AddResponse(kT3HostName, kT3Qtype, - new DnsResponse(reinterpret_cast<const char*>(kT3ResponseDatagram), - arraysize(kT3ResponseDatagram), - arraysize(kT3QueryDatagram))); - - resolver_.reset( - new AsyncHostResolver(kMaxTransactions, kMaxPendingRequests, - HostCache::CreateDefaultCache(), - scoped_ptr<DnsTransactionFactory>(client_), NULL)); - } - - protected: - AddressList addrlist0_, addrlist1_, addrlist2_, addrlist3_; - HostResolver::RequestInfo info0_, info1_, info2_, info3_; - std::vector<const char*> ip_addresses0_, ip_addresses1_, - ip_addresses2_, ip_addresses3_; - scoped_ptr<HostResolver> resolver_; - MockTransactionFactory* client_; // Owned by the AsyncHostResolver. - TestCompletionCallback callback0_, callback1_, callback2_, callback3_; -}; - -TEST_F(AsyncHostResolverTest, EmptyHostLookup) { - info0_.set_host_port_pair(HostPortPair("", kPortNum)); - int rv = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(ERR_NAME_NOT_RESOLVED, rv); -} - -TEST_F(AsyncHostResolverTest, IPv4LiteralLookup) { - const char* kIPLiteral = "192.168.1.2"; - info0_.set_host_port_pair(HostPortPair(kIPLiteral, kPortNum)); - info0_.set_host_resolver_flags(HOST_RESOLVER_CANONNAME); - int rv = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(OK, rv); - std::vector<const char*> ip_addresses(1, kIPLiteral); - VerifyAddressList(ip_addresses, kPortNum, addrlist0_); - EXPECT_STREQ(kIPLiteral, addrlist0_.head()->ai_canonname); -} - -TEST_F(AsyncHostResolverTest, IPv6LiteralLookup) { - info0_.set_host_port_pair(HostPortPair("2001:db8:0::42", kPortNum)); - int rv = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - // When support for IPv6 is added, this should succeed. - EXPECT_EQ(ERR_NAME_NOT_RESOLVED, rv); -} - -TEST_F(AsyncHostResolverTest, CachedLookup) { - int rv = resolver_->ResolveFromCache(info0_, &addrlist0_, BoundNetLog()); - EXPECT_EQ(ERR_DNS_CACHE_MISS, rv); - - // Cache the result of |info0_| lookup. - rv = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(ERR_IO_PENDING, rv); - rv = callback0_.WaitForResult(); - EXPECT_EQ(OK, rv); - VerifyAddressList(ip_addresses0_, kPortNum, addrlist0_); - - // Now lookup |info0_| from cache only, store results in |addrlist1_|, - // should succeed synchronously. - rv = resolver_->ResolveFromCache(info0_, &addrlist1_, BoundNetLog()); - EXPECT_EQ(OK, rv); - VerifyAddressList(ip_addresses0_, kPortNum, addrlist1_); -} - -// TODO(szym): This tests DnsTransaction not AsyncHostResolver. Remove or move -// to dns_transaction_unittest.cc -TEST_F(AsyncHostResolverTest, DISABLED_InvalidHostNameLookup) { - const std::string kHostName1(64, 'a'); - info0_.set_host_port_pair(HostPortPair(kHostName1, kPortNum)); - int rv = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(ERR_INVALID_ARGUMENT, rv); - - const std::string kHostName2(4097, 'b'); - info0_.set_host_port_pair(HostPortPair(kHostName2, kPortNum)); - rv = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(ERR_INVALID_ARGUMENT, rv); -} - -TEST_F(AsyncHostResolverTest, Lookup) { - int rv = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(ERR_IO_PENDING, rv); - rv = callback0_.WaitForResult(); - EXPECT_EQ(OK, rv); - VerifyAddressList(ip_addresses0_, kPortNum, addrlist0_); -} - -TEST_F(AsyncHostResolverTest, ConcurrentLookup) { - int rv0 = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - int rv1 = resolver_->Resolve(info1_, &addrlist1_, callback1_.callback(), NULL, - BoundNetLog()); - int rv2 = resolver_->Resolve(info2_, &addrlist2_, callback2_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(ERR_IO_PENDING, rv0); - EXPECT_EQ(ERR_IO_PENDING, rv1); - EXPECT_EQ(ERR_IO_PENDING, rv2); - - rv0 = callback0_.WaitForResult(); - EXPECT_EQ(OK, rv0); - VerifyAddressList(ip_addresses0_, kPortNum, addrlist0_); - - rv1 = callback1_.WaitForResult(); - EXPECT_EQ(OK, rv1); - VerifyAddressList(ip_addresses1_, kPortNum, addrlist1_); - - rv2 = callback2_.WaitForResult(); - EXPECT_EQ(OK, rv2); - VerifyAddressList(ip_addresses2_, kPortNum, addrlist2_); - - EXPECT_EQ(3, client_->num_requests()); -} - -TEST_F(AsyncHostResolverTest, SameHostLookupsConsumeSingleTransaction) { - // We pass the info0_ to all requests. - int rv0 = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - int rv1 = resolver_->Resolve(info0_, &addrlist1_, callback1_.callback(), NULL, - BoundNetLog()); - int rv2 = resolver_->Resolve(info0_, &addrlist2_, callback2_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(ERR_IO_PENDING, rv0); - EXPECT_EQ(ERR_IO_PENDING, rv1); - EXPECT_EQ(ERR_IO_PENDING, rv2); - - rv0 = callback0_.WaitForResult(); - EXPECT_EQ(OK, rv0); - VerifyAddressList(ip_addresses0_, kPortNum, addrlist0_); - - rv1 = callback1_.WaitForResult(); - EXPECT_EQ(OK, rv1); - VerifyAddressList(ip_addresses0_, kPortNum, addrlist1_); - - rv2 = callback2_.WaitForResult(); - EXPECT_EQ(OK, rv2); - VerifyAddressList(ip_addresses0_, kPortNum, addrlist2_); - - // Although we have three lookups, a single UDP socket was used. - EXPECT_EQ(1, client_->num_requests()); -} - -TEST_F(AsyncHostResolverTest, CancelLookup) { - HostResolver::RequestHandle req0 = NULL, req2 = NULL; - int rv0 = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), - &req0, BoundNetLog()); - int rv1 = resolver_->Resolve(info1_, &addrlist1_, callback1_.callback(), NULL, - BoundNetLog()); - int rv2 = resolver_->Resolve(info2_, &addrlist2_, callback2_.callback(), - &req2, BoundNetLog()); - EXPECT_EQ(ERR_IO_PENDING, rv0); - EXPECT_EQ(ERR_IO_PENDING, rv1); - EXPECT_EQ(ERR_IO_PENDING, rv2); - - resolver_->CancelRequest(req0); - resolver_->CancelRequest(req2); - - MessageLoop::current()->RunAllPending(); - - EXPECT_FALSE(callback0_.have_result()); - EXPECT_FALSE(callback2_.have_result()); - - rv1 = callback1_.WaitForResult(); - EXPECT_EQ(OK, rv1); - VerifyAddressList(ip_addresses1_, kPortNum, addrlist1_); -} - -// Tests the following scenario: start two resolutions for the same host, -// cancel one of them, make sure that the other one completes. -TEST_F(AsyncHostResolverTest, CancelSameHostLookup) { - HostResolver::RequestHandle req0 = NULL; - - // Pass the info0_ to both requests. - int rv0 = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), - &req0, BoundNetLog()); - int rv1 = resolver_->Resolve(info0_, &addrlist1_, callback1_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(ERR_IO_PENDING, rv0); - EXPECT_EQ(ERR_IO_PENDING, rv1); - - resolver_->CancelRequest(req0); - MessageLoop::current()->RunAllPending(); - EXPECT_FALSE(callback0_.have_result()); - - rv1 = callback1_.WaitForResult(); - EXPECT_EQ(OK, rv1); - VerifyAddressList(ip_addresses0_, kPortNum, addrlist1_); - - EXPECT_EQ(1, client_->num_requests()); -} - -// Test that a queued lookup completes. -TEST_F(AsyncHostResolverTest, QueuedLookup) { - // kMaxTransactions is 2, thus the following requests consume all - // available transactions. - int rv0 = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - int rv1 = resolver_->Resolve(info1_, &addrlist1_, callback1_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(ERR_IO_PENDING, rv0); - EXPECT_EQ(ERR_IO_PENDING, rv1); - - // The following request will end up in queue. - int rv2 = resolver_->Resolve(info2_, &addrlist2_, callback2_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(ERR_IO_PENDING, rv2); - EXPECT_EQ(1u, - static_cast<AsyncHostResolver*>(resolver_.get())->GetNumPending()); - - // Make sure all requests complete. - rv0 = callback0_.WaitForResult(); - EXPECT_EQ(OK, rv0); - VerifyAddressList(ip_addresses0_, kPortNum, addrlist0_); - - rv1 = callback1_.WaitForResult(); - EXPECT_EQ(OK, rv1); - VerifyAddressList(ip_addresses1_, kPortNum, addrlist1_); - - rv2 = callback2_.WaitForResult(); - EXPECT_EQ(OK, rv2); - VerifyAddressList(ip_addresses2_, kPortNum, addrlist2_); -} - -// Test that cancelling a queued lookup works. -TEST_F(AsyncHostResolverTest, CancelPendingLookup) { - // kMaxTransactions is 2, thus the following requests consume all - // available transactions. - int rv0 = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - int rv1 = resolver_->Resolve(info1_, &addrlist1_, callback1_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(ERR_IO_PENDING, rv0); - EXPECT_EQ(ERR_IO_PENDING, rv1); - - // The following request will end up in queue. - HostResolver::RequestHandle req2 = NULL; - int rv2 = resolver_->Resolve(info2_, &addrlist2_, callback2_.callback(), - &req2, BoundNetLog()); - EXPECT_EQ(ERR_IO_PENDING, rv2); - EXPECT_EQ(1u, - static_cast<AsyncHostResolver*>(resolver_.get())->GetNumPending()); - - resolver_->CancelRequest(req2); - - // Make sure first two requests complete while the cancelled one doesn't. - MessageLoop::current()->RunAllPending(); - EXPECT_FALSE(callback2_.have_result()); - - rv0 = callback0_.WaitForResult(); - EXPECT_EQ(OK, rv0); - VerifyAddressList(ip_addresses0_, kPortNum, addrlist0_); - - rv1 = callback1_.WaitForResult(); - EXPECT_EQ(OK, rv1); - VerifyAddressList(ip_addresses1_, kPortNum, addrlist1_); -} - -TEST_F(AsyncHostResolverTest, ResolverDestructionCancelsLookups) { - int rv0 = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - int rv1 = resolver_->Resolve(info1_, &addrlist1_, callback1_.callback(), NULL, - BoundNetLog()); - // This one is queued. - int rv2 = resolver_->Resolve(info2_, &addrlist2_, callback2_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(1u, - static_cast<AsyncHostResolver*>(resolver_.get())->GetNumPending()); - - EXPECT_EQ(ERR_IO_PENDING, rv0); - EXPECT_EQ(ERR_IO_PENDING, rv1); - EXPECT_EQ(ERR_IO_PENDING, rv2); - - resolver_.reset(); - - MessageLoop::current()->RunAllPending(); - - EXPECT_FALSE(callback0_.have_result()); - EXPECT_FALSE(callback1_.have_result()); - EXPECT_FALSE(callback2_.have_result()); -} - -// Test that when the number of pending lookups is at max, a new lookup -// with a priority lower than all of those in the queue fails. -TEST_F(AsyncHostResolverTest, OverflowQueueWithLowPriorityLookup) { - int rv0 = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - int rv1 = resolver_->Resolve(info1_, &addrlist1_, callback1_.callback(), NULL, - BoundNetLog()); - // This one is queued and fills up the queue since its size is 1. - int rv2 = resolver_->Resolve(info2_, &addrlist2_, callback2_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(1u, - static_cast<AsyncHostResolver*>(resolver_.get())->GetNumPending()); - - EXPECT_EQ(ERR_IO_PENDING, rv0); - EXPECT_EQ(ERR_IO_PENDING, rv1); - EXPECT_EQ(ERR_IO_PENDING, rv2); - - // This one fails. - info3_.set_priority(LOWEST); - int rv3 = resolver_->Resolve(info3_, &addrlist3_, callback3_.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(ERR_HOST_RESOLVER_QUEUE_TOO_LARGE, rv3); - - MessageLoop::current()->RunAllPending(); - EXPECT_FALSE(callback3_.have_result()); -} - -// Test that when the number of pending lookups is at max, a new lookup -// with a priority higher than any of those in the queue succeeds and -// causes the lowest priority lookup in the queue to fail. -TEST_F(AsyncHostResolverTest, OverflowQueueWithHighPriorityLookup) { - int rv0 = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL, - BoundNetLog()); - int rv1 = resolver_->Resolve(info1_, &addrlist1_, callback1_.callback(), NULL, - BoundNetLog()); - - // Next lookup is queued. Since this will be ejected from the queue and - // will not consume a socket from our factory, we are not passing it - // predefined members. - HostResolver::RequestInfo info(HostPortPair("cnn.com", 80)); - info.set_address_family(ADDRESS_FAMILY_IPV4); - AddressList addrlist_fail; - TestCompletionCallback callback_fail; - int rv_fail = resolver_->Resolve(info, &addrlist_fail, - callback_fail.callback(), NULL, - BoundNetLog()); - EXPECT_EQ(1u, - static_cast<AsyncHostResolver*>(resolver_.get())->GetNumPending()); - - EXPECT_EQ(ERR_IO_PENDING, rv0); - EXPECT_EQ(ERR_IO_PENDING, rv1); - EXPECT_EQ(ERR_IO_PENDING, rv_fail); - - // Lookup 2 causes the above to fail, but itself should succeed. - info2_.set_priority(HIGHEST); - int rv2 = resolver_->Resolve(info2_, &addrlist2_, callback2_.callback(), NULL, - BoundNetLog()); - - rv0 = callback0_.WaitForResult(); - EXPECT_EQ(OK, rv0); - VerifyAddressList(ip_addresses0_, kPortNum, addrlist0_); - - rv1 = callback1_.WaitForResult(); - EXPECT_EQ(OK, rv1); - VerifyAddressList(ip_addresses1_, kPortNum, addrlist1_); - - rv_fail = callback_fail.WaitForResult(); - EXPECT_EQ(ERR_HOST_RESOLVER_QUEUE_TOO_LARGE, rv_fail); - EXPECT_EQ(static_cast<addrinfo*>(NULL), addrlist_fail.head()); - - rv2 = callback2_.WaitForResult(); - EXPECT_EQ(OK, rv2); - VerifyAddressList(ip_addresses2_, kPortNum, addrlist2_); -} - -} // namespace net diff --git a/net/dns/dns_config_service.h b/net/dns/dns_config_service.h index b3e151f..51e26b3 100644 --- a/net/dns/dns_config_service.h +++ b/net/dns/dns_config_service.h @@ -12,6 +12,7 @@ #include "base/file_path.h" #include "base/gtest_prod_util.h" +#include "base/memory/scoped_ptr.h" #include "base/observer_list.h" #include "base/threading/non_thread_safe.h" #include "base/time.h" @@ -64,6 +65,7 @@ struct NET_EXPORT_PRIVATE DnsConfig { // Service for watching when the system DNS settings have changed. // Depending on the platform, watches files in /etc/ or win registry. +// This class also serves as the do-nothing mock implementation. class NET_EXPORT_PRIVATE DnsConfigService : NON_EXPORTED_BASE(public base::NonThreadSafe) { public: @@ -78,7 +80,7 @@ class NET_EXPORT_PRIVATE DnsConfigService }; // Creates the platform-specific DnsConfigService. - static DnsConfigService* CreateSystemService(); + static scoped_ptr<DnsConfigService> CreateSystemService(); DnsConfigService(); virtual ~DnsConfigService(); diff --git a/net/dns/dns_config_service_posix.cc b/net/dns/dns_config_service_posix.cc index 18a69cc..67df4e9 100644 --- a/net/dns/dns_config_service_posix.cc +++ b/net/dns/dns_config_service_posix.cc @@ -84,8 +84,8 @@ void DnsConfigServicePosix::Watch() { } // static -DnsConfigService* DnsConfigService::CreateSystemService() { - return new DnsConfigServicePosix(); +scoped_ptr<DnsConfigService> DnsConfigService::CreateSystemService() { + return scoped_ptr<DnsConfigService>(new DnsConfigServicePosix()); } #if !defined(OS_ANDROID) diff --git a/net/dns/dns_config_service_posix_unittest.cc b/net/dns/dns_config_service_posix_unittest.cc index 58c71ac..f1ff015 100644 --- a/net/dns/dns_config_service_posix_unittest.cc +++ b/net/dns/dns_config_service_posix_unittest.cc @@ -110,7 +110,7 @@ void CloseResState(res_state res) { } // namespace -TEST(DnsConfigTest, ResolverConfigConvertAndEquals) { +TEST(DnsConfigServicePosixTest, ConvertResStateToDnsConfig) { struct __res_state res[2]; DnsConfig config[2]; for (unsigned i = 0; i < 2; ++i) { diff --git a/net/dns/dns_config_service_win.cc b/net/dns/dns_config_service_win.cc index d4a615f..14bb773 100644 --- a/net/dns/dns_config_service_win.cc +++ b/net/dns/dns_config_service_win.cc @@ -307,9 +307,6 @@ class DnsConfigServiceWin::ConfigReader : public SerialWorker { bool StartWatch() { DCHECK(loop()->BelongsToCurrentThread()); - // This is done only once per lifetime so open the keys on this thread. - base::ThreadRestrictions::ScopedAllowIO allow_io; - base::Closure callback = base::Bind(&SerialWorker::WorkNow, base::Unretained(this)); @@ -448,6 +445,12 @@ DnsConfigServiceWin::~DnsConfigServiceWin() { void DnsConfigServiceWin::Watch() { DCHECK(CalledOnValidThread()); + + // This is done only once per lifetime so open the keys and file watcher + // handles on this thread. + // TODO(szym): Should/can this be avoided? http://crbug.com/114223 + base::ThreadRestrictions::ScopedAllowIO allow_io; + bool started = config_reader_->StartWatch(); // TODO(szym): handle possible failure DCHECK(started); @@ -461,8 +464,8 @@ void DnsConfigServiceWin::Watch() { } // static -DnsConfigService* DnsConfigService::CreateSystemService() { - return new DnsConfigServiceWin(); +scoped_ptr<DnsConfigService> DnsConfigService::CreateSystemService() { + return scoped_ptr<DnsConfigService>(new DnsConfigServiceWin()); } } // namespace net diff --git a/net/dns/dns_response.cc b/net/dns/dns_response.cc index 760bd5b..9518095 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,8 @@ DnsRecordParser::DnsRecordParser(const void* packet, DCHECK_LE(offset, length); } -int DnsRecordParser::ParseName(const void* const vpos, std::string* out) const { +unsigned 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); @@ -41,9 +44,9 @@ int DnsRecordParser::ParseName(const void* const vpos, std::string* out) const { const char* p = pos; const char* end = packet_ + length_; // Count number of seen bytes to detect loops. - size_t seen = 0; + unsigned seen = 0; // Remember how many bytes were consumed before first jump. - size_t consumed = 0; + unsigned consumed = 0; if (pos >= end) return 0; @@ -54,7 +57,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 +108,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, @@ -182,7 +185,7 @@ uint8 DnsResponse::rcode() const { return ntohs(header()->flags) & dns_protocol::kRcodeMask; } -int DnsResponse::answer_count() const { +unsigned DnsResponse::answer_count() const { DCHECK(parser_.IsValid()); return ntohs(header()->ancount); } @@ -220,4 +223,71 @@ 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. + // We err on the side of caution with the assumption that if we are too picky, + // we can always fall back to the system getaddrinfo. + + // 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; + + uint32 cname_ttl_sec = kuint32max; + uint32 addr_ttl_sec = kuint32max; + IPAddressList ip_addresses; + DnsRecordParser parser = Parser(); + DnsResourceRecord record; + unsigned ancount = answer_count(); + for (unsigned i = 0; i < ancount; ++i) { + if (!parser.ReadRecord(&record)) + return DNS_MALFORMED_RESPONSE; + + if (base::strcasecmp(record.name.c_str(), expected_name.c_str()) != 0) + return DNS_NAME_MISMATCH; + if (record.type == dns_protocol::kTypeCNAME) { + // Following the CNAME chain, only if no addresses seen. + if (!ip_addresses.empty()) + return DNS_CNAME_AFTER_ADDRESS; + + if (record.rdata.size() != + parser.ReadName(record.rdata.begin(), &expected_name)) + return DNS_MALFORMED_CNAME; + + 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 (ip_addresses.empty()) + return DNS_NO_ADDRESSES; + + // 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, + expected_name); + *ttl = base::TimeDelta::FromSeconds(std::min(cname_ttl_sec, addr_ttl_sec)); + return DNS_SUCCESS; +} + } // namespace net diff --git a/net/dns/dns_response.h b/net/dns/dns_response.h index 764517a..4866045 100644 --- a/net/dns/dns_response.h +++ b/net/dns/dns_response.h @@ -11,11 +11,13 @@ #include "base/basictypes.h" #include "base/memory/ref_counted.h" #include "base/string_piece.h" +#include "base/time.h" #include "net/base/net_export.h" #include "net/base/net_util.h" namespace net { +class AddressList; class DnsQuery; class IOBufferWithSize; @@ -61,10 +63,10 @@ class NET_EXPORT_PRIVATE DnsRecordParser { // This is exposed to allow parsing compressed names within RRDATA for TYPEs // such as NS, CNAME, PTR, MX, SOA. // See RFC 1035 section 4.1.4. - int ParseName(const void* pos, std::string* out) const; + unsigned ReadName(const void* pos, std::string* out) const; - // Parses the next resource record. Returns true if succeeded. - bool ParseRecord(DnsResourceRecord* record); + // Parses the next resource record into |record|. Returns true if succeeded. + bool ReadRecord(DnsResourceRecord* record); private: const char* packet_; @@ -78,6 +80,20 @@ class NET_EXPORT_PRIVATE DnsRecordParser { // position the RR parser. class NET_EXPORT_PRIVATE DnsResponse { public: + // Possible results from ParseToAddressList + enum Result { + DNS_SUCCESS = 0, + DNS_MALFORMED_RESPONSE, // DnsRecordParser failed before the end of + // packet. + DNS_MALFORMED_CNAME, // Could not parse CNAME out of RRDATA. + DNS_NAME_MISMATCH, // Got an address but no ordered chain of CNAMEs + // leads there. + DNS_SIZE_MISMATCH, // Got an address but size does not match. + DNS_CNAME_AFTER_ADDRESS, // Found CNAME after an address record. + DNS_ADDRESS_TTL_MISMATCH, // TTL of all address records are not identical. + DNS_NO_ADDRESSES, // No address records found. + }; + // Constructs an object with an IOBuffer large enough to read // one byte more than largest possible response, to detect malformed // responses. @@ -102,7 +118,7 @@ class NET_EXPORT_PRIVATE DnsResponse { // Accessors for the header. uint16 flags() const; // excluding rcode uint8 rcode() const; - int answer_count() const; + unsigned answer_count() const; // Accessors to the question. The qname is unparsed. base::StringPiece qname() const; @@ -116,6 +132,10 @@ class NET_EXPORT_PRIVATE DnsResponse { // This operation is idempotent. DnsRecordParser Parser() const; + // Extracts an AddressList from this response. Returns SUCCESS if succeeded. + // Otherwise returns a detailed error number. + Result ParseToAddressList(AddressList* addr_list, base::TimeDelta* ttl) const; + private: // Convenience for header access. const dns_protocol::Header* header() const; diff --git a/net/dns/dns_response_unittest.cc b/net/dns/dns_response_unittest.cc index 41f3a72..6cbf6a1 100644 --- a/net/dns/dns_response_unittest.cc +++ b/net/dns/dns_response_unittest.cc @@ -4,9 +4,14 @@ #include "net/dns/dns_response.h" +#include "base/time.h" +#include "net/base/address_list.h" #include "net/base/io_buffer.h" +#include "net/base/net_util.h" +#include "net/base/sys_addrinfo.h" #include "net/dns/dns_protocol.h" #include "net/dns/dns_query.h" +#include "net/dns/dns_test_util.h" #include "testing/gtest/include/gtest/gtest.h" namespace net { @@ -24,7 +29,7 @@ TEST(DnsRecordParserTest, Constructor) { EXPECT_TRUE(DnsRecordParser(data, 1, 1).AtEnd()); } -TEST(DnsRecordParserTest, ParseName) { +TEST(DnsRecordParserTest, ReadName) { const uint8 data[] = { // all labels "foo.example.com" 0x03, 'f', 'o', 'o', @@ -46,31 +51,31 @@ TEST(DnsRecordParserTest, ParseName) { DnsRecordParser parser(data, sizeof(data), 0); ASSERT_TRUE(parser.IsValid()); - EXPECT_EQ(0x11, parser.ParseName(data + 0x00, &out)); + EXPECT_EQ(0x11u, parser.ReadName(data + 0x00, &out)); EXPECT_EQ("foo.example.com", out); // Check that the last "." is never stored. out.clear(); - EXPECT_EQ(0x1, parser.ParseName(data + 0x10, &out)); + EXPECT_EQ(0x1u, parser.ReadName(data + 0x10, &out)); EXPECT_EQ("", out); out.clear(); - EXPECT_EQ(0x6, parser.ParseName(data + 0x11, &out)); + EXPECT_EQ(0x6u, parser.ReadName(data + 0x11, &out)); EXPECT_EQ("bar.example.com", out); out.clear(); - EXPECT_EQ(0x2, parser.ParseName(data + 0x17, &out)); + EXPECT_EQ(0x2u, parser.ReadName(data + 0x17, &out)); EXPECT_EQ("bar.example.com", out); // Parse name without storing it. - EXPECT_EQ(0x11, parser.ParseName(data + 0x00, NULL)); - EXPECT_EQ(0x1, parser.ParseName(data + 0x10, NULL)); - EXPECT_EQ(0x6, parser.ParseName(data + 0x11, NULL)); - EXPECT_EQ(0x2, parser.ParseName(data + 0x17, NULL)); + EXPECT_EQ(0x11u, parser.ReadName(data + 0x00, NULL)); + EXPECT_EQ(0x1u, parser.ReadName(data + 0x10, NULL)); + EXPECT_EQ(0x6u, parser.ReadName(data + 0x11, NULL)); + EXPECT_EQ(0x2u, parser.ReadName(data + 0x17, NULL)); // Check that it works even if initial position is different. parser = DnsRecordParser(data, sizeof(data), 0x12); - EXPECT_EQ(0x6, parser.ParseName(data + 0x11, NULL)); + EXPECT_EQ(0x6u, parser.ReadName(data + 0x11, NULL)); } -TEST(DnsRecordParserTest, ParseNameFail) { +TEST(DnsRecordParserTest, ReadNameFail) { const uint8 data[] = { // label length beyond packet 0x30, 'x', 'x', @@ -90,15 +95,15 @@ TEST(DnsRecordParserTest, ParseNameFail) { ASSERT_TRUE(parser.IsValid()); std::string out; - EXPECT_EQ(0, parser.ParseName(data + 0x00, &out)); - EXPECT_EQ(0, parser.ParseName(data + 0x04, &out)); - EXPECT_EQ(0, parser.ParseName(data + 0x08, &out)); - EXPECT_EQ(0, parser.ParseName(data + 0x0a, &out)); - EXPECT_EQ(0, parser.ParseName(data + 0x0c, &out)); - EXPECT_EQ(0, parser.ParseName(data + 0x0e, &out)); + EXPECT_EQ(0u, parser.ReadName(data + 0x00, &out)); + EXPECT_EQ(0u, parser.ReadName(data + 0x04, &out)); + EXPECT_EQ(0u, parser.ReadName(data + 0x08, &out)); + EXPECT_EQ(0u, parser.ReadName(data + 0x0a, &out)); + EXPECT_EQ(0u, parser.ReadName(data + 0x0c, &out)); + EXPECT_EQ(0u, parser.ReadName(data + 0x0e, &out)); } -TEST(DnsRecordParserTest, ParseRecord) { +TEST(DnsRecordParserTest, ReadRecord) { const uint8 data[] = { // Type CNAME record. 0x07, 'e', 'x', 'a', 'm', 'p', 'l', 'e', @@ -124,17 +129,17 @@ TEST(DnsRecordParserTest, ParseRecord) { DnsRecordParser parser(data, sizeof(data), 0); DnsResourceRecord record; - EXPECT_TRUE(parser.ParseRecord(&record)); + EXPECT_TRUE(parser.ReadRecord(&record)); EXPECT_EQ("example.com", record.name); EXPECT_EQ(dns_protocol::kTypeCNAME, record.type); EXPECT_EQ(dns_protocol::kClassIN, record.klass); EXPECT_EQ(0x00012474u, record.ttl); EXPECT_EQ(6u, record.rdata.length()); - EXPECT_EQ(6, parser.ParseName(record.rdata.data(), &out)); + EXPECT_EQ(6u, parser.ReadName(record.rdata.data(), &out)); EXPECT_EQ("foo.example.com", out); EXPECT_FALSE(parser.AtEnd()); - EXPECT_TRUE(parser.ParseRecord(&record)); + EXPECT_TRUE(parser.ReadRecord(&record)); EXPECT_EQ("bar.example.com", record.name); EXPECT_EQ(dns_protocol::kTypeA, record.type); EXPECT_EQ(dns_protocol::kClassIN, record.klass); @@ -145,9 +150,9 @@ TEST(DnsRecordParserTest, ParseRecord) { // Test truncated record. parser = DnsRecordParser(data, sizeof(data) - 2, 0); - EXPECT_TRUE(parser.ParseRecord(&record)); + EXPECT_TRUE(parser.ReadRecord(&record)); EXPECT_FALSE(parser.AtEnd()); - EXPECT_FALSE(parser.ParseRecord(&record)); + EXPECT_FALSE(parser.ReadRecord(&record)); } TEST(DnsResponseTest, InitParse) { @@ -225,7 +230,7 @@ TEST(DnsResponseTest, InitParse) { // Check header access. EXPECT_EQ(0x8180, resp.flags()); EXPECT_EQ(0x0, resp.rcode()); - EXPECT_EQ(2, resp.answer_count()); + EXPECT_EQ(2u, resp.answer_count()); // Check question access. EXPECT_EQ(query->qname(), resp.qname()); @@ -234,11 +239,224 @@ TEST(DnsResponseTest, InitParse) { DnsResourceRecord record; DnsRecordParser parser = resp.Parser(); - EXPECT_TRUE(parser.ParseRecord(&record)); + EXPECT_TRUE(parser.ReadRecord(&record)); EXPECT_FALSE(parser.AtEnd()); - EXPECT_TRUE(parser.ParseRecord(&record)); + EXPECT_TRUE(parser.ReadRecord(&record)); EXPECT_TRUE(parser.AtEnd()); - EXPECT_FALSE(parser.ParseRecord(&record)); + EXPECT_FALSE(parser.ReadRecord(&record)); +} + +void VerifyAddressList(const std::vector<const char*>& ip_addresses, + const AddressList& addrlist) { + ASSERT_GT(ip_addresses.size(), 0u); + ASSERT_NE(static_cast<addrinfo*>(NULL), addrlist.head()); + + IPAddressNumber ip_number; + const struct addrinfo* ainfo = addrlist.head(); + for (std::vector<const char*>::const_iterator i = ip_addresses.begin(); + i != ip_addresses.end(); ++i, ainfo = ainfo->ai_next) { + ASSERT_NE(static_cast<addrinfo*>(NULL), ainfo); + EXPECT_EQ(sizeof(struct sockaddr_in), + static_cast<size_t>(ainfo->ai_addrlen)); + + const struct sockaddr* sa = ainfo->ai_addr; + EXPECT_STREQ(*i, NetAddressToString(sa, ainfo->ai_addrlen).c_str()); + } + ASSERT_EQ(static_cast<addrinfo*>(NULL), ainfo); +} + +TEST(DnsResponseTest, ParseToAddressList) { + const struct TestCase { + size_t query_size; + const uint8* response_data; + size_t response_size; + const char* const* expected_addresses; + size_t num_expected_addresses; + const char* expected_cname; + int expected_ttl_sec; + } cases[] = { + { + kT0QuerySize, + kT0ResponseDatagram, arraysize(kT0ResponseDatagram), + kT0IpAddresses, arraysize(kT0IpAddresses), + kT0CanonName, + kT0TTL, + }, + { + kT1QuerySize, + kT1ResponseDatagram, arraysize(kT1ResponseDatagram), + kT1IpAddresses, arraysize(kT1IpAddresses), + kT1CanonName, + kT1TTL, + }, + { + kT2QuerySize, + kT2ResponseDatagram, arraysize(kT2ResponseDatagram), + kT2IpAddresses, arraysize(kT2IpAddresses), + kT2CanonName, + kT2TTL, + }, + { + kT3QuerySize, + kT3ResponseDatagram, arraysize(kT3ResponseDatagram), + kT3IpAddresses, arraysize(kT3IpAddresses), + kT3CanonName, + kT3TTL, + }, + }; + + for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) { + const TestCase& t = cases[i]; + DnsResponse response(t.response_data, t.response_size, t.query_size); + AddressList addr_list; + base::TimeDelta ttl; + EXPECT_EQ(DnsResponse::DNS_SUCCESS, + response.ParseToAddressList(&addr_list, &ttl)); + std::vector<const char*> expected_addresses( + t.expected_addresses, + t.expected_addresses + t.num_expected_addresses); + VerifyAddressList(expected_addresses, addr_list); + std::string cname; + ASSERT_TRUE(addr_list.GetCanonicalName(&cname)); + EXPECT_EQ(t.expected_cname, cname); + EXPECT_EQ(base::TimeDelta::FromSeconds(t.expected_ttl_sec), ttl); + } +} + +const uint8 kResponseTruncatedRecord[] = { + // Header: 1 question, 1 answer RR + 0x00, 0x00, 0x81, 0x80, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, + // Question: name = 'a', type = A (0x1) + 0x01, 'a', 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer: name = 'a', type = A, TTL = 0xFF, RDATA = 10.10.10.10 + 0x01, 'a', 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, + 0x00, 0x04, 0x0A, 0x0A, 0x0A, // Truncated RDATA. +}; + +const uint8 kResponseTruncatedCNAME[] = { + // Header: 1 question, 1 answer RR + 0x00, 0x00, 0x81, 0x80, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, + // Question: name = 'a', type = A (0x1) + 0x01, 'a', 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer: name = 'a', type = CNAME, TTL = 0xFF, RDATA = 'foo' (truncated) + 0x01, 'a', 0x00, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, + 0x00, 0x03, 0x03, 'f', 'o', // Truncated name. +}; + +const uint8 kResponseNameMismatch[] = { + // Header: 1 question, 1 answer RR + 0x00, 0x00, 0x81, 0x80, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, + // Question: name = 'a', type = A (0x1) + 0x01, 'a', 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer: name = 'b', type = A, TTL = 0xFF, RDATA = 10.10.10.10 + 0x01, 'b', 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, + 0x00, 0x04, 0x0A, 0x0A, 0x0A, 0x0A, +}; + +const uint8 kResponseNameMismatchInChain[] = { + // Header: 1 question, 3 answer RR + 0x00, 0x00, 0x81, 0x80, 0x00, 0x01, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, + // Question: name = 'a', type = A (0x1) + 0x01, 'a', 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer: name = 'a', type = CNAME, TTL = 0xFF, RDATA = "b" + 0x01, 'a', 0x00, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, + 0x00, 0x03, 0x01, 'b', 0x00, + // Answer: name = 'b', type = A, TTL = 0xFF, RDATA = 10.10.10.10 + 0x01, 'b', 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, + 0x00, 0x04, 0x0A, 0x0A, 0x0A, 0x0A, + // Answer: name = 'c', type = A, TTL = 0xFF, RDATA = 10.10.10.11 + 0x01, 'c', 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, + 0x00, 0x04, 0x0A, 0x0A, 0x0A, 0x0B, +}; + +const uint8 kResponseSizeMismatch[] = { + // Header: 1 answer RR + 0x00, 0x00, 0x81, 0x80, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, + // Question: name = 'a', type = AAAA (0x1c) + 0x01, 'a', 0x00, 0x00, 0x1c, 0x00, 0x01, + // Answer: name = 'a', type = AAAA, TTL = 0xFF, RDATA = 10.10.10.10 + 0x01, 'a', 0x00, 0x00, 0x1c, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, + 0x00, 0x04, 0x0A, 0x0A, 0x0A, 0x0A, +}; + +const uint8 kResponseCNAMEAfterAddress[] = { + // Header: 2 answer RR + 0x00, 0x00, 0x81, 0x80, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, + // Question: name = 'a', type = A (0x1) + 0x01, 'a', 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer: name = 'a', type = A, TTL = 0xFF, RDATA = 10.10.10.10. + 0x01, 'a', 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, + 0x00, 0x04, 0x0A, 0x0A, 0x0A, 0x0A, + // Answer: name = 'a', type = CNAME, TTL = 0xFF, RDATA = "b" + 0x01, 'a', 0x00, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, + 0x00, 0x03, 0x01, 'b', 0x00, +}; + +const uint8 kResponseTTLMismatch[] = { + // Header: 1 question, 3 answer RR + 0x00, 0x00, 0x81, 0x80, 0x00, 0x01, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, + // Question: name = 'a', type = A (0x1) + 0x01, 'a', 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer: name = 'a', type = CNAME, TTL = 0xFF, RDATA = "b" + 0x01, 'a', 0x00, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, + 0x00, 0x03, 0x01, 'b', 0x00, + // Answer: name = 'b', type = A, TTL = 0xFF, RDATA = 10.10.10.10 + 0x01, 'b', 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, + 0x00, 0x04, 0x0A, 0x0A, 0x0A, 0x0A, + // Answer: name = 'b', type = A, TTL = 0xBB, RDATA = 10.10.10.11 + 0x01, 'b', 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0xBB, + 0x00, 0x04, 0x0A, 0x0A, 0x0A, 0x0B, +}; + +const uint8 kResponseNoAddresses[] = { + // Header: 1 question, 1 answer RR, 1 authority RR + 0x00, 0x00, 0x81, 0x80, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, + // Question: name = 'a', type = A (0x1) + 0x01, 'a', 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer: name = 'a', type = CNAME, TTL = 0xFF, RDATA = "b" + 0x01, 'a', 0x00, 0x00, 0x05, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, + 0x00, 0x03, 0x01, 'b', 0x00, + // Authority section + // Answer: name = 'b', type = A, TTL = 0xFF, RDATA = 10.10.10.10 + 0x01, 'b', 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, + 0x00, 0x04, 0x0A, 0x0A, 0x0A, 0x0A, +}; + +TEST(DnsResponseTest, ParseToAddressListFail) { + const struct TestCase { + const uint8* data; + size_t size; + DnsResponse::Result expected_result; + } cases[] = { + { kResponseTruncatedRecord, arraysize(kResponseTruncatedRecord), + DnsResponse::DNS_MALFORMED_RESPONSE }, + { kResponseTruncatedCNAME, arraysize(kResponseTruncatedCNAME), + DnsResponse::DNS_MALFORMED_CNAME }, + { kResponseNameMismatch, arraysize(kResponseNameMismatch), + DnsResponse::DNS_NAME_MISMATCH }, + { kResponseNameMismatchInChain, arraysize(kResponseNameMismatchInChain), + DnsResponse::DNS_NAME_MISMATCH }, + { kResponseSizeMismatch, arraysize(kResponseSizeMismatch), + DnsResponse::DNS_SIZE_MISMATCH }, + { kResponseCNAMEAfterAddress, arraysize(kResponseCNAMEAfterAddress), + DnsResponse::DNS_CNAME_AFTER_ADDRESS }, + { kResponseTTLMismatch, arraysize(kResponseTTLMismatch), + DnsResponse::DNS_ADDRESS_TTL_MISMATCH }, + { kResponseNoAddresses, arraysize(kResponseNoAddresses), + DnsResponse::DNS_NO_ADDRESSES }, + }; + + const size_t kQuerySize = 12 + 7; + + for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) { + const TestCase& t = cases[i]; + + DnsResponse response(t.data, t.size, kQuerySize); + AddressList addr_list; + base::TimeDelta ttl; + EXPECT_EQ(t.expected_result, + response.ParseToAddressList(&addr_list, &ttl)); + } } } // namespace diff --git a/net/dns/dns_session.h b/net/dns/dns_session.h index 85aea0a..aa070ac 100644 --- a/net/dns/dns_session.h +++ b/net/dns/dns_session.h @@ -34,7 +34,7 @@ class NET_EXPORT_PRIVATE DnsSession const DnsConfig& config() const { return config_; } NetLog* net_log() const { return net_log_; } - ClientSocketFactory* socket_factory() { return socket_factory_.get(); } + ClientSocketFactory* socket_factory() { return socket_factory_; } // Return the next random query ID. int NextQueryId() const; @@ -50,7 +50,7 @@ class NET_EXPORT_PRIVATE DnsSession ~DnsSession(); const DnsConfig config_; - scoped_ptr<ClientSocketFactory> socket_factory_; + ClientSocketFactory* socket_factory_; RandCallback rand_callback_; NetLog* net_log_; diff --git a/net/dns/dns_test_util.h b/net/dns/dns_test_util.h index 6c29058..8d8c0a6 100644 --- a/net/dns/dns_test_util.h +++ b/net/dns/dns_test_util.h @@ -21,13 +21,7 @@ static const char kT0DnsName[] = { 0x03, 'c', 'o', 'm', 0x00 }; -static const uint8 kT0QueryDatagram[] = { - // query for www.google.com, type A. - 0x00, 0x00, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x03, 0x77, 0x77, 0x77, - 0x06, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x03, - 0x63, 0x6f, 0x6d, 0x00, 0x00, 0x01, 0x00, 0x01 -}; +static const size_t kT0QuerySize = 32; static const uint8 kT0ResponseDatagram[] = { // response contains one CNAME for www.l.google.com and the following // IP addresses: 74.125.226.{179,180,176,177,178} @@ -53,6 +47,8 @@ static const char* const kT0IpAddresses[] = { "74.125.226.179", "74.125.226.180", "74.125.226.176", "74.125.226.177", "74.125.226.178" }; +static const char kT0CanonName[] = "www.l.google.com"; +static const int kT0TTL = 0x000000e4; //----------------------------------------------------------------------------- // Query/response set for codereview.chromium.org, ID is fixed to 1. @@ -64,15 +60,7 @@ static const char kT1DnsName[] = { 0x03, 'o', 'r', 'g', 0x00 }; -static const uint8 kT1QueryDatagram[] = { - // query for codereview.chromium.org, type A. - 0x00, 0x01, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x0a, 0x63, 0x6f, 0x64, - 0x65, 0x72, 0x65, 0x76, 0x69, 0x65, 0x77, 0x08, - 0x63, 0x68, 0x72, 0x6f, 0x6d, 0x69, 0x75, 0x6d, - 0x03, 0x6f, 0x72, 0x67, 0x00, 0x00, 0x01, 0x00, - 0x01 -}; +static const size_t kT1QuerySize = 41; static const uint8 kT1ResponseDatagram[] = { // response contains one CNAME for ghs.l.google.com and the following // IP address: 64.233.169.121 @@ -91,6 +79,8 @@ static const uint8 kT1ResponseDatagram[] = { static const char* const kT1IpAddresses[] = { "64.233.169.121" }; +static const char kT1CanonName[] = "ghs.l.google.com"; +static const int kT1TTL = 0x0000010b; //----------------------------------------------------------------------------- // Query/response set for www.ccs.neu.edu, ID is fixed to 2. @@ -103,14 +93,7 @@ static const char kT2DnsName[] = { 0x03, 'e', 'd', 'u', 0x00 }; -static const uint8 kT2QueryDatagram[] = { - // query for www.ccs.neu.edu, type A. - 0x00, 0x02, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x03, 0x77, 0x77, 0x77, - 0x03, 0x63, 0x63, 0x73, 0x03, 0x6e, 0x65, 0x75, - 0x03, 0x65, 0x64, 0x75, 0x00, 0x00, 0x01, 0x00, - 0x01 -}; +static const size_t kT2QuerySize = 33; static const uint8 kT2ResponseDatagram[] = { // response contains one CNAME for vulcan.ccs.neu.edu and the following // IP address: 129.10.116.81 @@ -127,6 +110,8 @@ static const uint8 kT2ResponseDatagram[] = { static const char* const kT2IpAddresses[] = { "129.10.116.81" }; +static const char kT2CanonName[] = "vulcan.ccs.neu.edu"; +static const int kT2TTL = 0x0000012c; //----------------------------------------------------------------------------- // Query/response set for www.google.az, ID is fixed to 3. @@ -138,13 +123,7 @@ static const char kT3DnsName[] = { 0x02, 'a', 'z', 0x00 }; -static const uint8 kT3QueryDatagram[] = { - // query for www.google.az, type A. - 0x00, 0x03, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x03, 0x77, 0x77, 0x77, - 0x06, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x02, - 0x61, 0x7a, 0x00, 0x00, 0x01, 0x00, 0x01 -}; +static const size_t kT3QuerySize = 31; static const uint8 kT3ResponseDatagram[] = { // response contains www.google.com as CNAME for www.google.az and // www.l.google.com as CNAME for www.google.com and the following @@ -174,6 +153,8 @@ static const char* const kT3IpAddresses[] = { "74.125.226.178", "74.125.226.179", "74.125.226.180", "74.125.226.176", "74.125.226.177" }; +static const char kT3CanonName[] = "www.l.google.com"; +static const int kT3TTL = 0x00000015; } // namespace net diff --git a/net/dns/dns_transaction.cc b/net/dns/dns_transaction.cc index 6cd02a3..8bffa12 100644 --- a/net/dns/dns_transaction.cc +++ b/net/dns/dns_transaction.cc @@ -51,22 +51,19 @@ bool IsIPLiteral(const std::string& hostname) { class StartParameters : public NetLog::EventParameters { public: StartParameters(const std::string& hostname, - uint16 qtype, - const NetLog::Source& source) - : hostname_(hostname), qtype_(qtype), source_(source) {} + uint16 qtype) + : hostname_(hostname), qtype_(qtype) {} virtual Value* ToValue() const OVERRIDE { DictionaryValue* dict = new DictionaryValue(); dict->SetString("hostname", hostname_); dict->SetInteger("query_type", qtype_); - dict->Set("source_dependency", source_.ToValue()); return dict; } private: const std::string hostname_; const uint16 qtype_; - const NetLog::Source source_; }; class ResponseParameters : public NetLog::EventParameters { @@ -78,7 +75,7 @@ class ResponseParameters : public NetLog::EventParameters { DictionaryValue* dict = new DictionaryValue(); dict->SetInteger("rcode", rcode_); dict->SetInteger("answer_count", answer_count_); - dict->Set("socket_source", source_.ToValue()); + dict->Set("source_dependency", source_.ToValue()); return dict; } @@ -211,8 +208,14 @@ class DnsUDPAttempt { return rv; DCHECK(rv); - if (!response_->InitParse(rv, *query_)) + if (!response_->InitParse(rv, *query_)) { + // TODO(szym): Consider making this reaction less aggressive. + // Other implementations simply ignore mismatched responses. Since each + // DnsUDPAttempt binds to a different port, we might find that responses + // to previously timed out queries lead to failures in the future. + // http://crbug.com/107413 return ERR_DNS_MALFORMED_RESPONSE; + } if (response_->flags() & dns_protocol::kFlagTC) return ERR_DNS_SERVER_REQUIRES_TCP; if (response_->rcode() != dns_protocol::kRcodeNOERROR && @@ -258,13 +261,12 @@ class DnsTransactionImpl : public DnsTransaction, public base::NonThreadSafe { const std::string& hostname, uint16 qtype, const DnsTransactionFactory::CallbackType& callback, - const BoundNetLog& source_net_log) + const BoundNetLog& net_log) : session_(session), hostname_(hostname), qtype_(qtype), callback_(callback), - net_log_(BoundNetLog::Make(session->net_log(), - NetLog::SOURCE_DNS_TRANSACTION)), + net_log_(net_log), first_server_index_(0) { DCHECK(session_); DCHECK(!hostname_.empty()); @@ -273,7 +275,7 @@ class DnsTransactionImpl : public DnsTransaction, public base::NonThreadSafe { DCHECK(!IsIPLiteral(hostname_)); net_log_.BeginEvent(NetLog::TYPE_DNS_TRANSACTION, make_scoped_refptr( - new StartParameters(hostname_, qtype_, source_net_log.source()))); + new StartParameters(hostname_, qtype_))); } virtual ~DnsTransactionImpl() { @@ -299,7 +301,19 @@ class DnsTransactionImpl : public DnsTransaction, public base::NonThreadSafe { int rv = PrepareSearch(); if (rv == OK) rv = StartQuery(); - DCHECK_NE(OK, rv); + if (rv == OK) { + // In the very unlikely case that we immediately received the response, we + // cannot simply return OK nor run the callback, but instead complete + // asynchronously. + // TODO(szym): replace Unretained with WeakPtr factory. + MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(&DnsTransactionImpl::DoCallback, + base::Unretained(this), + OK, + attempts_.back())); + return ERR_IO_PENDING; + } return rv; } @@ -387,7 +401,8 @@ class DnsTransactionImpl : public DnsTransaction, public base::NonThreadSafe { } net_log_.AddEvent(NetLog::TYPE_DNS_TRANSACTION_ATTEMPT, make_scoped_refptr( - new NetLogSourceParameter("socket_source", socket->NetLog().source()))); + new NetLogSourceParameter("source_dependency", + socket->NetLog().source()))); const DnsConfig& config = session_->config(); @@ -451,7 +466,7 @@ class DnsTransactionImpl : public DnsTransaction, public base::NonThreadSafe { DoCallback(rv, attempt); return; default: - // TODO(szym): Some nameservers could fail so try the next one. + // Some nameservers could fail so try the next one. const DnsConfig& config = session_->config(); if (attempts_.size() < config.attempts * config.nameservers.size()) { rv = MakeAttempt(); @@ -512,12 +527,12 @@ class DnsTransactionFactoryImpl : public DnsTransactionFactory { const std::string& hostname, uint16 qtype, const CallbackType& callback, - const BoundNetLog& source_net_log) OVERRIDE { + const BoundNetLog& net_log) OVERRIDE { return scoped_ptr<DnsTransaction>(new DnsTransactionImpl(session_, hostname, qtype, callback, - source_net_log)); + net_log)); } private: diff --git a/net/dns/dns_transaction.h b/net/dns/dns_transaction.h index 038f4b2..383f4a5 100644 --- a/net/dns/dns_transaction.h +++ b/net/dns/dns_transaction.h @@ -61,12 +61,12 @@ class NET_EXPORT_PRIVATE DnsTransactionFactory { // search. |hostname| should not be an IP literal. // // The transaction will run |callback| upon asynchronous completion. - // The source of |source_net_log| is used as source dependency in log. + // The |net_log| is used as the parent log. virtual scoped_ptr<DnsTransaction> CreateTransaction( const std::string& hostname, uint16 qtype, const CallbackType& callback, - const BoundNetLog& source_net_log) WARN_UNUSED_RESULT = 0; + const BoundNetLog& net_log) WARN_UNUSED_RESULT = 0; // Creates a DnsTransactionFactory which creates DnsTransactionImpl using the // |session|. diff --git a/net/dns/dns_transaction_unittest.cc b/net/dns/dns_transaction_unittest.cc index 8e0888a..b21eb794 100644 --- a/net/dns/dns_transaction_unittest.cc +++ b/net/dns/dns_transaction_unittest.cc @@ -141,14 +141,16 @@ class TransactionHelper { if (expected_answer_count_ >= 0) { EXPECT_EQ(OK, rv); - EXPECT_EQ(expected_answer_count_, response->answer_count()); + 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.ParseRecord(&record)); + EXPECT_TRUE(parser.ReadRecord(&record)); } + // Technically, there could be additional RRs, but not in our test data. EXPECT_TRUE(parser.AtEnd()); } else { EXPECT_EQ(expected_answer_count_, rv); @@ -212,10 +214,10 @@ class DnsTransactionTest : public testing::Test { // Called after fully configuring |config|. void ConfigureFactory() { - socket_factory_ = new TestSocketFactory(); + socket_factory_.reset(new TestSocketFactory()); session_ = new DnsSession( config_, - socket_factory_, + socket_factory_.get(), base::Bind(&DnsTransactionTest::GetNextId, base::Unretained(this)), NULL /* NetLog */); transaction_factory_ = DnsTransactionFactory::CreateFactory(session_.get()); @@ -232,7 +234,7 @@ class DnsTransactionTest : public testing::Test { uint16 id, const char* data, size_t data_length) { - CHECK(socket_factory_); + CHECK(socket_factory_.get()); DnsQuery* query = new DnsQuery(id, DomainFromDot(dotted_name), qtype); queries_.push_back(query); @@ -252,7 +254,7 @@ class DnsTransactionTest : public testing::Test { // Add expected query of |dotted_name| and |qtype| and no response. void AddTimeout(const char* dotted_name, uint16 qtype) { - CHECK(socket_factory_); + CHECK(socket_factory_.get()); uint16 id = base::RandInt(0, kuint16max); DnsQuery* query = new DnsQuery(id, DomainFromDot(dotted_name), qtype); queries_.push_back(query); @@ -268,7 +270,7 @@ class DnsTransactionTest : public testing::Test { // 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) { - CHECK(socket_factory_); + 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); @@ -360,8 +362,7 @@ class DnsTransactionTest : public testing::Test { ScopedVector<DelayedSocketData> socket_data_; std::deque<int> transaction_ids_; - // Owned by |session_|. - TestSocketFactory* socket_factory_; + scoped_ptr<TestSocketFactory> socket_factory_; scoped_refptr<DnsSession> session_; scoped_ptr<DnsTransactionFactory> transaction_factory_; }; diff --git a/net/net.gyp b/net/net.gyp index bd530f1..e3bdb7a 100644 --- a/net/net.gyp +++ b/net/net.gyp @@ -327,8 +327,6 @@ 'disk_cache/stress_support.h', 'disk_cache/trace.cc', 'disk_cache/trace.h', - 'dns/async_host_resolver.cc', - 'dns/async_host_resolver.h', 'dns/dns_config_service.cc', 'dns/dns_config_service.h', 'dns/dns_config_service_posix.cc', @@ -1070,7 +1068,6 @@ 'disk_cache/entry_unittest.cc', 'disk_cache/mapped_file_unittest.cc', 'disk_cache/storage_block_unittest.cc', - 'dns/async_host_resolver_unittest.cc', 'dns/dns_config_service_posix_unittest.cc', 'dns/dns_config_service_unittest.cc', 'dns/dns_config_service_win_unittest.cc', |