[BrowserSwitcher] Fix uppercase/lowercase for rules with a '/'

url_list rules with a '/' in them are now treated more similar to how
the LBS extension treated them. There is some pre-processing on the
rules, which parses the URL and converts the scheme + hostname to
lowercase.

Bug: 943497
Change-Id: I07814493213e7af30a237aac6791817167840461
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1531578Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644415}

chrome/browser/browser_switcher/browser_switcher_prefs.cc

@@ -9,6 +9,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
+#include "chrome/browser/browser_switcher/browser_switcher_sitelist.h"
 #include "chrome/browser/policy/profile_policy_connector.h"
 #include "chrome/browser/policy/profile_policy_connector_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -219,7 +220,9 @@ void BrowserSwitcherPrefs::UrlListChanged() {
 
   bool has_wildcard = false;
   for (const auto& url : *prefs_->GetList(prefs::kUrlList)) {
-    rules_.sitelist.push_back(url.GetString());
+    std::string canonical = url.GetString();
+    CanonicalizeRule(&canonical);
+    rules_.sitelist.push_back(std::move(canonical));
     if (url.GetString() == "*")
       has_wildcard = true;
   }
@@ -240,7 +243,9 @@ void BrowserSwitcherPrefs::GreylistChanged() {
 
   bool has_wildcard = false;
   for (const auto& url : *prefs_->GetList(prefs::kUrlGreylist)) {
-    rules_.greylist.push_back(url.GetString());
+    std::string canonical = url.GetString();
+    CanonicalizeRule(&canonical);
+    rules_.greylist.push_back(std::move(canonical));
     if (url.GetString() == "*")
       has_wildcard = true;
   }

chrome/browser/browser_switcher/browser_switcher_sitelist.cc

@@ -4,12 +4,15 @@
 
 #include "chrome/browser/browser_switcher/browser_switcher_sitelist.h"
 
+#include <string.h>
+
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/strings/strcat.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
 #include "chrome/browser/browser_switcher/browser_switcher_prefs.h"
@@ -60,7 +63,8 @@ bool UrlMatchesPattern(const NoCopyUrl& url, base::StringPiece pattern) {
     return true;
   }
   if (pattern.find('/') != base::StringPiece::npos) {
-    // Check prefix using the normalized URL, case sensitive.
+    // Check prefix using the normalized URL. Case sensitive, but with
+    // case-insensitive scheme/hostname.
     size_t pos = url.spec.find(pattern);
     if (pos == base::StringPiece::npos)
       return false;
@@ -99,6 +103,63 @@ bool StringSizeCompare(const base::StringPiece& a, const base::StringPiece& b) {
 
 }  // namespace
 
+void CanonicalizeRule(std::string* pattern) {
+  if (*pattern == "*" || pattern->find("/") == std::string::npos) {
+    // No "/" in the string. It's a hostnmae or wildcard, convert to lowercase.
+    *pattern = base::ToLowerASCII(*pattern);
+    return;
+  }
+
+  // The string has a "/" in it. It could be:
+  // - "//example.com/abc", convert hostname to lowercase
+  // - "example.com/abc", treat same as "//example.com/abc"
+  // - "http://example.com/abc", convert hostname and scheme to lowercase
+  // - "/abc", keep capitalization
+
+  const char* prefix = "";
+  base::StringPiece pattern_strpiece(*pattern);
+  if (IsInverted(*pattern)) {
+    prefix = "!";
+    *pattern = pattern->substr(1);
+  }
+
+  if (base::StartsWith(*pattern, "/", base::CompareCase::SENSITIVE) &&
+      !base::StartsWith(*pattern, "//", base::CompareCase::SENSITIVE)) {
+    // Rule starts with a single slash, e.g. "/abc". Don't change case.
+    pattern->insert(0, prefix);
+    return;
+  }
+
+  if (pattern->find("/") != 0 && pattern->find("://") == std::string::npos) {
+    // Transform "example.com/abc" => "//example.com/abc".
+    pattern->insert(0, "//");
+  }
+
+  // For patterns that include a "/": parse the URL to get the proper
+  // capitalization (for scheme/hostname).
+  //
+  // To properly parse URLs with no scheme, we need a valid base URL. We use
+  // "ftp://XXX/", which is a valid URL with an unsupported scheme. That way,
+  // parsing still succeeds, and we can easily know when the scheme isn't part
+  // of the original pattern (and omit it from the output).
+  const char* placeholder_scheme = "ftp:";
+  std::string placeholder = base::StrCat({placeholder_scheme, "//XXX/"});
+  GURL base_url(placeholder);
+
+  GURL relative_url = base_url.Resolve(*pattern);
+  base::StringPiece spec = relative_url.possibly_invalid_spec();
+
+  // The parsed URL might start with "ftp://XXX/" or "ftp://". Remove that
+  // prefix.
+  if (base::StartsWith(spec, placeholder, base::CompareCase::INSENSITIVE_ASCII))
+    spec = spec.substr(placeholder.size());
+  if (base::StartsWith(spec, placeholder_scheme,
+                       base::CompareCase::INSENSITIVE_ASCII))
+    spec = spec.substr(strlen(placeholder_scheme));
+
+  *pattern = base::StrCat({prefix, spec.as_string()});
+}
+
 BrowserSwitcherSitelist::~BrowserSwitcherSitelist() = default;
 
 BrowserSwitcherSitelistImpl::BrowserSwitcherSitelistImpl(

chrome/browser/browser_switcher/browser_switcher_sitelist.h

@@ -16,6 +16,10 @@ namespace browser_switcher {
 class BrowserSwitcherPrefs;
 class ParsedXml;
 
+// Pre-processes the URL rule and modifies it in-place if needed, to avoid
+// having to convert uppercase/lowercase for every rule at every navigation.
+void CanonicalizeRule(std::string* rule);
+
 // Interface that decides whether a navigation should trigger a browser
 // switch.
 class BrowserSwitcherSitelist {

chrome/browser/browser_switcher/browser_switcher_sitelist_unittest.cc

@@ -66,6 +66,44 @@ class BrowserSwitcherSitelistTest : public testing::Test {
   std::unique_ptr<BrowserSwitcherSitelist> sitelist_;
 };
 
+TEST_F(BrowserSwitcherSitelistTest, CanonicalizeRule) {
+  std::string rule = "Example.Com";
+  CanonicalizeRule(&rule);
+  EXPECT_EQ("example.com", rule);
+
+  rule = "Example.Com/";
+  CanonicalizeRule(&rule);
+  EXPECT_EQ("//example.com/", rule);
+
+  rule = "!Example.Com/Abc";
+  CanonicalizeRule(&rule);
+  EXPECT_EQ("!//example.com/Abc", rule);
+
+  rule = "/Example.Com";
+  CanonicalizeRule(&rule);
+  EXPECT_EQ("/Example.Com", rule);
+
+  rule = "//Example.Com";
+  CanonicalizeRule(&rule);
+  EXPECT_EQ("//example.com/", rule);
+
+  rule = "!//Example.Com";
+  CanonicalizeRule(&rule);
+  EXPECT_EQ("!//example.com/", rule);
+
+  rule = "HTTP://EXAMPLE.COM";
+  CanonicalizeRule(&rule);
+  EXPECT_EQ("http://example.com/", rule);
+
+  rule = "HTTP://EXAMPLE.COM/ABC";
+  CanonicalizeRule(&rule);
+  EXPECT_EQ("http://example.com/ABC", rule);
+
+  rule = "User@Example.Com:8080/Test";
+  CanonicalizeRule(&rule);
+  EXPECT_EQ("//User@example.com:8080/Test", rule);
+}
+
 TEST_F(BrowserSwitcherSitelistTest, ShouldRedirectWildcard) {
   // A "*" by itself means everything matches.
   Initialize({"*"}, {});