Allow extensions on chrome:// URLs, when flag is set and permission is explicitly requested

chrome/common/extensions/extension.cc

Index: chrome/common/extensions/extension.cc
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index b1db79d4fc8fb004ad7a39a33db37d86271d732f..c4ac2d09ab87c3a55573be488dd35903ee441d25 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -590,26 +590,50 @@ bool Extension::ParsePermissions(const char* key,
       URLPattern pattern = URLPattern(kAllowedSchemes);
       URLPattern::ParseResult parse_result = pattern.Parse(permission_str);
       if (parse_result == URLPattern::PARSE_SUCCESS) {
-        if (!CanSpecifyHostPermission(pattern, *api_permissions)) {
-          *error = ErrorUtils::FormatErrorMessageUTF16(
-              errors::kInvalidPermissionScheme, permission_str);
-          return false;
-        }
-
         // The path component is not used for host permissions, so we force it
         // to match all paths.
         pattern.SetPath("/*");
-
+        int valid_schemes = pattern.valid_schemes();
         if (pattern.MatchesScheme(chrome::kFileScheme) &&
             !CanExecuteScriptEverywhere()) {
           wants_file_access_ = true;
-          if (!(creation_flags_ & ALLOW_FILE_ACCESS)) {
-            pattern.SetValidSchemes(
-                pattern.valid_schemes() & ~URLPattern::SCHEME_FILE);
-          }
+          if (!(creation_flags_ & ALLOW_FILE_ACCESS))
+            valid_schemes &= ~URLPattern::SCHEME_FILE;
+        }
+
+        if (pattern.scheme() != chrome::kChromeUIScheme &&
+            !CanExecuteScriptEverywhere()) {
+          // Keep chrome:// in allowed schemes only if it's explicitly requested
+          // or CanExecuteScriptEverywhere is true. If the
+          // extensions_on_chrome_urls flag is not set, CanSpecifyHostPermission
+          // will fail, so don't check the flag here.
+          valid_schemes &= ~URLPattern::SCHEME_CHROMEUI;
+        }
+        pattern.SetValidSchemes(valid_schemes);
+
+        if (!CanSpecifyHostPermission(pattern, *api_permissions)) {
+          // TODO(aboxhall): make a warning (see line 633)
+          *error = ErrorUtils::FormatErrorMessageUTF16(
+              errors::kInvalidPermissionScheme, permission_str);
+          return false;
         }
 
         host_permissions->AddPattern(pattern);
+
+        // We need to make sure all_urls matches chrome://favicon and
+        // (maybe) chrome://thumbnail, so add them back in to host_permissions
+        // separately.
+        if (pattern.match_all_urls()) {
+          host_permissions->AddPattern(
+              URLPattern(URLPattern::SCHEME_CHROMEUI,
+                         chrome::kChromeUIFaviconURL));
+          if (api_permissions->find(APIPermission::kExperimental) !=
+              api_permissions->end()) {
+            host_permissions->AddPattern(
+                URLPattern(URLPattern::SCHEME_CHROMEUI,
+                           chrome::kChromeUIThumbnailURL));
+          }
+        }
         continue;
       }
 
@@ -664,13 +688,6 @@ bool Extension::CanSilentlyIncreasePermissions() const {
 }
 
 bool Extension::HasHostPermission(const GURL& url) const {
-  if (url.SchemeIs(chrome::kChromeUIScheme) &&
-      url.host() != chrome::kChromeUIFaviconHost &&
-      url.host() != chrome::kChromeUIThumbnailHost &&
-      location() != Manifest::COMPONENT) {
-    return false;
-  }
-
   base::AutoLock auto_lock(runtime_data_lock_);
   return runtime_data_.GetActivePermissions()->
       HasExplicitAccessToOrigin(url);
@@ -803,9 +820,12 @@ bool Extension::CanExecuteScriptOnPage(const GURL& document_url,
     return false;
   }
 
-  if (document_url.SchemeIs(chrome::kChromeUIScheme) &&
-      !CanExecuteScriptEverywhere()) {
-    return false;
+  if (!CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kExtensionsOnChromeURLs)) {
+    if (document_url.SchemeIs(chrome::kChromeUIScheme) &&
+        !CanExecuteScriptEverywhere()) {
+      return false;

[dmazzoni, 2013/03/19 20:48:44]

Doesn't this allow any extension to run on chrome:// urls with the flag, even if
they don't request it? Or is that caught earlier?

[aboxhall, 2013/03/20 22:04:59]

It's caught later, specifically on either line 850 or 854/55. This line just
early-outs in the case of a chrome:// url.

+    }
   }
 
   if (top_frame_url.SchemeIs(extensions::kExtensionScheme) &&
@@ -2155,9 +2175,8 @@ bool Extension::LoadUserScriptHelper(const DictionaryValue* content_script,
       return false;
     }
 
-    URLPattern pattern(UserScript::kValidUserScriptSchemes);
-    if (CanExecuteScriptEverywhere())
-      pattern.SetValidSchemes(URLPattern::SCHEME_ALL);
+    URLPattern pattern(
+        UserScript::ValidUserScriptSchemes(CanExecuteScriptEverywhere()));
 
     URLPattern::ParseResult parse_result = pattern.Parse(match_str);
     if (parse_result != URLPattern::PARSE_SUCCESS) {
@@ -2169,6 +2188,13 @@ bool Extension::LoadUserScriptHelper(const DictionaryValue* content_script,
       return false;
     }
 
+    // TODO(aboxhall): check for webstore
+    if (!CanExecuteScriptEverywhere() &&
+        pattern.scheme() != chrome::kChromeUIScheme) {
+      pattern.SetValidSchemes(

[Matt Perry, 2013/03/19 17:49:32]

Add a comment on what's happening here.

[aboxhall, 2013/03/20 22:04:59]

Done.

[aboxhall, 2013/03/20 22:04:59]

Done.

+          pattern.valid_schemes() & ~URLPattern::SCHEME_CHROMEUI);
+    }
+
     if (pattern.MatchesScheme(chrome::kFileScheme) &&
         !CanExecuteScriptEverywhere()) {
       wants_file_access_ = true;
@@ -2408,6 +2434,12 @@ bool Extension::CanSpecifyHostPermission(const URLPattern& pattern,
     if (CanExecuteScriptEverywhere())
       return true;
 
+    if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kExtensionsOnChromeURLs))
+      return true;
+
+    // TODO(aboxhall): return from_webstore() when webstore handles blocking
+    // extensions which request chrome:// urls
     return false;
   }