Allow data URLs to load chrome-extension:// resources. Basic

notifications use data: URLs internally so this was required
to support that.

Also rejigger the logic in this area again to clarify and fix
an issue where the incognito window check did not have high
enough priority.

Also add back a null-pointer check that was previously in this code and is apparently necessary with a comment.

BUG=52313,52374

Review URL: http://codereview.chromium.org/3159019

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

2 files changed, 53 insertions, 16 deletions

diff --git a/chrome/browser/extensions/extension_browsertests_misc.cc b/chrome/browser/extensions/extension_browsertests_misc.cc
index 99966df..4ede48d 100644
--- a/chrome/browser/extensions/extension_browsertests_misc.cc
+++ b/chrome/browser/extensions/extension_browsertests_misc.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/file_util.h"
 #include "base/ref_counted.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser.h"
@@ -99,6 +100,20 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, OriginPrivileges) {
       &result);
   EXPECT_EQ(result, "Image failed to load");
 
+  // A data URL. Data URLs should always be able to load chrome-extension://
+  // resources.
+  std::string file_source;
+  ASSERT_TRUE(file_util::ReadFileToString(
+      test_data_dir_.AppendASCII("origin_privileges")
+                    .AppendASCII("index.html"), &file_source));
+  ui_test_utils::NavigateToURL(browser(),
+      GURL(std::string("data:text/html;charset=utf-8,") + file_source));
+  ui_test_utils::ExecuteJavaScriptAndExtractString(
+    browser()->GetSelectedTabContents()->render_view_host(), L"",
+      L"window.domAutomationController.send(document.title)",
+      &result);
+  EXPECT_EQ(result, "Loaded");
+
   // A different extension. Extensions should always be able to load each
   // other's resources.
   ASSERT_TRUE(LoadExtension(test_data_dir_
diff --git a/chrome/browser/extensions/extension_protocols.cc b/chrome/browser/extensions/extension_protocols.cc
index f1b09eb..30f7b37 100644
--- a/chrome/browser/extensions/extension_protocols.cc
+++ b/chrome/browser/extensions/extension_protocols.cc
@@ -72,6 +72,14 @@ bool AllowExtensionResourceLoad(URLRequest* request,
   const ResourceDispatcherHostRequestInfo* info =
       ResourceDispatcherHost::InfoForRequest(request);
 
+  // We have seen crashes where info is NULL: crbug.com/52374.
+  if (!info) {
+    LOG(ERROR) << "Allowing load of " << request->url().spec()
+               << "from unknown origin. Could not find user data for "
+               << "request.";
+    return true;
+  }
+
   GURL origin_url(info->frame_origin());
 
   // chrome:// URLs are always allowed to load chrome-extension:// resources.
@@ -81,32 +89,46 @@ bool AllowExtensionResourceLoad(URLRequest* request,
 
   // Disallow loading of packaged resources for hosted apps. We don't allow
   // hybrid hosted/packaged apps.
-  if (context->ExtensionHasWebExtent(request->url().host()))
-    return false;
-
-  // chrome-extension:// pages can load resources from extensions and packaged
-  // apps. This is allowed for legacy reasons.
-  if (origin_url.SchemeIs(chrome::kExtensionScheme))
-    return true;
-
-  // Extension resources should only be loadable from web pages which the
-  // extension has host permissions to (and therefore could be running script
-  // in, which might need access to the extension resources).
-  ExtensionExtent host_permissions =
-      context->GetEffectiveHostPermissionsForExtension(request->url().host());
-  if (!origin_url.is_empty() && !host_permissions.ContainsURL(origin_url))
+  if (context->ExtensionHasWebExtent(request->url().host())) {
+    LOG(ERROR) << "Denying load of " << request->url().spec() << " from "
+               << "hosted app.";
     return false;
+  }
 
   // Don't allow toplevel navigations to extension resources in incognito mode.
   // This is because an extension must run in a single process, and an
   // incognito tab prevents that.
   if (context->is_off_the_record() &&
       info->resource_type() == ResourceType::MAIN_FRAME) {
+    LOG(ERROR) << "Denying load of " << request->url().spec() << " from "
+               << "incognito tab.";
     return false;
   }
 
-  // Otherwise, the resource load is allowed.
-  return true;
+  // Otherwise, pages are allowed to load resources from extensions if the
+  // extension has host permissions to (and therefore could be running script
+  // in, which might need access to the extension resources).
+  //
+  // Exceptions are:
+  // - empty origin (needed for some edge cases when we have empty origins)
+  // - chrome-extension:// (for legacy reasons -- some extensions interop)
+  // - data: (basic HTML notifications use data URLs internally)
+  if (origin_url.is_empty() ||
+      origin_url.SchemeIs(chrome::kExtensionScheme) |
+      origin_url.SchemeIs(chrome::kDataScheme)) {
+    return true;
+  } else {
+    ExtensionExtent host_permissions =
+        context->GetEffectiveHostPermissionsForExtension(request->url().host());
+    if (host_permissions.ContainsURL(origin_url)) {
+      return true;
+    } else {
+      LOG(ERROR) << "Denying load of " << request->url().spec() << " from "
+                 << origin_url.spec() << " because the extension does not have "
+                 << "access to the requesting page.";
+      return false;
+    }
+  }
 }
 
 }  // namespace