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

components/omnibox/autocomplete_match.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/omnibox/autocomplete_match.h"
 
 #include "base/i18n/time_formatting.h"
 #include "base/logging.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "components/omnibox/autocomplete_provider.h"
 #include "components/omnibox/suggestion_answer.h"
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_service.h"
 #include "grit/components_scaled_resources.h"
 
 namespace {
 
 bool IsTrivialClassification(const ACMatchClassifications& classifications) {
   return classifications.empty() ||
       ((classifications.size() == 1) &&
        (classifications.back().style == ACMatchClassification::NONE));
 }
 
+// Returns true if one of the |http_following_terms| matches the beginning of
+// the URL (sans scheme).  (Recall that |http_following_terms|, 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.
+bool WordMatchesURLContent(
+    const std::vector<base::string16>& http_following_terms,
+    const GURL& url) {
+  const base::string16& url_spec_without_scheme_as_utf16 =

[Peter Kasting, 2015/06/09 20:36:39]

If you follow my suggestion in autocomplete_input.cc of making
|http_following_terms| a std::string, this can get simpler by avoiding the
encoding conversion.

[Mark P, 2015/06/10 23:38:35]

Acknowledged, but not relevant given my earlier protest.

+      base::UTF8ToUTF16(url.spec().substr(
+          url.scheme().length() +
+          std::string(url::kStandardSchemeSeparator).length()));
+  for (auto it : http_following_terms) {

[Peter Kasting, 2015/06/09 20:36:39]

Nit: const auto&, to avoid a copy

[Mark P, 2015/06/10 23:38:35]

Done.

BTW, also renamed "it" to "term".

+    if (StartsWith(url_spec_without_scheme_as_utf16, it, false)) {

[Peter Kasting, 2015/06/09 20:36:39]

Nit: No {}

[Mark P, 2015/06/10 23:38:35]

Done.

+      return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace
 
 // AutocompleteMatch ----------------------------------------------------------
 
 // static
 const base::char16 AutocompleteMatch::kInvalidChars[] = {
   '\n', '\r', '\t',
   0x2028,  // Line separator
   0x2029,  // Paragraph separator
   0
@@ skipping 324 matching lines @@
     return NULL;
   TemplateURL* template_url = keyword.empty() ?
       NULL : template_url_service->GetTemplateURLForKeyword(keyword);
   return (template_url || host.empty()) ?
       template_url : template_url_service->GetTemplateURLForHost(host);
 }
 
 // static
 GURL AutocompleteMatch::GURLToStrippedGURL(
     const GURL& url,
+    const AutocompleteInput& input,
     TemplateURLService* template_url_service,
     const base::string16& keyword) {
   if (!url.is_valid())
     return url;
 
   GURL stripped_destination_url = url;
 
   // If the destination URL looks like it was generated from a TemplateURL,
   // remove all substitutions other than the search terms.  This allows us
   // to eliminate cases like past search URLs from history that differ only
@@ skipping 25 matching lines @@
 
   // Remove the www. prefix from the host.
   static const char prefix[] = "www.";
   static const size_t prefix_len = arraysize(prefix) - 1;
   std::string host = stripped_destination_url.host();
   if (host.compare(0, prefix_len, prefix) == 0) {
     replacements.SetHostStr(base::StringPiece(host).substr(prefix_len));
     needs_replacement = true;
   }
 
-  // Replace https protocol with http protocol.
-  if (stripped_destination_url.SchemeIs(url::kHttpsScheme)) {
+  // Replace https protocol with http, as long as the user didn't explicitly
+  // specify one of the two.
+  if (!WordMatchesURLContent(input.http_following_terms(), url) &&
+      stripped_destination_url.SchemeIs(url::kHttpsScheme)) {

[Peter Kasting, 2015/06/09 20:36:39]

Nit: For efficiency, reverse these subexpressions; the SchemeIs() check should
be cheaper than the other call.

I would probably also add an explicit "!input.http_following_terms().empty() &&"
before both of these, again, just for efficiency (avoids some string
constructions).

[Mark P, 2015/06/10 23:38:35]

Good idea.  Done.
This kind of thing makes sense to me.  Putting in where you suggested with a &&
is wrong however.  (Thank goodness for unit tests to notice this.)  Put in the
correct formulation.

     replacements.SetScheme(url::kHttpScheme,
                            url::Component(0, strlen(url::kHttpScheme)));
     needs_replacement = true;
   }
 
   if (needs_replacement)
     stripped_destination_url = stripped_destination_url.ReplaceComponents(
         replacements);
   return stripped_destination_url;
 }
 
 void AutocompleteMatch::ComputeStrippedDestinationURL(
+    const AutocompleteInput& input,
     TemplateURLService* template_url_service) {
-  stripped_destination_url =
-      GURLToStrippedGURL(destination_url, template_url_service, keyword);
+  stripped_destination_url = GURLToStrippedGURL(
+      destination_url, input, template_url_service, keyword);
 }
 
 void AutocompleteMatch::EnsureUWYTIsAllowedToBeDefault(
-    const GURL& canonical_input_url,
+    const AutocompleteInput& input,
     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, template_url_service,
+            base::string16());
+    ComputeStrippedDestinationURL(input, template_url_service);
     allowed_to_be_default_match =
         stripped_canonical_input_url == stripped_destination_url;
   }
 }
 
 void AutocompleteMatch::GetKeywordUIState(
     TemplateURLService* template_url_service,
     base::string16* keyword,
     bool* is_keyword_hint) const {
   *is_keyword_hint = associated_keyword.get() != NULL;
@@ skipping 102 matching lines @@
         << " is unsorted in relation to last offset of " << last_offset
         << ". Provider: " << provider_name << ".";
     DCHECK_LT(i->offset, text.length())
         << " Classification of [" << i->offset << "," << text.length()
         << "] is out of bounds for \"" << text << "\". Provider: "
         << provider_name << ".";
     last_offset = i->offset;
   }
 }
 #endif