ParseHosts: Allow commas as separators on Mac OS X

Apparently, OS X allows commas as separators between hostnames in the hosts
file. Treat commas the same as whitespace to support this. (Hostnames and IP
addresses will never contain commas, so this shouldn't break anything.)

BUG=396309

Review URL: https://codereview.chromium.org/415153002

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

net/dns/dns_hosts.cc

@@ -8,24 +8,25 @@
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_util.h"
-#include "base/strings/string_tokenizer.h"
 
 using base::StringPiece;
 
 namespace net {
 
+namespace {
+
 // Parses the contents of a hosts file.  Returns one token (IP or hostname) at
 // a time.  Doesn't copy anything; accepts the file as a StringPiece and
 // returns tokens as StringPieces.
 class HostsParser {
  public:
-  explicit HostsParser(const StringPiece& text)
+  explicit HostsParser(const StringPiece& text, ParseHostsCommaMode comma_mode)
       : text_(text),
         data_(text.data()),
         end_(text.size()),
         pos_(0),
-        token_(),
-        token_is_ip_(false) {}
+        token_is_ip_(false),
+        comma_mode_(comma_mode) {}
 
   // Advances to the next token (IP or hostname).  Returns whether another
   // token was available.  |token_is_ip| and |token| can be used to find out
@@ -49,6 +50,14 @@ class HostsParser {
           SkipRestOfLine();
           break;
 
+        case ',':
+          if (comma_mode_ == PARSE_HOSTS_COMMA_IS_WHITESPACE) {
+            SkipWhitespace();
+            break;
+          }
+
+          // If comma_mode_ is COMMA_IS_TOKEN, fall through:
+
         default: {
           size_t token_start = pos_;
           SkipToken();
@@ -62,7 +71,6 @@ class HostsParser {
       }
     }
 
-    text_ = StringPiece();
     return false;
   }
 
@@ -85,14 +93,28 @@ class HostsParser {
 
  private:
   void SkipToken() {
-    pos_ = text_.find_first_of(" \t\n\r#", pos_);
+    switch (comma_mode_) {
+      case PARSE_HOSTS_COMMA_IS_TOKEN:
+        pos_ = text_.find_first_of(" \t\n\r#", pos_);
+        break;
+      case PARSE_HOSTS_COMMA_IS_WHITESPACE:
+        pos_ = text_.find_first_of(" ,\t\n\r#", pos_);
+        break;
+    }
   }
 
   void SkipWhitespace() {
-    pos_ = text_.find_first_not_of(" \t", pos_);
+    switch (comma_mode_) {
+      case PARSE_HOSTS_COMMA_IS_TOKEN:
+        pos_ = text_.find_first_not_of(" \t", pos_);
+        break;
+      case PARSE_HOSTS_COMMA_IS_WHITESPACE:
+        pos_ = text_.find_first_not_of(" ,\t", pos_);
+        break;
+    }
   }
 
-  StringPiece text_;
+  const StringPiece text_;
   const char* data_;
   const size_t end_;
 
@@ -100,19 +122,21 @@ class HostsParser {
   StringPiece token_;
   bool token_is_ip_;
 
+  const ParseHostsCommaMode comma_mode_;
+
   DISALLOW_COPY_AND_ASSIGN(HostsParser);
 };
 
-
-
-void ParseHosts(const std::string& contents, DnsHosts* dns_hosts) {
+void ParseHostsWithCommaMode(const std::string& contents,
+                             DnsHosts* dns_hosts,
+                             ParseHostsCommaMode comma_mode) {
   CHECK(dns_hosts);
   DnsHosts& hosts = *dns_hosts;
 
   StringPiece ip_text;
   IPAddressNumber ip;
   AddressFamily family = ADDRESS_FAMILY_IPV4;
-  HostsParser parser(contents);
+  HostsParser parser(contents, comma_mode);
   while (parser.Advance()) {
     if (parser.token_is_ip()) {
       StringPiece new_ip_text = parser.token();
@@ -140,6 +164,27 @@ void ParseHosts(const std::string& contents, DnsHosts* dns_hosts) {
   }
 }
 
+}  // namespace
+
+void ParseHostsWithCommaModeForTesting(const std::string& contents,
+                                       DnsHosts* dns_hosts,
+                                       ParseHostsCommaMode comma_mode) {
+  ParseHostsWithCommaMode(contents, dns_hosts, comma_mode);
+}
+
+void ParseHosts(const std::string& contents, DnsHosts* dns_hosts) {
+  ParseHostsCommaMode comma_mode;
+#if defined(OS_MACOSX)
+  // Mac OS X allows commas to separate hostnames.
+  comma_mode = PARSE_HOSTS_COMMA_IS_WHITESPACE;
+#else
+  // Linux allows commas in hostnames.
+  comma_mode = PARSE_HOSTS_COMMA_IS_TOKEN;
+#endif
+
+  ParseHostsWithCommaMode(contents, dns_hosts, comma_mode);
+}
+
 bool ParseHostsFile(const base::FilePath& path, DnsHosts* dns_hosts) {
   dns_hosts->clear();
   // Missing file indicates empty HOSTS.

net/dns/dns_hosts.h

@@ -44,6 +44,17 @@ inline size_t hash_value(const net::DnsHostsKey& key) {
 
 namespace net {
 
+// There are OS-specific variations in how commas in the hosts file behave.
+enum ParseHostsCommaMode {
+  // Comma is treated as part of a hostname:
+  // "127.0.0.1 foo,bar" parses as "foo,bar" mapping to "127.0.0.1".
+  PARSE_HOSTS_COMMA_IS_TOKEN,
+
+  // Comma is treated as a hostname separator:
+  // "127.0.0.1 foo,bar" parses as "foo" and "bar" both mapping to "127.0.0.1".
+  PARSE_HOSTS_COMMA_IS_WHITESPACE,
+};
+
 // Parsed results of a Hosts file.
 //
 // Although Hosts files map IP address to a list of domain names, for name
@@ -62,6 +73,15 @@ typedef base::hash_map<DnsHostsKey, IPAddressNumber> DnsHosts;
 typedef std::map<DnsHostsKey, IPAddressNumber> DnsHosts;
 #endif
 
+// Parses |contents| (as read from /etc/hosts or equivalent) and stores results
+// in |dns_hosts|. Invalid lines are ignored (as in most implementations).
+// Overrides the OS-specific default handling of commas, so unittests can test
+// both modes.
+void NET_EXPORT_PRIVATE ParseHostsWithCommaModeForTesting(
+    const std::string& contents,
+    DnsHosts* dns_hosts,
+    ParseHostsCommaMode comma_mode);
+
 // Parses |contents| (as read from /etc/hosts or equivalent) and stores results
 // in |dns_hosts|. Invalid lines are ignored (as in most implementations).
 void NET_EXPORT_PRIVATE ParseHosts(const std::string& contents,

net/dns/dns_hosts_unittest.cc

@@ -10,8 +10,27 @@ namespace net {
 
 namespace {
 
+struct ExpectedHostsEntry {
+  const char* host;
+  AddressFamily family;
+  const char* ip;
+};
+
+void PopulateExpectedHosts(const ExpectedHostsEntry* entries,
+                           size_t num_entries,
+                           DnsHosts* expected_hosts_out) {
+  for (size_t i = 0; i < num_entries; ++i) {
+    DnsHostsKey key(entries[i].host, entries[i].family);
+    IPAddressNumber& ip_ref = (*expected_hosts_out)[key];
+    ASSERT_TRUE(ip_ref.empty());
+    ASSERT_TRUE(ParseIPLiteralToNumber(entries[i].ip, &ip_ref));
+    ASSERT_EQ(ip_ref.size(),
+        (entries[i].family == ADDRESS_FAMILY_IPV4) ? 4u : 16u);
+  }
+}
+
 TEST(DnsHostsTest, ParseHosts) {
-  std::string contents =
+  const std::string kContents =
       "127.0.0.1       localhost\tlocalhost.localdomain # standard\n"
       "\n"
       "1.0.0.1 localhost # ignored, first hit above\n"
@@ -30,11 +49,7 @@ TEST(DnsHostsTest, ParseHosts) {
       "127.0.0.2 cache5\n"
       "gibberish";
 
-  const struct {
-    const char* host;
-    AddressFamily family;
-    const char* ip;
-  } entries[] = {
+  const ExpectedHostsEntry kEntries[] = {
     { "localhost", ADDRESS_FAMILY_IPV4, "127.0.0.1" },
     { "localhost.localdomain", ADDRESS_FAMILY_IPV4, "127.0.0.1" },
     { "company", ADDRESS_FAMILY_IPV4, "1.0.0.1" },
@@ -50,18 +65,60 @@ TEST(DnsHostsTest, ParseHosts) {
     { "cache5", ADDRESS_FAMILY_IPV4, "127.0.0.2" },
   };
 
-  DnsHosts expected;
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(entries); ++i) {
-    DnsHostsKey key(entries[i].host, entries[i].family);
-    IPAddressNumber& ip = expected[key];
-    ASSERT_TRUE(ip.empty());
-    ASSERT_TRUE(ParseIPLiteralToNumber(entries[i].ip, &ip));
-    ASSERT_EQ(ip.size(), (entries[i].family == ADDRESS_FAMILY_IPV4) ? 4u : 16u);
-  }
+  DnsHosts expected_hosts, actual_hosts;
+  PopulateExpectedHosts(kEntries, ARRAYSIZE_UNSAFE(kEntries), &expected_hosts);
+  ParseHosts(kContents, &actual_hosts);
+  ASSERT_EQ(expected_hosts, actual_hosts);
+}
 
-  DnsHosts hosts;
-  ParseHosts(contents, &hosts);
-  ASSERT_EQ(expected, hosts);
+TEST(DnsHostsTest, ParseHosts_CommaIsToken) {
+  const std::string kContents = "127.0.0.1 comma1,comma2";
+
+  const ExpectedHostsEntry kEntries[] = {
+    { "comma1,comma2", ADDRESS_FAMILY_IPV4, "127.0.0.1" },
+  };
+
+  DnsHosts expected_hosts, actual_hosts;
+  PopulateExpectedHosts(kEntries, ARRAYSIZE_UNSAFE(kEntries), &expected_hosts);
+  ParseHostsWithCommaModeForTesting(
+      kContents, &actual_hosts, PARSE_HOSTS_COMMA_IS_TOKEN);
+  ASSERT_EQ(expected_hosts, actual_hosts);
+}
+
+TEST(DnsHostsTest, ParseHosts_CommaIsWhitespace) {
+  std::string kContents = "127.0.0.1 comma1,comma2";
+
+  const ExpectedHostsEntry kEntries[] = {
+    { "comma1", ADDRESS_FAMILY_IPV4, "127.0.0.1" },
+    { "comma2", ADDRESS_FAMILY_IPV4, "127.0.0.1" },
+  };
+
+  DnsHosts expected_hosts, actual_hosts;
+  PopulateExpectedHosts(kEntries, ARRAYSIZE_UNSAFE(kEntries), &expected_hosts);
+  ParseHostsWithCommaModeForTesting(
+      kContents, &actual_hosts, PARSE_HOSTS_COMMA_IS_WHITESPACE);
+  ASSERT_EQ(expected_hosts, actual_hosts);
+}
+
+// Test that the right comma mode is used on each platform.
+TEST(DnsHostsTest, ParseHosts_CommaModeByPlatform) {
+  std::string kContents = "127.0.0.1 comma1,comma2";
+
+#if defined(OS_MACOSX)
+  const ExpectedHostsEntry kEntries[] = {
+    { "comma1", ADDRESS_FAMILY_IPV4, "127.0.0.1" },
+    { "comma2", ADDRESS_FAMILY_IPV4, "127.0.0.1" },
+  };
+#else
+  const ExpectedHostsEntry kEntries[] = {
+    { "comma1,comma2", ADDRESS_FAMILY_IPV4, "127.0.0.1" },
+  };
+#endif
+
+  DnsHosts expected_hosts, actual_hosts;
+  PopulateExpectedHosts(kEntries, ARRAYSIZE_UNSAFE(kEntries), &expected_hosts);
+  ParseHosts(kContents, &actual_hosts);
+  ASSERT_EQ(expected_hosts, actual_hosts);
 }
 
 TEST(DnsHostsTest, HostsParser_Empty) {