shill: Connection: Don't call Resolver::SetDNSFromIPConfig
Don't ever call this function (in fact, remove this from the
resolver), and refactor the code that correctly generates the
DNS search list so that it is usable both from
Connection::UpdateFromIPConfig() and from void
Connection::SetIsDefault().
BUG=chromium-os:34260
TEST=Rerun unit tests + Manual: Connect to AP that supplies only
"DomainName", and ensure that /etc/resolv.conf contains this domain
in the search field both initially and after DHCP renewal.
Change-Id: I9a0705cb54e6588915533cc343b05efcdee71293
Reviewed-on: https://gerrit.chromium.org/gerrit/32996
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Ready: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/connection.cc b/connection.cc
index 4e18521..20496fb 100644
--- a/connection.cc
+++ b/connection.cc
@@ -191,7 +191,7 @@
ipconfig_rpc_identifier_ = config->GetRpcIdentifier();
if (is_default_) {
- resolver_->SetDNSFromIPConfig(config, dns_timeout_parameters_);
+ PushDNSConfig();
}
local_ = local;
@@ -212,12 +212,7 @@
is_default_ = is_default;
if (is_default) {
- vector<string> domain_search = dns_domain_search_;
- if (domain_search.empty() && !dns_domain_name_.empty()) {
- domain_search.push_back(dns_domain_name_ + ".");
- }
- resolver_->SetDNSFromLists(dns_servers_, domain_search,
- dns_timeout_parameters_);
+ PushDNSConfig();
DeviceRefPtr device = device_info_->GetDevice(interface_index_);
if (device) {
device->RequestPortalDetection();
@@ -226,6 +221,17 @@
routing_table_->FlushCache();
}
+void Connection::PushDNSConfig() {
+ vector<string> domain_search = dns_domain_search_;
+ if (domain_search.empty() && !dns_domain_name_.empty()) {
+ SLOG(Connection, 2) << "Setting domain search to domain name "
+ << dns_domain_name_;
+ domain_search.push_back(dns_domain_name_ + ".");
+ }
+ resolver_->SetDNSFromLists(dns_servers_, domain_search,
+ dns_timeout_parameters_);
+}
+
void Connection::RequestRouting() {
if (routing_request_count_++ == 0) {
DeviceRefPtr device = device_info_->GetDevice(interface_index_);
diff --git a/connection.h b/connection.h
index 491b9cb..187b421 100644
--- a/connection.h
+++ b/connection.h
@@ -138,6 +138,9 @@
void OnLowerDisconnect();
+ // Send our DNS configuration to the resolver.
+ void PushDNSConfig();
+
base::WeakPtrFactory<Connection> weak_ptr_factory_;
bool is_default_;
diff --git a/connection_unittest.cc b/connection_unittest.cc
index 9fc5edd..8faff31 100644
--- a/connection_unittest.cc
+++ b/connection_unittest.cc
@@ -386,8 +386,9 @@
ConfigureRoutes(kTestDeviceInterfaceIndex0,
ipconfig_,
GetDefaultMetric()));
- EXPECT_CALL(resolver_, SetDNSFromIPConfig(ipconfig_,
- Resolver::kDefaultTimeout));
+ EXPECT_CALL(resolver_, SetDNSFromLists(ipconfig_->properties().dns_servers,
+ ipconfig_->properties().domain_search,
+ Resolver::kDefaultTimeout));
connection_->UpdateFromIPConfig(ipconfig_);
}
@@ -425,8 +426,9 @@
EXPECT_CALL(routing_table_, FlushCache()).WillOnce(Return(true));
connection->SetIsDefault(true);
EXPECT_CALL(*device_info_, HasOtherAddress(_, _)).WillOnce(Return(false));
- EXPECT_CALL(resolver_, SetDNSFromIPConfig(ipconfig_,
- Resolver::kShortTimeout));
+ EXPECT_CALL(resolver_, SetDNSFromLists(ipconfig_->properties().dns_servers,
+ ipconfig_->properties().domain_search,
+ Resolver::kShortTimeout));
connection->UpdateFromIPConfig(ipconfig_);
AddDestructorExpectations();
}
diff --git a/mock_resolver.h b/mock_resolver.h
index 5850ec5..fe2a5a2 100644
--- a/mock_resolver.h
+++ b/mock_resolver.h
@@ -20,8 +20,6 @@
MockResolver();
virtual ~MockResolver();
- MOCK_METHOD2(SetDNSFromIPConfig, bool(const IPConfigRefPtr &ipconfig,
- TimeoutParameters timeout));
MOCK_METHOD3(SetDNSFromLists,
bool(const std::vector<std::string> &dns_servers,
const std::vector<std::string> &domain_search,
diff --git a/resolver.cc b/resolver.cc
index c9594f3..e10c587 100644
--- a/resolver.cc
+++ b/resolver.cc
@@ -38,22 +38,13 @@
return g_resolver.Pointer();
}
-bool Resolver::SetDNSFromIPConfig(const IPConfigRefPtr &ipconfig,
- TimeoutParameters timeout) {
- SLOG(Resolver, 2) << __func__;
-
- CHECK(!path_.empty());
-
- const IPConfig::Properties &props = ipconfig->properties();
-
- return SetDNSFromLists(props.dns_servers, props.domain_search, timeout);
-}
-
bool Resolver::SetDNSFromLists(const std::vector<std::string> &dns_servers,
const std::vector<std::string> &domain_search,
TimeoutParameters timeout) {
SLOG(Resolver, 2) << __func__;
+ CHECK(!path_.empty());
+
if (dns_servers.empty() && domain_search.empty()) {
SLOG(Resolver, 2) << "DNS list is empty";
return ClearDNS();
diff --git a/resolver.h b/resolver.h
index 8c56712..2aeb4a7 100644
--- a/resolver.h
+++ b/resolver.h
@@ -36,11 +36,9 @@
virtual void set_path(const FilePath &path) { path_ = path; }
- // Set the default domain name service parameters, given an ipconfig entry.
- virtual bool SetDNSFromIPConfig(const IPConfigRefPtr &ipconfig,
- TimeoutParameters timeout);
-
- // Set the default domain name service parameters, given an ipconfig entry.
+ // Install domain name service parameters, given a list of
+ // DNS servers in |dns_servers|, a list of DNS search suffixes in
+ // |domain_search| and a DNS timeout parameter in |timeout|.
virtual bool SetDNSFromLists(const std::vector<std::string> &dns_servers,
const std::vector<std::string> &domain_search,
TimeoutParameters timeout);
diff --git a/resolver_unittest.cc b/resolver_unittest.cc
index 31501e2..2ede654 100644
--- a/resolver_unittest.cc
+++ b/resolver_unittest.cc
@@ -10,7 +10,6 @@
#include <base/stringprintf.h>
#include <gtest/gtest.h>
-#include "shill/ipconfig.h"
#include "shill/mock_control.h"
using std::string;
@@ -70,18 +69,16 @@
EXPECT_FALSE(file_util::PathExists(path_));
EXPECT_TRUE(resolver_->ClearDNS());
- // Add DNS info from an IPConfig entry
MockControl control;
- IPConfigRefPtr ipconfig(new IPConfig(&control, kTestDeviceName0));
- IPConfig::Properties properties;
- properties.dns_servers.push_back(kNameServer0);
- properties.dns_servers.push_back(kNameServer1);
- properties.domain_search.push_back(kSearchDomain0);
- properties.domain_search.push_back(kSearchDomain1);
- ipconfig->UpdateProperties(properties, true);
+ vector<string> dns_servers;
+ vector<string> domain_search;
+ dns_servers.push_back(kNameServer0);
+ dns_servers.push_back(kNameServer1);
+ domain_search.push_back(kSearchDomain0);
+ domain_search.push_back(kSearchDomain1);
- EXPECT_TRUE(resolver_->SetDNSFromIPConfig(
- ipconfig, Resolver::kDefaultTimeout));
+ EXPECT_TRUE(resolver_->SetDNSFromLists(
+ dns_servers, domain_search, Resolver::kDefaultTimeout));
EXPECT_TRUE(file_util::PathExists(path_));
EXPECT_EQ(kExpectedOutput, ReadFile());
@@ -92,18 +89,16 @@
EXPECT_FALSE(file_util::PathExists(path_));
EXPECT_TRUE(resolver_->ClearDNS());
- // Add DNS info from an IPConfig entry
MockControl control;
- IPConfigRefPtr ipconfig(new IPConfig(&control, kTestDeviceName0));
- IPConfig::Properties properties;
- properties.dns_servers.push_back(kNameServer0);
- properties.dns_servers.push_back(kNameServer1);
- properties.domain_search.push_back(kSearchDomain0);
- properties.domain_search.push_back(kSearchDomain1);
- ipconfig->UpdateProperties(properties, true);
+ vector<string> dns_servers;
+ vector<string> domain_search;
+ dns_servers.push_back(kNameServer0);
+ dns_servers.push_back(kNameServer1);
+ domain_search.push_back(kSearchDomain0);
+ domain_search.push_back(kSearchDomain1);
- EXPECT_TRUE(resolver_->SetDNSFromIPConfig(
- ipconfig, Resolver::kShortTimeout));
+ EXPECT_TRUE(resolver_->SetDNSFromLists(
+ dns_servers, domain_search, Resolver::kShortTimeout));
EXPECT_TRUE(file_util::PathExists(path_));
EXPECT_EQ(kExpectedShortTimeoutOutput, ReadFile());
@@ -113,14 +108,12 @@
TEST_F(ResolverTest, Empty) {
EXPECT_FALSE(file_util::PathExists(path_));
- // Use empty ifconfig
MockControl control;
- IPConfigRefPtr ipconfig(new IPConfig(&control, kTestDeviceName0));
- IPConfig::Properties properties;
- ipconfig->UpdateProperties(properties, true);
+ vector<string> dns_servers;
+ vector<string> domain_search;
- EXPECT_TRUE(resolver_->SetDNSFromIPConfig(
- ipconfig, Resolver::kDefaultTimeout));
+ EXPECT_TRUE(resolver_->SetDNSFromLists(
+ dns_servers, domain_search, Resolver::kDefaultTimeout));
EXPECT_FALSE(file_util::PathExists(path_));
}