Implement PasswordStore::GetLoginsForSameOrganizationName.

components/password_manager/core/browser/psl_matching_helper_unittest.cc

Index: components/password_manager/core/browser/psl_matching_helper_unittest.cc
diff --git a/components/password_manager/core/browser/psl_matching_helper_unittest.cc b/components/password_manager/core/browser/psl_matching_helper_unittest.cc
index 66c798df366b9877620e8a973b5f2c3981952550..01df9ec4963f7370aae5fd53d78735759c842f62 100644
--- a/components/password_manager/core/browser/psl_matching_helper_unittest.cc
+++ b/components/password_manager/core/browser/psl_matching_helper_unittest.cc
@@ -5,10 +5,14 @@
 #include "components/password_manager/core/browser/psl_matching_helper.h"
 
 #include <stddef.h>
+#include <cctype>
 
 #include "base/macros.h"
+#include "base/stl_util.h"
+#include "base/strings/stringprintf.h"
 #include "components/autofill/core/common/password_form.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
 
 namespace password_manager {
 
@@ -387,6 +391,96 @@ TEST(PSLMatchingUtilsTest, IsFederatedPSLMatch) {
   }
 }
 
+TEST(PSLMatchingUtilsTest, GetOrganizationIdentifyingName) {
+  const struct {
+    const char* url;
+    const char* expected_organization_name;
+    bool expect_invalid_url;

[vasilii, 2017/05/24 17:48:38]

Why are you testing GURL::is_valid() here?

[engedy, 2017/05/24 19:27:12]

The intention was to convince readers that some of the below URLs are indeed
valid, but on second thought, this is already obvious from the fact that their
org-name is non-empty. Removed.

+  } kTestCases[] = {
+      {"http://example.com/login", "example"},
+      {"https://example.com", "example"},
+      {"ftp://example.com/ftp_realm", "example"},
+
+      {"http://foo.bar.example.com", "example"},
+      {"http://example.co.uk", "example"},
+      {"http://bar.example.appspot.com", "example"},
+      {"http://foo.bar", "foo"},
+      {"https://user:pass@www.example.com:80/path?query#ref", "example"},
+
+      {"http://www.foo+bar.com", "foo+bar"},
+      {"http://www.foo-bar.com", "foo-bar"},
+      {"https://foo_bar.com", "foo_bar"},
+      {"http://www.foo%2Bbar.com", "foo+bar"},
+      {"http://www.foo%2Dbar.com", "foo-bar"},
+      {"https://foo%5Fbar.com", "foo_bar"},
+      {"http://www.foo%2Ebar.com", "bar"},
+      {"https://www.sz\xc3\xb3t\xc3\xa1r.appspot.com", "xn--sztr-7na0i"},

[vasilii, 2017/05/24 17:48:38]

What is happening here?

[engedy, 2017/05/24 19:27:12]

Ah yes, added a comment to clarify. I had been reading `url_canon_unittest.cc`
before writing this, so the ugliness of this line didn't really register
anymore. :-)

+
+      {"http://www.foo!bar.com", "foo%21bar"},
+      {"http://www.foo%21bar.com", "foo%21bar"},
+      {"http://www.foo$bar.com", "foo%24bar"},
+      {"http://www.foo&bar.com", "foo%26bar"},
+      {"http://www.foo\'bar.com", "foo%27bar"},
+      {"http://www.foo(bar.com", "foo%28bar"},
+      {"http://www.foo)bar.com", "foo%29bar"},
+      {"http://www.foo*bar.com", "foo%2Abar"},
+      {"http://www.foo,bar.com", "foo%2Cbar"},
+      {"http://www.foo=bar.com", "foo%3Dbar"},
+
+      // URLs without host portions, hosts without registry controlled domains
+      // should, or hosts consisting of a registry yield the empty string.
+      {"http://localhost", ""},
+      {"http://co.uk", ""},
+      {"http://google", ""},
+      {"http://127.0.0.1", ""},
+      {"file:///usr/bin/stuff", ""},
+      {"federation://example.com/google.com", ""},
+      {"android://hash@com.example/", ""},
+      {"http://[1080:0:0:0:8:800:200C:417A]/", ""},
+      {"http://[3ffe:2a00:100:7031::1]", ""},
+      {"http://[::192.9.5.5]/", ""},
+
+      // Invalid URLs should yield the empty string.
+      {"", "", true},
+      {"http://", "", true},
+      {"bad url", "", true},
+      {"http://www.example.com/%00", "", true},
+      {"http://www.foo;bar.com", "", true},
+      {"http://www.foo~bar.com", "", true},
+  };
+
+  for (const auto& test_case : kTestCases) {
+    SCOPED_TRACE(test_case.url);
+    GURL url(test_case.url);
+    EXPECT_EQ(test_case.expect_invalid_url, !url.is_valid());
+    EXPECT_EQ(test_case.expected_organization_name,
+              GetOrganizationIdentifyingName(url));
+  }
+}
+
+// Apart from alphanumeric characters and '.', only |kExpectedUnescapedChars|
+// are expected to appear in a domain portion of a valid canonicalized URL
+// without percent-encoding.
+TEST(PSLMatchingUtilsTest,
+     GetOrganizationIdentifyingName_UnescapedSpecialChars) {
+  char kExpectedUnescapedChars[] = {'+', '-', '_'};
+  for (int chr = 0; chr <= 127; ++chr) {
+    if (chr == '.' || isalnum(chr))

[vasilii, 2017/05/24 17:48:39]

Wouldn't the test pass without this?

[engedy, 2017/05/24 19:27:12]

How about this now?

+      continue;
+
+    std::string non_canonical_spec =
+        base::StringPrintf("http://foo%%%2Xbar.com/", chr);
+    GURL url(non_canonical_spec);
+    if (!url.is_valid())
+      continue;
+
+    if (base::ContainsValue(kExpectedUnescapedChars, chr))
+      EXPECT_EQ(base::StringPrintf("http://foo%cbar.com/", chr), url.spec());
+    else
+      EXPECT_EQ(non_canonical_spec, url.spec());
+  }
+}
+
 }  // namespace
 
 }  // namespace password_manager