[net/dns] Fixes to DnsConfigServiceWin

Reject empty string16 early to avoid exception when converting to string.

Default to 53, if port is not set.

Fix CreateAdapterAddresses for the case there is more than one address.

BUG=113692, 113693
TEST=./net_unittest --gtest_filter=DnsConfigService*


Review URL: http://codereview.chromium.org/9384009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@121573 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 59 insertions, 11 deletions

diff --git a/net/dns/dns_config_service_win.cc b/net/dns/dns_config_service_win.cc
index e089387..d4a615f 100644
--- a/net/dns/dns_config_service_win.cc
+++ b/net/dns/dns_config_service_win.cc
@@ -21,6 +21,7 @@
 #include "base/win/windows_version.h"
 #include "googleurl/src/url_canon.h"
 #include "net/base/net_util.h"
+#include "net/dns/dns_protocol.h"
 #include "net/dns/serial_worker.h"
 #include "net/dns/watching_file_reader.h"
 
@@ -106,10 +107,13 @@ class RegistryWatcher : public base::win::ObjectWatcher::Delegate {
 };
 
 // Converts a string16 domain name to ASCII, possibly using punycode.
-// Returns true if the conversion succeeds. In case of failure, |domain| might
-// become dirty.
+// Returns true if the conversion succeeds and output is not empty. In case of
+// failure, |domain| might become dirty.
 bool ParseDomainASCII(const string16& widestr, std::string* domain) {
   DCHECK(domain);
+  if (widestr.empty())
+    return false;
+
   // Check if already ASCII.
   if (IsStringASCII(widestr)) {
     *domain = UTF16ToASCII(widestr);
@@ -129,7 +133,7 @@ bool ParseDomainASCII(const string16& widestr, std::string* domain) {
   bool success = UTF16ToUTF8(punycode.data(), punycode.length(), domain);
   DCHECK(success);
   DCHECK(IsStringASCII(*domain));
-  return success;
+  return success && !domain->empty();
 }
 
 }  // namespace
@@ -152,7 +156,7 @@ bool ParseSearchList(const string16& value, std::vector<std::string>* output) {
     // handle such suffixes.
     const string16& t = woutput[i];
     std::string parsed;
-    if (t.empty() || !ParseDomainASCII(t, &parsed))
+    if (!ParseDomainASCII(t, &parsed))
       break;
     output->push_back(parsed);
   }
@@ -181,6 +185,9 @@ bool ConvertSettingsToDnsConfig(const DnsSystemSettings& settings,
       IPEndPoint ipe;
       if (ipe.FromSockAddr(address->Address.lpSockaddr,
                            address->Address.iSockaddrLength)) {
+        // Override unset port.
+        if (!ipe.port())
+          ipe = IPEndPoint(ipe.address(), dns_protocol::kDefaultPort);
         config->nameservers.push_back(ipe);
       } else {
         return false;
diff --git a/net/dns/dns_config_service_win_unittest.cc b/net/dns/dns_config_service_win_unittest.cc
index 917656b..269bb55 100644
--- a/net/dns/dns_config_service_win_unittest.cc
+++ b/net/dns/dns_config_service_win_unittest.cc
@@ -5,6 +5,7 @@
 #include "net/dns/dns_config_service_win.h"
 
 #include "base/win/windows_version.h"
+#include "net/dns/dns_protocol.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
@@ -52,6 +53,7 @@ struct AdapterInfo {
   IF_OPER_STATUS oper_status;
   PWCHAR dns_suffix;
   std::string dns_server_addresses[4];  // Empty string indicates end.
+  int ports[4];
 };
 
 scoped_ptr_malloc<IP_ADAPTER_ADDRESSES> CreateAdapterAddresses(
@@ -93,11 +95,12 @@ scoped_ptr_malloc<IP_ADAPTER_ADDRESSES> CreateAdapterAddresses(
       if (j == 0) {
         address = adapter->FirstDnsServerAddress = addresses + num_addresses;
       } else {
-        address = address->Next = address + 1;
+        // Note that |address| is moving backwards.
+        address = address->Next = address - 1;
       }
       IPAddressNumber ip;
       CHECK(ParseIPLiteralToNumber(info.dns_server_addresses[j], &ip));
-      IPEndPoint ipe(ip, 53);
+      IPEndPoint ipe(ip, info.ports[j]);
       address->Address.lpSockaddr =
           reinterpret_cast<LPSOCKADDR>(storage + num_addresses);
       size_t length = sizeof(struct sockaddr_storage);
@@ -109,12 +112,13 @@ scoped_ptr_malloc<IP_ADAPTER_ADDRESSES> CreateAdapterAddresses(
   return heap.Pass();
 }
 
-TEST(DnsConfigServiceWinTest, ConvertAdaptersAddresses) {
+TEST(DnsConfigServiceWinTest, ConvertAdapterAddresses) {
   // Check nameservers and connection-specific suffix.
   const struct TestCase {
     AdapterInfo input_adapters[4];        // |if_type| == 0 indicates end.
     std::string expected_nameservers[4];  // Empty string indicates end.
     std::string expected_suffix;
+    int expected_ports[4];
   } cases[] = {
     {  // Ignore loopback and inactive adapters.
       {
@@ -129,6 +133,16 @@ TEST(DnsConfigServiceWinTest, ConvertAdaptersAddresses) {
       { "10.0.0.10", "2001:FFFF::1111" },
       "chromium.org",
     },
+    {  // Respect configured ports.
+      {
+        { IF_TYPE_USB, IfOperStatusUp, L"chromium.org",
+        { "10.0.0.10", "2001:FFFF::1111" }, { 1024, 24 } },
+        { 0 },
+      },
+      { "10.0.0.10", "2001:FFFF::1111" },
+      "chromium.org",
+      { 1024, 24 },
+    },
     {  // No usable nameservers.
       {
         { IF_TYPE_SOFTWARE_LOOPBACK, IfOperStatusUp, L"localhost",
@@ -151,7 +165,10 @@ TEST(DnsConfigServiceWinTest, ConvertAdaptersAddresses) {
     for (size_t j = 0; !t.expected_nameservers[j].empty(); ++j) {
       IPAddressNumber ip;
       ASSERT_TRUE(ParseIPLiteralToNumber(t.expected_nameservers[j], &ip));
-      expected_nameservers.push_back(IPEndPoint(ip, 53));
+      int port = t.expected_ports[j];
+      if (!port)
+        port = dns_protocol::kDefaultPort;
+      expected_nameservers.push_back(IPEndPoint(ip, port));
     }
 
     DnsConfig config;
@@ -203,6 +220,16 @@ TEST(DnsConfigServiceWinTest, ConvertSuffixSearch) {
       },
       { "tcpip.domain", "connection.suffix" },
     },
+    {  // No primary suffix. Devolution does not matter.
+      {
+        CreateAdapterAddresses(infos),
+        { false },
+        { false },
+        { true },
+        { { true, 1 }, { true, 2 } },
+      },
+      { "connection.suffix" },
+    },
     {  // Devolution enabled by policy, level by dnscache.
       {
         CreateAdapterAddresses(infos),
@@ -239,6 +266,18 @@ TEST(DnsConfigServiceWinTest, ConvertSuffixSearch) {
       },
       { "a.b.c.d.e", "connection.suffix", "b.c.d.e", "c.d.e" },
     },
+    {  // Devolution enabled at level = 2, but nothing to devolve.
+      {
+        CreateAdapterAddresses(infos),
+        { false },
+        { false },
+        { true, L"a.b" },
+        { { false }, { false } },
+        { { false }, { true, 2 } },
+        { { false }, { true, 2 } },
+      },
+      { "a.b", "connection.suffix" },
+    },
     {  // Devolution disabled when no explicit level.
        // Windows XP and Vista use a default level = 2, but we don't.
       {
diff --git a/net/dns/dns_protocol.h b/net/dns/dns_protocol.h
index 25f485a..38e1975 100644
--- a/net/dns/dns_protocol.h
+++ b/net/dns/dns_protocol.h
@@ -13,6 +13,8 @@ namespace net {
 
 namespace dns_protocol {
 
+static const uint16 kDefaultPort = 53;
+
 // DNS packet consists of a header followed by questions and/or answers.
 // For the meaning of specific fields, please see RFC 1035 and 2535
 
diff --git a/net/dns/dns_test_util.h b/net/dns/dns_test_util.h
index e1fc9fe..6c29058 100644
--- a/net/dns/dns_test_util.h
+++ b/net/dns/dns_test_util.h
@@ -11,8 +11,6 @@
 
 namespace net {
 
-static const uint16 kDnsPort = 53;
-
 //-----------------------------------------------------------------------------
 // Query/response set for www.google.com, ID is fixed to 0.
 static const char kT0HostName[] = "www.google.com";
diff --git a/net/dns/dns_transaction_unittest.cc b/net/dns/dns_transaction_unittest.cc
index d3eadb9..8e0888a 100644
--- a/net/dns/dns_transaction_unittest.cc
+++ b/net/dns/dns_transaction_unittest.cc
@@ -205,7 +205,8 @@ class DnsTransactionTest : public testing::Test {
     }
     for (unsigned i = 0; i < num_servers; ++i) {
       dns_ip[3] = i;
-      config_.nameservers.push_back(IPEndPoint(dns_ip, kDnsPort));
+      config_.nameservers.push_back(IPEndPoint(dns_ip,
+                                               dns_protocol::kDefaultPort));
     }
   }
 
diff --git a/net/net.gyp b/net/net.gyp
index 754a4cc..b0c100d 100644
--- a/net/net.gyp
+++ b/net/net.gyp
@@ -336,6 +336,7 @@
         'dns/dns_config_service_win.h',
         'dns/dns_hosts.cc',
         'dns/dns_hosts.h',
+        'dns/dns_protocol.h',
         'dns/dns_query.cc',
         'dns/dns_query.h',
         'dns/dns_response.cc',