Make <all_urls> and file:///* in permissions trigger "Allow file access"

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 e65d9e18847bac3f55342f3a2598503077e8716b..5f961c00eb18d6a536e12763c9a41d853bee2240 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -264,8 +264,7 @@ const int Extension::kValidWebExtentSchemes =
     URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS;
 
 const int Extension::kValidHostPermissionSchemes =
-    (UserScript::kValidUserScriptSchemes |
-     URLPattern::SCHEME_CHROMEUI) & ~URLPattern::SCHEME_FILE;
+    UserScript::kValidUserScriptSchemes | URLPattern::SCHEME_CHROMEUI;
 
 //
 // Extension
@@ -556,9 +555,14 @@ bool Extension::GenerateId(const std::string& input, std::string* output) {
 // content_script list of the manifest.
 bool Extension::LoadUserScriptHelper(const DictionaryValue* content_script,
                                      int definition_index,
-                                     URLPattern::ParseOption parse_strictness,
+                                     int flags,
                                      std::string* error,
                                      UserScript* result) {
+  // When strict error checks are enabled, make URL pattern parsing strict.
+  URLPattern::ParseOption parse_strictness =
+      (flags & STRICT_ERROR_CHECKS ? URLPattern::PARSE_STRICT
+                                   : URLPattern::PARSE_LENIENT);
+
   // run_at
   if (content_script->HasKey(keys::kRunAt)) {
     std::string run_location;
@@ -616,13 +620,29 @@ bool Extension::LoadUserScriptHelper(const DictionaryValue* content_script,
       return false;
     }
 
-    URLPattern pattern(UserScript::kValidUserScriptSchemes);
-    if (CanExecuteScriptEverywhere())
-      pattern.set_valid_schemes(URLPattern::SCHEME_ALL);
+    InitWantsFileAccess(match_str, parse_strictness);

[Aaron Boodman, 2011/03/30 01:24:54]

The handling of wants_file_access_ is kind of weird, but I'm having trouble
putting my finger on what bothers me about it. So here are some alternate ideas
to consider:

1. Instead of parsing each pattern twice, you could instead always parse with
SCHEME_FILE, then check MatchesScheme(FILE), then remove SCHEME_FILE from
allowed_schemes if necessary.

2. You could always parse URLPatterns with SCHEME_FILE and then centralize
handling of wants_file_access_ to the end of ::InitFromValue(). Iterate through
all the content script matches and set wants_file_access_ and remove SCHEME_FILE
from the URLPatterns as necessary.

3. You could nix wants_files_access_ completely and make this functionality part
of ExtensionPrefs. ExtensionPrefs has access to the real manifest, and could
have code that looks at it and decides whether the extension wants file access,
without having to have anything in Extension.

4. Maybe you already considered this and have some future plan?

+
+    int valid_user_script_schemes = CanExecuteScriptEverywhere() ?
+        URLPattern::SCHEME_ALL : UserScript::kValidUserScriptSchemes;
+    if (flags & ALLOW_FILE_ACCESS)
+      valid_user_script_schemes |= URLPattern::SCHEME_FILE;
 
+    URLPattern pattern(valid_user_script_schemes);
     URLPattern::ParseResult parse_result = pattern.Parse(match_str,
                                                          parse_strictness);
     if (parse_result != URLPattern::PARSE_SUCCESS) {
+      // Normally invalid URL patterns result in errors, but if a script is
+      // requesting file:/// access before it has been granted, we silently
+      // skip over that pattern.
+      if (parse_result == URLPattern::PARSE_ERROR_INVALID_SCHEME &&
+          !(flags & ALLOW_FILE_ACCESS)) {
+        URLPattern temp_pattern(URLPattern::SCHEME_FILE);
+        URLPattern::ParseResult temp_parse_result =
+            temp_pattern.Parse(match_str, parse_strictness);
+        if (temp_parse_result == URLPattern::PARSE_SUCCESS)
+          continue;
+      }
+
       *error = ExtensionErrorUtils::FormatErrorMessage(
           errors::kInvalidMatch,
           base::IntToString(definition_index),
@@ -1212,7 +1232,8 @@ Extension::Extension(const FilePath& path, Location location)
       is_storage_isolated_(false),
       launch_container_(extension_misc::LAUNCH_TAB),
       launch_width_(0),
-      launch_height_(0) {
+      launch_height_(0),
+      wants_file_access_(false) {
   DCHECK(path.IsAbsolute());
   path_ = MaybeNormalizePath(path);
 }
@@ -1774,8 +1795,7 @@ bool Extension::InitFromValue(const DictionaryValue& source, int flags,
       }
 
       UserScript script;
-      if (!LoadUserScriptHelper(content_script, i, parse_strictness, error,
-                                &script))
+      if (!LoadUserScriptHelper(content_script, i, flags, error, &script))
         return false;  // Failed to parse script context definition.
       script.set_extension_id(id());
       if (converted_from_user_script_) {
@@ -1945,9 +1965,12 @@ bool Extension::InitFromValue(const DictionaryValue& source, int flags,
       }
 
       // Check if it's a host pattern permission.
-      URLPattern pattern = URLPattern(CanExecuteScriptEverywhere() ?
-          URLPattern::SCHEME_ALL : kValidHostPermissionSchemes);
+      int valid_host_permission_schemes = CanExecuteScriptEverywhere() ?
+          URLPattern::SCHEME_ALL : kValidHostPermissionSchemes;
+      if (flags & ALLOW_FILE_ACCESS)
+        valid_host_permission_schemes |= URLPattern::SCHEME_FILE;
 
+      URLPattern pattern = URLPattern(valid_host_permission_schemes);
       URLPattern::ParseResult parse_result = pattern.Parse(permission_str,
                                                            parse_strictness);
       if (parse_result == URLPattern::PARSE_SUCCESS) {
@@ -1964,6 +1987,8 @@ bool Extension::InitFromValue(const DictionaryValue& source, int flags,
         host_permissions_.push_back(pattern);
       }
 
+      InitWantsFileAccess(permission_str, parse_strictness);
+
       // If it's not a host permission, then it's probably an unknown API
       // permission. Do not throw an error so extensions can retain
       // backwards compatability (http://crbug.com/42742).
@@ -2371,6 +2396,17 @@ bool Extension::HasHostPermission(const GURL& url) const {
   return false;
 }
 
+void Extension::InitWantsFileAccess(const std::string& host_str,
+                                    URLPattern::ParseOption parse_strictness) {
+  URLPattern pattern = URLPattern(URLPattern::SCHEME_ALL);
+  URLPattern::ParseResult parse_result = pattern.Parse(host_str,
+                                                       parse_strictness);
+  if (parse_result == URLPattern::PARSE_SUCCESS &&
+      pattern.MatchesScheme(chrome::kFileScheme)) {
+    wants_file_access_ = true;
+  }
+}
+
 void Extension::InitEffectiveHostPermissions() {
   for (URLPatternList::const_iterator host = host_permissions().begin();
        host != host_permissions().end(); ++host)
@@ -2402,7 +2438,7 @@ bool Extension::HasMultipleUISurfaces() const {
 }
 
 bool Extension::CanExecuteScriptOnPage(const GURL& page_url,
-                                       UserScript* script,
+                                       const UserScript* script,
                                        std::string* error) const {
   // The gallery is special-cased as a restricted URL for scripting to prevent
   // access to special JS bindings we expose to the gallery (and avoid things