Fix crash with webRequest.event.addListener when provided an invalid URL

chrome/browser/extensions/extension_webrequest_api.cc

 // Copyright (c) 2011 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 "chrome/browser/extensions/extension_webrequest_api.h"
 
 #include <algorithm>
 
 #include "base/json/json_writer.h"
 #include "base/metrics/histogram.h"
@@ skipping 130 matching lines @@
 
 // Internal representation of the webRequest.RequestFilter type, used to
 // filter what network events an extension cares about.
 struct ExtensionWebRequestEventRouter::RequestFilter {
   ExtensionExtent urls;
   std::vector<ResourceType::Type> types;
   int tab_id;
   int window_id;
 
   RequestFilter() : tab_id(-1), window_id(-1) {}
-  bool InitFromValue(const DictionaryValue& value);
+  // Returns false if there was an error initializing. If it is a user error,
+  // an error message is provided, otherwise the error is internal (and
+  // unexpected).
+  bool InitFromValue(const DictionaryValue& value, std::string* error);
 };
 
 // Internal representation of the extraInfoSpec parameter on webRequest events,
 // used to specify extra information to be included with network events.
 struct ExtensionWebRequestEventRouter::ExtraInfoSpec {
   enum Flags {
     REQUEST_LINE = 1<<0,
     REQUEST_HEADERS = 1<<1,
     STATUS_LINE = 1<<2,
     RESPONSE_HEADERS = 1<<3,
@@ skipping 38 matching lines @@
   // If non-empty, this contains the new URL that the request will redirect to.
   GURL* new_url;
 
   // Time the request was issued. Used for logging purposes.
   base::Time request_time;
 
   BlockedRequest() : num_handlers_blocking(0), callback(NULL), new_url(NULL) {}
 };
 
 bool ExtensionWebRequestEventRouter::RequestFilter::InitFromValue(
-    const DictionaryValue& value) {
+    const DictionaryValue& value, std::string* error) {
   for (DictionaryValue::key_iterator key = value.begin_keys();
        key != value.end_keys(); ++key) {
     if (*key == "urls") {
       ListValue* urls_value = NULL;
       if (!value.GetList("urls", &urls_value))
         return false;
       for (size_t i = 0; i < urls_value->GetSize(); ++i) {
         std::string url;
         URLPattern pattern(URLPattern::SCHEME_ALL);
         if (!urls_value->GetString(i, &url) ||
             pattern.Parse(url, URLPattern::PARSE_STRICT) !=
-                URLPattern::PARSE_SUCCESS)
+                URLPattern::PARSE_SUCCESS) {
+          *error = ExtensionErrorUtils::FormatErrorMessage(
+              keys::kInvalidRequestFilterUrl, url);
           return false;
+        }
         urls.AddPattern(pattern);
       }
     } else if (*key == "types") {
       ListValue* types_value = NULL;
       if (!value.GetList("types", &types_value))
         return false;
       for (size_t i = 0; i < types_value->GetSize(); ++i) {
         std::string type_str;
         ResourceType::Type type;
         if (!types_value->GetString(i, &type_str) ||
@@ skipping 425 matching lines @@
   size_t slash_sep = sub_event_name.find('/');
   std::string event_name = sub_event_name.substr(0, slash_sep);
 
   if (!IsWebRequestEvent(event_name))
     return;
 
   EventListener listener;
   listener.extension_id = extension_id;
   listener.sub_event_name = sub_event_name;
 
+  // It's possible for AddEventListener to fail asynchronously. In that case,
+  // the renderer to believe the listener exists, while the browser does not.

[battre, 2011/04/27 09:45:31]

nit: grammar in "the renderer to believe"

[Matt Perry, 2011/04/27 19:45:59]

Done.

+  // Ignore a RemoveEventListener in that case.
+  std::set<EventListener>::iterator found =
+      listeners_[profile_id][event_name].find(listener);
+  if (found == listeners_[profile_id][event_name].end())
+    return;
+
   CHECK_EQ(listeners_[profile_id][event_name].count(listener), 1u) <<
       "extension=" << extension_id << " event=" << event_name;
 
   // Unblock any request that this event listener may have been blocking.
-  std::set<EventListener>::iterator found =
-      listeners_[profile_id][event_name].find(listener);
   for (std::set<uint64>::iterator it = found->blocked_requests.begin();
        it != found->blocked_requests.end(); ++it) {
     DecrementBlockCount(*it, false, GURL());
   }
 
   listeners_[profile_id][event_name].erase(listener);
 }
 
 std::vector<const ExtensionWebRequestEventRouter::EventListener*>
 ExtensionWebRequestEventRouter::GetMatchingListeners(
@@ skipping 87 matching lines @@
     return;
   iter->second &= ~event_type;
 }
 
 bool WebRequestAddEventListener::RunImpl() {
   // Argument 0 is the callback, which we don't use here.
 
   ExtensionWebRequestEventRouter::RequestFilter filter;
   if (HasOptionalArgument(1)) {
     DictionaryValue* value = NULL;
+    error_.clear();
     EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(1, &value));
-    EXTENSION_FUNCTION_VALIDATE(filter.InitFromValue(*value));
+    // Failure + an empty error string means a fatal error.
+    EXTENSION_FUNCTION_VALIDATE(filter.InitFromValue(*value, &error_) ||
+                                !error_.empty());
+    if (!error_.empty())
+      return false;
   }
 
   int extra_info_spec = 0;
   if (HasOptionalArgument(2)) {
     ListValue* value = NULL;
     EXTENSION_FUNCTION_VALIDATE(args_->GetList(2, &value));
     EXTENSION_FUNCTION_VALIDATE(
         ExtensionWebRequestEventRouter::ExtraInfoSpec::InitFromValue(
             *value, &extra_info_spec));
   }
@@ skipping 51 matching lines @@
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       NewRunnableFunction(
           &EventHandledOnIOThread,
           profile()->GetRuntimeId(), extension_id(),
           event_name, sub_event_name, request_id, cancel, new_url));
 
   return true;
 }