Omnibox - Do Not Allow HTTP/HTTPS Equivalence if User Explicitly Entered A Scheme

components/omnibox/autocomplete_match.cc

Index: components/omnibox/autocomplete_match.cc
diff --git a/components/omnibox/autocomplete_match.cc b/components/omnibox/autocomplete_match.cc
index 8bd091745b8ebf50fe4f9c7e7f8575776c44b157..f991830ae65b68bf603f302edb2d2e83778e2df8 100644
--- a/components/omnibox/autocomplete_match.cc
+++ b/components/omnibox/autocomplete_match.cc
@@ -19,6 +19,7 @@
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_service.h"
 #include "grit/components_scaled_resources.h"
+#include "net/base/net_util.h"
 
 namespace {
 
@@ -28,6 +29,43 @@ bool IsTrivialClassification(const ACMatchClassifications& classifications) {
        (classifications.back().style == ACMatchClassification::NONE));
 }
 
+// Returns true if one of the |terms_prefixed_by_http_or_https| matches the
+// beginning of the URL (sans scheme).  (Recall that
+// |terms_prefixed_by_http_or_https|, for the input "http://a b" will be
+// ["a"].)  This suggests that the user wants a particular URL with a scheme
+// in mind, hence the caller should not consider another URL like this one
+// but with a different scheme to be a duplicate.  |languages| is used to
+// format punycoded URLs to decide if they match.
+bool WordMatchesURLContent(
+    const std::vector<std::pair<std::string, base::string16> >&
+        terms_prefixed_by_http_or_https,
+    const std::string& languages,
+    const GURL& url) {
+  size_t prefix_length =
+      url.scheme().length() + strlen(url::kStandardSchemeSeparator);
+  DCHECK_GE(url.spec().length(), prefix_length);
+  const std::string& url_spec_without_scheme = url.spec().substr(prefix_length);
+  // In addition to checking the URL spec, check the formatted URL in order to
+  // detect a prefix match against a punycode-encoded hostname.

[Peter Kasting, 2015/06/29 05:04:59]

How come you check both?  Shouldn't the check against the formatted URL catch
everything except the user explicitly typing punycode (which is so unlikely I'm
OK with not supporting it)?  If so it seems like doing just that check would be
clearer, and you wouldn't need to keep a vector of pairs.

[Mark P, 2015/06/30 04:23:17]

I thought I had a reason at the time but I don't recall it now.  Perhaps I was
thinking
that FormatUrl() was reasonably likely to fail?  (This notion is wrong.)  Now
fixed
to only check the formatted url.

+  const base::string16& formatted_url = net::FormatUrl(
+      url, languages, net::kFormatUrlOmitNothing, net::UnescapeRule::NORMAL,
+      NULL, NULL, &prefix_length);
+  base::string16 formatted_url_without_scheme;
+  if (prefix_length != base::string16::npos)
+    formatted_url_without_scheme = formatted_url.substr(prefix_length);
+  for (const auto& term_pair : terms_prefixed_by_http_or_https) {
+    // At the moment we do not support case-insensitive prefix matching
+    // for international (punycode) domain names.
+    if (base::StartsWith(url_spec_without_scheme, term_pair.first,
+                         base::CompareCase::INSENSITIVE_ASCII) ||
+        ((prefix_length != base::string16::npos) &&
+         base::StartsWith(formatted_url_without_scheme, term_pair.second,
+                          base::CompareCase::SENSITIVE)))
+      return true;
+  }
+  return false;
+}
+
 }  // namespace
 
 // AutocompleteMatch ----------------------------------------------------------
@@ -372,6 +410,8 @@ TemplateURL* AutocompleteMatch::GetTemplateURLWithKeyword(
 // static
 GURL AutocompleteMatch::GURLToStrippedGURL(
     const GURL& url,
+    const AutocompleteInput& input,
+    const std::string& languages,
     TemplateURLService* template_url_service,
     const base::string16& keyword) {
   if (!url.is_valid())
@@ -417,8 +457,27 @@ GURL AutocompleteMatch::GURLToStrippedGURL(
     needs_replacement = true;
   }
 
-  // Replace https protocol with http protocol.
-  if (stripped_destination_url.SchemeIs(url::kHttpsScheme)) {
+  // Remove any trailing slash (if it's not a lone slash), or add a slash (to
+  // make a lone slash) if the path is empty.  (We can't unconditionally
+  // remove even lone slashes because for some schemes the path must consist
+  // of at least a slash.)
+  const std::string& path = stripped_destination_url.path();
+  if ((path.length() > 1) && (path[path.length() - 1] == '/')) {
+    replacements.SetPathStr(
+        base::StringPiece(path).substr(0, path.length() - 1));
+    needs_replacement = true;
+  } else if (path.empty()) {
+    static const char slash[] = "/";
+    replacements.SetPathStr(base::StringPiece(slash));
+    needs_replacement = true;
+  }
+
+  // Replace https protocol with http, as long as the user didn't explicitly
+  // specify one of the two.
+  if (stripped_destination_url.SchemeIs(url::kHttpsScheme) &&
+      (input.terms_prefixed_by_http_or_https().empty() ||
+       !WordMatchesURLContent(
+           input.terms_prefixed_by_http_or_https(), languages, url))) {
     replacements.SetScheme(url::kHttpScheme,
                            url::Component(0, strlen(url::kHttpScheme)));
     needs_replacement = true;
@@ -431,19 +490,23 @@ GURL AutocompleteMatch::GURLToStrippedGURL(
 }
 
 void AutocompleteMatch::ComputeStrippedDestinationURL(
+    const AutocompleteInput& input,
+    const std::string& languages,
     TemplateURLService* template_url_service) {
-  stripped_destination_url =
-      GURLToStrippedGURL(destination_url, template_url_service, keyword);
+  stripped_destination_url = GURLToStrippedGURL(
+      destination_url, input, languages, template_url_service, keyword);
 }
 
 void AutocompleteMatch::EnsureUWYTIsAllowedToBeDefault(
-    const GURL& canonical_input_url,
+    const AutocompleteInput& input,
+    const std::string& languages,
     TemplateURLService* template_url_service) {
   if (!allowed_to_be_default_match) {
     const GURL& stripped_canonical_input_url =
         AutocompleteMatch::GURLToStrippedGURL(
-            canonical_input_url, template_url_service, base::string16());
-    ComputeStrippedDestinationURL(template_url_service);
+            input.canonicalized_url(), input, languages, template_url_service,
+            base::string16());
+    ComputeStrippedDestinationURL(input, languages, template_url_service);
     allowed_to_be_default_match =
         stripped_canonical_input_url == stripped_destination_url;
   }