PSLMatchingHelper::IsPublicSuffixDomainMatch checks URL validity

Before this fix the function was returning true when comparing two invalid URLs. After this fix, if one or both of the URLs are invalid the function will return false.

Checking incoming arguments of the function for validity is a good practice and may prevent future problems. (Currently the PSLMatchingHelper::IsPublicSuffixDomainMatch is never called with invalid arguments, but this can change in the future.) 

BUG=391529

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

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

AUTHORS

@@ -229,6 +229,7 @@ Leith Bade <leith@leithalweapon.geek.nz>
 Li Yin <li.yin@intel.com>
 Lorenzo Stoakes <lstoakes@gmail.com>
 Lu Guanqun <guanqun.lu@gmail.com>
+Lucie Brozkova <lucinka.brozkova@gmail.com>
 Luke Inman-Semerau <luke.semerau@gmail.com>
 Luke Zarko <lukezarko@gmail.com>
 Maarten Lankhorst <m.b.lankhorst@gmail.com>

components/password_manager/core/browser/psl_matching_helper.cc

@@ -46,6 +46,10 @@ bool PSLMatchingHelper::IsPublicSuffixDomainMatch(const std::string& url1,
                                                   const std::string& url2) {
   GURL gurl1(url1);
   GURL gurl2(url2);
+
+  if (!gurl1.is_valid() || !gurl2.is_valid())
+    return false;
+
   return gurl1.scheme() == gurl2.scheme() &&
          GetRegistryControlledDomain(gurl1) ==
              GetRegistryControlledDomain(gurl2) &&

components/password_manager/core/browser/psl_matching_helper.h

@@ -49,7 +49,8 @@ class PSLMatchingHelper {
       const std::string& registry_controlled_domain) const;
 
   // Two URLs are considered a Public Suffix Domain match if they have the same
-  // scheme, ports, and their registry controlled domains are equal.
+  // scheme, ports, and their registry controlled domains are equal. If one or
+  // both arguments do not describe valid URLs, returns false.
   static bool IsPublicSuffixDomainMatch(const std::string& url1,
                                         const std::string& url2);
 

components/password_manager/core/browser/psl_matching_helper_unittest.cc

@@ -20,12 +20,26 @@ TEST(PSLMatchingUtilsTest, IsPublicSuffixDomainMatch) {
   };
 
   TestPair pairs[] = {
-    { "http://facebook.com", "http://m.facebook.com", true },
-    { "http://www.facebook.com", "http://m.facebook.com", true },
-    { "http://www.example.com", "http://wwwexample.com", false },
-    { "http://www.example.com", "https://www.example.com", false },
-    { "http://www.example.com:123", "http://www.example.com", false },
-    { "http://www.example.org", "http://www.example.com", false },
+      {"http://facebook.com", "http://facebook.com", true},
+      {"http://facebook.com/path", "http://facebook.com/path", true},
+      {"http://facebook.com/path1", "http://facebook.com/path2", true},
+      {"http://facebook.com", "http://m.facebook.com", true},
+      {"http://www.facebook.com", "http://m.facebook.com", true},
+      {"http://facebook.com/path", "http://m.facebook.com/path", true},
+      {"http://facebook.com/path1", "http://m.facebook.com/path2", true},
+      {"http://example.com/has space", "http://example.com/has space", true},
+      {"http://www.example.com", "http://wwwexample.com", false},
+      {"http://www.example.com", "https://www.example.com", false},
+      {"http://www.example.com:123", "http://www.example.com", false},
+      {"http://www.example.org", "http://www.example.com", false},
+      // Invalid urls should not match anything.
+      {"http://", "http://", false},
+      {"", "", false},
+      {"bad url", "bad url", false},
+      {"http://www.example.com", "http://", false},
+      {"", "http://www.example.com", false},
+      {"http://www.example.com", "bad url", false},
+      {"http://www.example.com/%00", "http://www.example.com/%00", false},
   };
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(pairs); ++i) {