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..0e1c0255078ca7ec61285a8d337adcf4f6cd6abc 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,37 @@ 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<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 base::string16& formatted_url = net::FormatUrl(
+      url, languages, net::kFormatUrlOmitNothing, net::UnescapeRule::NORMAL,
+      NULL, NULL, &prefix_length);
+  if (prefix_length == base::string16::npos)
+    return false;
+  const base::string16& formatted_url_without_scheme =
+      formatted_url.substr(prefix_length);
+  for (const auto& term : terms_prefixed_by_http_or_https) {
+    // At the moment we do not support case-insensitive prefix matching
+    // for international (punycode) domain names.

[Peter Kasting, 2015/06/30 06:50:30]

I bet I know why you did both comparisons.  You were trying to do a
case-insensitive comparison for inputs like "HTTP://FOO", but you didn't want to
do the case-folding with ICU for a true case-insensitive UTF-8 comparison, so
you backstopped it with a case-sensitive UTF-8 comparison since the existing
code could do that.

Now that you only have one comparison, the comment is kinda misleading.  We now
don't support case-insensitive comparisons at all.

It would be nice to do this right, by simply using ICU to lowercase both the URL
and the terms before doing the case-sensitive comparison.  But I suppose I could
probably implement that as a followup if it looks tricky.

[Mark P, 2015/06/30 17:29:20]

Ah, that sounds like what I was thinking.  Thanks for giving me credit. :-)

FWIW, we do get the case where the protocol is capitalized right (that goes
through
a case insensitive comparison in autocomplete_input.cc); it's on the main part
of the
URL that we get wrong.
Yeah, I agree the comment is misleading.  Removed.
One reason I left out case insensitivity for IDNs is that it's not clear to me
that it's
the right thing to do.  Are you sure that it's safe to transform one IDN domain
name
into another by lowercasing it?  Can they be owned by different entities?  Note
that
the lowercasing algorithm depends on the locale.

In any case, I'll leave whether to do this kind of thing up to you.  (I think
the answer is
that it's not worth the time to think about, let alone code, given that it only
affects deduping.)

+    if (base::StartsWith(formatted_url_without_scheme, term,
+                         base::CompareCase::SENSITIVE))
+      return true;
+  }
+  return false;
+}
+
 }  // namespace
 
 // AutocompleteMatch ----------------------------------------------------------
@@ -372,6 +404,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 +451,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 +484,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;
   }