Refuse invalid SearchProvider and OSDD suggest URLs; etc.

Change SearchProvider::CreateSuggestFetcher:
Validate the TemplateURL[Ref] and suggest URL on search.
Return NULL early if invalid; this prevents crashes.

Change TemplateURLParsingContext::GetTemplateURL:
Validate the TemplateURLRefs from OSDDs import.
Return NULL early if invalid (don't import bad engines).

Return NULL early on TemplateURLParser::Parse error.

TODO(followup): Validate existing TemplateURLs in profile data?
TODO(followup): Support invalid GURLs that may be valid after replacements.

BUG=145852
TEST=Less crashes as reported in bug, import succeeds on [1], fails (no-op) on [2] and [3]...

[1] http://ready.to/search/en/?sna=MSW_TEST_SUGGEST&prf=http%3A%2F%2Fsearch.yahoo.com%2Fsearch%3Fei%3D%7BinputEncoding%7D%26amp%3Bfr%3Dcrmas%26amp%3Bp%3D&des=MSW_TEST_SUGGEST&opt=%26lt%3BUrl%20type%3D%26quot%3Bapplication%2Fx-suggestions%2Bjson%26quot%3B%20template%3D%26quot%3Bhttp%3A%2F%2Fff.search.yahoo.com%2Fgossip%3Foutput%3Dfxjson%26amp%3Bcommand%3D%7BsearchTerms%7D%26quot%3B%2F%26gt%3B&in=utf&ou=ono&mod=pn#

[2] http://ready.to/search/en/?sna=MSW_TEST_SUGGEST_BADSCHEME&prf=http%3A%2F%2Fsearch.yahoo.com%2Fsearch%3Fei%3D%7BinputEncoding%7D%26amp%3Bfr%3Dcrmas%26amp%3Bp%3D&des=MSW_TEST_SUGGEST_BADSCHEME&opt=%26lt%3BUrl%20type%3D%26quot%3Bapplication%2Fx-suggestions%2Bjson%26quot%3B%20template%3D%26quot%3Bbadscheme%3A%2F%2Fff.search.yahoo.com%2Fgossip%3Foutput%3Dfxjson%26amp%3Bcommand%3D%7BsearchTerms%7D%26quot%3B%2F%26gt%3B&in=utf&ou=ono&mod=pn#

[3] http://ready.to/search/en/?sna=MSW_TEST_SUGGEST_BADOPENBRACE&prf=http%3A%2F%2Fsearch.yahoo.com%2Fsearch%3Fei%3D%7BinputEncoding%7D%26amp%3Bfr%3Dcrmas%26amp%3Bp%3D&des=MSW_TEST_SUGGEST_BADOPENBRACE&opt=%26lt%3BUrl%20type%3D%26quot%3Bapplication%2Fx-suggestions%2Bjson%26quot%3B%20template%3D%26quot%3Bhttp%3A%2F%2Fff.search.yahoo.com%2Fgossip%3Foutput%3Dfxjson%26amp%3Bcommand%3D%7BsearchTerms%7D%26amp%3Bfoo%3D%7Bbad%26quot%3B%2F%26gt%3B&in=utf&ou=ono&mod=pn

Review URL: https://chromiumcodereview.appspot.com/10908044

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

1 files changed, 21 insertions, 17 deletions

diff --git a/chrome/browser/search_engines/template_url_parser.cc b/chrome/browser/search_engines/template_url_parser.cc
index f48eb39..5b5eb83 100644
--- a/chrome/browser/search_engines/template_url_parser.cc
+++ b/chrome/browser/search_engines/template_url_parser.cc
@@ -144,7 +144,7 @@ class TemplateURLParsingContext {
   // Returns a heap-allocated TemplateURL representing the result of parsing.
   // This will be NULL if parsing failed or if the results were invalid for some
   // reason (e.g. the resulting URL was not HTTP[S], a name wasn't supplied,
-  // etc.).
+  // a resulting TemplateURLRef was invalid, etc.).
   TemplateURL* GetTemplateURL(Profile* profile, bool show_in_default_list);
 
  private:
@@ -291,27 +291,30 @@ void TemplateURLParsingContext::CharactersImpl(void* ctx,
 TemplateURL* TemplateURLParsingContext::GetTemplateURL(
     Profile* profile,
     bool show_in_default_list) {
-  // Basic legality checks.
-  if (data_.short_name.empty() || !IsHTTPRef(data_.url()) ||
-      !IsHTTPRef(data_.suggestions_url))
+  // TODO(jcampan): Support engines that use POST; see http://crbug.com/18107
+  if (method_ == TemplateURLParsingContext::POST || data_.short_name.empty() ||
+      !IsHTTPRef(data_.url()) || !IsHTTPRef(data_.suggestions_url))
     return NULL;
+  if (suggestion_method_ == TemplateURLParsingContext::POST)
+    data_.suggestions_url.clear();
 
   // If the image was a data URL, use the favicon from the search URL instead.
-  // (see TODO inEndElementImpl()).
-  GURL url(data_.url());
+  // (see the TODO in EndElementImpl()).
+  GURL search_url(data_.url());
   if (derive_image_from_url_ && data_.favicon_url.is_empty())
-    data_.favicon_url = TemplateURL::GenerateFaviconURL(url);
+    data_.favicon_url = TemplateURL::GenerateFaviconURL(search_url);
+
+  data_.SetKeyword(TemplateURLService::GenerateKeyword(search_url));
+  data_.show_in_default_list = show_in_default_list;
 
-  // TODO(jcampan): http://b/issue?id=1196285 we do not support search engines
-  //                that use POST yet.
-  if (method_ == TemplateURLParsingContext::POST)
+  // Bail if the search URL is empty or if either TemplateURLRef is invalid.
+  scoped_ptr<TemplateURL> template_url(new TemplateURL(profile, data_));
+  if (template_url->url().empty() || !template_url->url_ref().IsValid() ||
+      (!template_url->suggestions_url().empty() &&
+       !template_url->suggestions_url_ref().IsValid()))
     return NULL;
-  if (suggestion_method_ == TemplateURLParsingContext::POST)
-    data_.suggestions_url.clear();
 
-  data_.SetKeyword(TemplateURLService::GenerateKeyword(url));
-  data_.show_in_default_list = show_in_default_list;
-  return new TemplateURL(profile, data_);
+  return template_url.release();
 }
 
 // static
@@ -485,8 +488,9 @@ TemplateURL* TemplateURLParser::Parse(
   sax_handler.startElement = &TemplateURLParsingContext::StartElementImpl;
   sax_handler.endElement = &TemplateURLParsingContext::EndElementImpl;
   sax_handler.characters = &TemplateURLParsingContext::CharactersImpl;
-  xmlSAXUserParseMemory(&sax_handler, &context, data, static_cast<int>(length));
+  int error = xmlSAXUserParseMemory(&sax_handler, &context, data,
+                                    static_cast<int>(length));
   xmlSubstituteEntitiesDefault(last_sub_entities_value);
 
-  return context.GetTemplateURL(profile, show_in_default_list);
+  return error ? NULL : context.GetTemplateURL(profile, show_in_default_list);
 }