net: include the root label in DNSDomainFromDot

I was previously sloppy with DNSDomainFromDot. Really it should have
included a terminating NUL in the result string to represent the root
label, but it actually omitted it because it was more convenient to
test that way. This CL fixes that.

BUG=none
TEST=net_unittests

http://codereview.chromium.org/3070002/show

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

5 files changed, 34 insertions, 22 deletions

diff --git a/net/base/dns_util.cc b/net/base/dns_util.cc
index 87d1866..2305b7b 100644
--- a/net/base/dns_util.cc
+++ b/net/base/dns_util.cc
@@ -52,7 +52,7 @@ bool DNSDomainFromDot(const std::string& dotted, std::string* out) {
     return false;
   name[namelen++] = 0;
 
-  *out = name;
+  *out = std::string(name, namelen);
   return true;
 }
 
diff --git a/net/base/dns_util_unittest.cc b/net/base/dns_util_unittest.cc
index 7995b92..6ad7e1d 100644
--- a/net/base/dns_util_unittest.cc
+++ b/net/base/dns_util_unittest.cc
@@ -10,38 +10,44 @@ namespace net {
 class DNSUtilTest : public testing::Test {
 };
 
+// IncludeNUL converts a char* to a std::string and includes the terminating
+// NUL in the result.
+static std::string IncludeNUL(const char* in) {
+  return std::string(in, strlen(in) + 1);
+}
+
 TEST_F(DNSUtilTest, DNSDomainFromDot) {
   std::string out;
 
   EXPECT_TRUE(DNSDomainFromDot("", &out));
-  EXPECT_EQ(out, "");
+  EXPECT_EQ(out, IncludeNUL(""));
   EXPECT_TRUE(DNSDomainFromDot("com", &out));
-  EXPECT_EQ(out, "\003com");
+  EXPECT_EQ(out, IncludeNUL("\003com"));
   EXPECT_TRUE(DNSDomainFromDot("google.com", &out));
-  EXPECT_EQ(out, "\x006google\003com");
+  EXPECT_EQ(out, IncludeNUL("\x006google\003com"));
   EXPECT_TRUE(DNSDomainFromDot("www.google.com", &out));
-  EXPECT_EQ(out, "\003www\006google\003com");
+  EXPECT_EQ(out, IncludeNUL("\003www\006google\003com"));
 
   // Label is 63 chars: still valid
   EXPECT_TRUE(DNSDomainFromDot("123456789a123456789a123456789a123456789a123456789a123456789a123", &out));
-  EXPECT_EQ(out, "\077123456789a123456789a123456789a123456789a123456789a123456789a123");
+  EXPECT_EQ(out, IncludeNUL("\077123456789a123456789a123456789a123456789a123456789a123456789a123"));
 
   // Label is too long: invalid
   EXPECT_FALSE(DNSDomainFromDot("123456789a123456789a123456789a123456789a123456789a123456789a1234", &out));
 
   // 253 characters in the name: still valid
   EXPECT_TRUE(DNSDomainFromDot("123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123", &out));
-  EXPECT_EQ(out, "\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\003123");
+  EXPECT_EQ(out, IncludeNUL("\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\011123456789\003123"));
 
   // 254 characters in the name: invalid
   EXPECT_FALSE(DNSDomainFromDot("123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.1234", &out));
 
   // Zero length labels should be dropped.
   EXPECT_TRUE(DNSDomainFromDot("www.google.com.", &out));
-  EXPECT_EQ(out, "\003www\006google\003com");
+  EXPECT_EQ(out, IncludeNUL("\003www\006google\003com"));
 
   EXPECT_TRUE(DNSDomainFromDot(".google.com", &out));
-  EXPECT_EQ(out, "\006google\003com");
+  EXPECT_EQ(out, IncludeNUL("\006google\003com"));
 }
 
 TEST_F(DNSUtilTest, STD3ASCII) {
diff --git a/net/base/transport_security_state.cc b/net/base/transport_security_state.cc
index 952e78b..4fcc2b8 100644
--- a/net/base/transport_security_state.cc
+++ b/net/base/transport_security_state.cc
@@ -29,7 +29,7 @@ void TransportSecurityState::EnableHost(const std::string& host,
     return;
 
   bool temp;
-  if (isPreloadedSTS(canonicalised_host, &temp))
+  if (IsPreloadedSTS(canonicalised_host, &temp))
     return;
 
   char hashed[base::SHA256_LENGTH];
@@ -45,6 +45,12 @@ void TransportSecurityState::EnableHost(const std::string& host,
   DirtyNotify();
 }
 
+// IncludeNUL converts a char* to a std::string and includes the terminating
+// NUL in the result.
+static std::string IncludeNUL(const char* in) {
+  return std::string(in, strlen(in) + 1);
+}
+
 bool TransportSecurityState::IsEnabledForHost(DomainState* result,
                                               const std::string& host) {
   const std::string canonicalised_host = CanonicaliseHost(host);
@@ -52,7 +58,7 @@ bool TransportSecurityState::IsEnabledForHost(DomainState* result,
     return false;
 
   bool include_subdomains;
-  if (isPreloadedSTS(canonicalised_host, &include_subdomains)) {
+  if (IsPreloadedSTS(canonicalised_host, &include_subdomains)) {
     result->created = result->expiry = base::Time::FromTimeT(0);
     result->mode = DomainState::MODE_STRICT;
     result->include_subdomains = include_subdomains;
@@ -64,7 +70,7 @@ bool TransportSecurityState::IsEnabledForHost(DomainState* result,
   for (size_t i = 0; canonicalised_host[i]; i += canonicalised_host[i] + 1) {
     char hashed_domain[base::SHA256_LENGTH];
 
-    base::SHA256HashString(&canonicalised_host[i], &hashed_domain,
+    base::SHA256HashString(IncludeNUL(&canonicalised_host[i]), &hashed_domain,
                            sizeof(hashed_domain));
     std::map<std::string, DomainState>::iterator j =
         enabled_hosts_.find(std::string(hashed_domain, sizeof(hashed_domain)));
@@ -380,10 +386,10 @@ std::string TransportSecurityState::CanonicaliseHost(const std::string& host) {
   return new_host;
 }
 
-// isPreloadedSTS returns true if the canonicalised hostname should always be
+// IsPreloadedSTS returns true if the canonicalised hostname should always be
 // considered to have STS enabled.
 // static
-bool TransportSecurityState::isPreloadedSTS(
+bool TransportSecurityState::IsPreloadedSTS(
     const std::string& canonicalised_host, bool *include_subdomains) {
   // In the medium term this list is likely to just be hardcoded here. This,
   // slightly odd, form removes the need for additional relocations records.
@@ -402,7 +408,7 @@ bool TransportSecurityState::isPreloadedSTS(
 
   for (size_t i = 0; canonicalised_host[i]; i += canonicalised_host[i] + 1) {
     for (size_t j = 0; j < kNumPreloadedSTS; j++) {
-      if (kPreloadedSTS[j].length == canonicalised_host.size() + 1 - i &&
+      if (kPreloadedSTS[j].length == canonicalised_host.size() - i &&
           (kPreloadedSTS[j].include_subdomains || i == 0) &&
           memcmp(kPreloadedSTS[j].dns_name, &canonicalised_host[i],
                  kPreloadedSTS[j].length) == 0) {
diff --git a/net/base/transport_security_state.h b/net/base/transport_security_state.h
index 2cd6abe..9cb2884 100644
--- a/net/base/transport_security_state.h
+++ b/net/base/transport_security_state.h
@@ -110,7 +110,7 @@ class TransportSecurityState :
   Delegate* delegate_;
 
   static std::string CanonicaliseHost(const std::string& host);
-  static bool isPreloadedSTS(const std::string& canonicalised_host,
+  static bool IsPreloadedSTS(const std::string& canonicalised_host,
                              bool* out_include_subdomains);
 
   DISALLOW_COPY_AND_ASSIGN(TransportSecurityState);
diff --git a/net/base/transport_security_state_unittest.cc b/net/base/transport_security_state_unittest.cc
index 457df90..9f30783 100644
--- a/net/base/transport_security_state_unittest.cc
+++ b/net/base/transport_security_state_unittest.cc
@@ -305,13 +305,13 @@ TEST_F(TransportSecurityStateTest, IsPreloaded) {
       TransportSecurityState::CanonicaliseHost("aypal.com");
 
   bool b;
-  EXPECT_FALSE(TransportSecurityState::isPreloadedSTS(paypal, &b));
-  EXPECT_TRUE(TransportSecurityState::isPreloadedSTS(www_paypal, &b));
+  EXPECT_FALSE(TransportSecurityState::IsPreloadedSTS(paypal, &b));
+  EXPECT_TRUE(TransportSecurityState::IsPreloadedSTS(www_paypal, &b));
   EXPECT_FALSE(b);
-  EXPECT_FALSE(TransportSecurityState::isPreloadedSTS(a_www_paypal, &b));
-  EXPECT_FALSE(TransportSecurityState::isPreloadedSTS(abc_paypal, &b));
-  EXPECT_FALSE(TransportSecurityState::isPreloadedSTS(example, &b));
-  EXPECT_FALSE(TransportSecurityState::isPreloadedSTS(aypal, &b));
+  EXPECT_FALSE(TransportSecurityState::IsPreloadedSTS(a_www_paypal, &b));
+  EXPECT_FALSE(TransportSecurityState::IsPreloadedSTS(abc_paypal, &b));
+  EXPECT_FALSE(TransportSecurityState::IsPreloadedSTS(example, &b));
+  EXPECT_FALSE(TransportSecurityState::IsPreloadedSTS(aypal, &b));
 }
 
 TEST_F(TransportSecurityStateTest, Preloaded) {