Common Name Mismatch Handler For WWW Subdomain Mismatch case

chrome/browser/ssl/ssl_error_classification.h

Index: chrome/browser/ssl/ssl_error_classification.h
diff --git a/chrome/browser/ssl/ssl_error_classification.h b/chrome/browser/ssl/ssl_error_classification.h
index 89869b9ad6dc0f9804841523e57874b81aec24da..4edbc3c15f8aaf554418b4b0df922f4a847da951 100644
--- a/chrome/browser/ssl/ssl_error_classification.h
+++ b/chrome/browser/ssl/ssl_error_classification.h
@@ -72,6 +72,19 @@ class SSLErrorClassification : public content::NotificationObserver {
   void RecordCaptivePortalUMAStatistics(bool overridable) const;
   base::TimeDelta TimePassedSinceExpiry() const;
 
+  // Returns true if the site's hostname differs from one of the DNS
+  // names in the certificate (CN or SANs) only by the presence or
+  // absence of the single-label prefix "www". E.g.: (The first domain
+  // is hostname and the second domain is a DNS name in the certificate)
+  //
+  //     www.example.com ~ example.com -> true
+  //     example.com ~ www.example.com -> true
+  //     www.food.example.com ~ example.com -> false
+  //     mail.example.com ~ example.com -> false
+  static bool GetWWWSubDomainMatch(const std::string& host_name,

[Ryan Sleevi, 2015/08/07 00:14:11]

This is more of a nit, but it seems like this would be better clumped with the
remaining static functions on line 52.

From an API contract level, it makes me somewhat sad to see all of these as
static public, since it feels like it's revealing a lot of the internal details
just for unit tests. Makes me wonder whether it'd be better with just friending
the test fixture in the private section. You shouldn't have to do that in this
CL, though, because we should update the other public static methods
simultaneously.

[Bhanu Dev, 2015/08/07 22:28:48]

Done.

+                                   const std::vector<std::string>& dns_names,
+                                   std::string* www_match_host_name);
+
  private:
   FRIEND_TEST_ALL_PREFIXES(SSLErrorClassificationTest, TestDateInvalidScore);
   FRIEND_TEST_ALL_PREFIXES(SSLErrorClassificationTest, TestNameMismatch);
@@ -83,14 +96,7 @@ class SSLErrorClassification : public content::NotificationObserver {
   // Returns true if the hostname has a known Top Level Domain.
   static bool IsHostNameKnownTLD(const std::string& host_name);
 
-  // Returns true if the site's hostname differs from one of the DNS
-  // names in the certificate (CN or SANs) only by the presence or
-  // absence of the single-label prefix "www". E.g.:
-  //
-  //     www.example.com ~ example.com -> true
-  //     example.com ~ www.example.com -> true
-  //     www.food.example.com ~ example.com -> false
-  //     mail.example.com ~ example.com -> false
+  // Returns true if GetWWWSubDomainMatch finds a www mismatch.

[Ryan Sleevi, 2015/08/07 00:14:11]

Why have this function at all then? It seems like it can just be
replaced/inlined, isn't needed as a separate function.

[Bhanu Dev, 2015/08/07 22:28:48]

This method gets the hostname,dns names and calls the static method
GetWWWSubDomainMatch. I thought that it might not be good to have it inline with
definition in header file, tests also use this.

[Ryan Sleevi, 2015/08/07 22:31:11]

I didn't mean inlined in the .h file, I meant just delete this function entirely
and inline what this function does in the one place it's called. That is, now
that you have the static function, it's somewhat questionable whether or not
there's a benefit from having a small function like this that calls said static
function.

   bool IsWWWSubDomainMatch() const;
 
   // Returns true if |child| is a subdomain of any of the |potential_parents|.