The search terms are escaped using + or %20 for space depending on whether replacement is in query part of the URL or not.

Removed duplicate EscapeQueryParamValue functions without |use_plus| argument.

BUG=24571
TEST=Verify that space is escaped as stated in description; see bug description for example with search on Wikipedia.
Review URL: http://codereview.chromium.org/543077

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@36398 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 53 insertions, 24 deletions

diff --git a/chrome/browser/dom_ui/history_ui.cc b/chrome/browser/dom_ui/history_ui.cc
index 0d78ddd..df4ef27 100644
--- a/chrome/browser/dom_ui/history_ui.cc
+++ b/chrome/browser/dom_ui/history_ui.cc
@@ -377,7 +377,7 @@ HistoryUI::HistoryUI(TabContents* contents) : DOMUI(contents) {
 // static
 const GURL HistoryUI::GetHistoryURLWithSearchText(const std::wstring& text) {
   return GURL(std::string(chrome::kChromeUIHistoryURL) + "#q=" +
-              EscapeQueryParamValue(WideToUTF8(text)));
+              EscapeQueryParamValue(WideToUTF8(text), true));
 }
 
 // static
diff --git a/chrome/browser/extensions/extension_updater.cc b/chrome/browser/extensions/extension_updater.cc
index 9228905..da55735 100644
--- a/chrome/browser/extensions/extension_updater.cc
+++ b/chrome/browser/extensions/extension_updater.cc
@@ -472,7 +472,7 @@ void AppendExtensionInfo(std::string* str, const Extension& extension) {
     parts.push_back("v=" + version->GetString());
     parts.push_back("uc");
 
-    str->append("x=" + EscapeQueryParamValue(JoinString(parts, '&')));
+    str->append("x=" + EscapeQueryParamValue(JoinString(parts, '&'), true));
 }
 
 // Creates a blacklist update url.
@@ -480,7 +480,7 @@ GURL ExtensionUpdater::GetBlacklistUpdateUrl(const std::wstring& version) {
   std::string blklist_info = StringPrintf("id=%s&v=%s&uc", kBlacklistAppID,
       WideToASCII(version).c_str());
   return GURL(StringPrintf("%s?x=%s", kBlacklistUpdateUrl,
-                           EscapeQueryParamValue(blklist_info).c_str()));
+                           EscapeQueryParamValue(blklist_info, true).c_str()));
 }
 
 void ExtensionUpdater::ScheduleNextCheck(const TimeDelta& target_delay) {
@@ -575,7 +575,7 @@ void ExtensionUpdater::CheckNow() {
       std::string uid = uid_provider_->GetUidString();
       if (uid.length() > 0 && uid.length() <= UidProvider::maxUidLength) {
         full_url_string.append("&" + std::string(ExtensionUpdater::kUidKey) +
-                               "=" + EscapeQueryParamValue(uid));
+                               "=" + EscapeQueryParamValue(uid, true));
       }
     }
 
diff --git a/chrome/browser/safe_browsing/protocol_manager.cc b/chrome/browser/safe_browsing/protocol_manager.cc
index 42c6158..d47d4d1 100644
--- a/chrome/browser/safe_browsing/protocol_manager.cc
+++ b/chrome/browser/safe_browsing/protocol_manager.cc
@@ -619,9 +619,9 @@ void SafeBrowsingProtocolManager::ReportMalware(const GURL& malware_url,
                                                 const GURL& referrer_url) {
   std::string report_str = StringPrintf(
       kSbMalwareReportUrl,
-      EscapeQueryParamValue(malware_url.spec()).c_str(),
-      EscapeQueryParamValue(page_url.spec()).c_str(),
-      EscapeQueryParamValue(referrer_url.spec()).c_str(),
+      EscapeQueryParamValue(malware_url.spec(), true).c_str(),
+      EscapeQueryParamValue(page_url.spec(), true).c_str(),
+      EscapeQueryParamValue(referrer_url.spec(), true).c_str(),
       client_name_.c_str(),
       version_.c_str());
   GURL report_url(report_str);
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
index 04aa1b1..114473c 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
@@ -375,7 +375,7 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) {
     // We're going to take the user to Google's SafeBrowsing diagnostic page.
     std::string diagnostic =
         StringPrintf(kSbDiagnosticUrl,
-                     EscapeQueryParamValue(bad_url_spec).c_str());
+                     EscapeQueryParamValue(bad_url_spec, true).c_str());
     GURL diagnostic_url(diagnostic);
     diagnostic_url = google_util::AppendGoogleLocaleParam(diagnostic_url);
     DCHECK(unsafe_resources_[element_index].threat_type ==
diff --git a/chrome/browser/safe_browsing/safe_browsing_util.cc b/chrome/browser/safe_browsing/safe_browsing_util.cc
index f036646..2d6cd9c 100644
--- a/chrome/browser/safe_browsing/safe_browsing_util.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_util.cc
@@ -596,8 +596,8 @@ GURL GeneratePhishingReportUrl(const std::string& report_page,
   if (!lang)
     lang = "en";  // fallback
   const std::string continue_esc =
-      EscapeQueryParamValue(StringPrintf(kContinueUrlFormat, lang));
-  const std::string current_esc = EscapeQueryParamValue(url_to_report);
+      EscapeQueryParamValue(StringPrintf(kContinueUrlFormat, lang), true);
+  const std::string current_esc = EscapeQueryParamValue(url_to_report, true);
 
 #if defined(OS_WIN)
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
diff --git a/chrome/browser/search_engines/template_url.cc b/chrome/browser/search_engines/template_url.cc
index 82bec3b..26da6fa 100644
--- a/chrome/browser/search_engines/template_url.cc
+++ b/chrome/browser/search_engines/template_url.cc
@@ -246,6 +246,19 @@ std::wstring TemplateURLRef::ReplaceSearchTerms(
   if (replacements_.empty())
     return parsed_url_;
 
+  // Determine if the search terms are in the query or before. We're escaping
+  // space as '+' in the former case and as '%20' in the latter case.
+  bool use_plus = true;
+  for (Replacements::iterator i = replacements_.begin();
+       i != replacements_.end(); ++i) {
+    if (i->type == SEARCH_TERMS) {
+      std::wstring::size_type query_start = parsed_url_.find(L'?');
+      use_plus = query_start != std::wstring::npos &&
+          (static_cast<std::wstring::size_type>(i->index) > query_start);
+      break;
+    }
+  }
+
   // Encode the search terms so that we know the encoding.
   const std::vector<std::string>& encodings = host.input_encodings();
   string16 encoded_terms;
@@ -253,21 +266,23 @@ std::wstring TemplateURLRef::ReplaceSearchTerms(
   std::wstring input_encoding;
   for (size_t i = 0; i < encodings.size(); ++i) {
     if (EscapeQueryParamValue(WideToUTF16Hack(terms),
-                              encodings[i].c_str(), &encoded_terms)) {
+                              encodings[i].c_str(), use_plus, &encoded_terms)) {
       if (!original_query_for_suggestion.empty()) {
         EscapeQueryParamValue(WideToUTF16Hack(original_query_for_suggestion),
-                              encodings[i].c_str(), &encoded_original_query);
+                              encodings[i].c_str(),
+                              true,
+                              &encoded_original_query);
       }
       input_encoding = ASCIIToWide(encodings[i]);
       break;
     }
   }
   if (input_encoding.empty()) {
-    encoded_terms = WideToUTF16Hack(EscapeQueryParamValueUTF8(terms));
+    encoded_terms = WideToUTF16Hack(EscapeQueryParamValueUTF8(terms, use_plus));
     if (!original_query_for_suggestion.empty()) {
       encoded_original_query =
           WideToUTF16Hack(
-              EscapeQueryParamValueUTF8(original_query_for_suggestion));
+              EscapeQueryParamValueUTF8(original_query_for_suggestion, true));
     }
     input_encoding = L"UTF-8";
   }
diff --git a/chrome/browser/search_engines/template_url_unittest.cc b/chrome/browser/search_engines/template_url_unittest.cc
index f15d6fc3..261adf0 100644
--- a/chrome/browser/search_engines/template_url_unittest.cc
+++ b/chrome/browser/search_engines/template_url_unittest.cc
@@ -38,15 +38,29 @@ TEST_F(TemplateURLTest, TestValidWithComplete) {
 }
 
 TEST_F(TemplateURLTest, URLRefTestSearchTerms) {
-  TemplateURL t_url;
-  TemplateURLRef ref(L"http://foo{searchTerms}", 0, 0);
-  ASSERT_TRUE(ref.IsValid());
-
-  ASSERT_TRUE(ref.SupportsReplacement());
-  GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(t_url, L"search",
-      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
-  ASSERT_TRUE(result.is_valid());
-  ASSERT_EQ("http://foosearch/", result.spec());
+  struct SearchTermsCase {
+    const wchar_t* url;
+    const wchar_t* terms;
+    const char* output;
+  } search_term_cases[] = {
+    { L"http://foo{searchTerms}", L"sea rch", "http://foosea%20rch/" },
+    { L"http://foo{searchTerms}?boo=abc", L"sea rch",
+      "http://foosea%20rch/?boo=abc" },
+    { L"http://foo/?boo={searchTerms}", L"sea rch",
+      "http://foo/?boo=sea+rch" }
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(search_term_cases); ++i) {
+    const SearchTermsCase& value = search_term_cases[i];
+    TemplateURL t_url;
+    TemplateURLRef ref(value.url, 0, 0);
+    ASSERT_TRUE(ref.IsValid());
+
+    ASSERT_TRUE(ref.SupportsReplacement());
+    GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(t_url, value.terms,
+        TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
+    ASSERT_TRUE(result.is_valid());
+    ASSERT_EQ(value.output, result.spec());
+  }
 }
 
 TEST_F(TemplateURLTest, URLRefTestCount) {
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 92b42a7..649542f 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -2935,7 +2935,7 @@ GURL RenderView::GetAlternateErrorPageURL(const GURL& failed_url,
   // Construct the query params to send to link doctor.
   std::string params(alternate_error_page_url_.query());
   params.append("&url=");
-  params.append(EscapeQueryParamValue(url_to_send.spec()));
+  params.append(EscapeQueryParamValue(url_to_send.spec(), true));
   params.append("&sourceid=chrome");
   params.append("&error=");
   switch (error_type) {