Fix bug where we'd allow chrome-extension URLs to be loaded in incognito mode
as subresources, even though the extension is not allowed in incognito.
BUG=118721
TEST=see bug for repro; try the proof of concept in incognito mode with adblock installed but not allowed in incognito. adblock should not be detected.
Review URL: http://codereview.chromium.org/9726025
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128097 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/extensions/extension_protocols.cc b/chrome/browser/extensions/extension_protocols.cc
index 55f435470..4b55153 100644
--- a/chrome/browser/extensions/extension_protocols.cc
+++ b/chrome/browser/extensions/extension_protocols.cc
@@ -165,12 +165,22 @@
net::HttpResponseInfo response_info_;
};
-bool ExtensionCanLoadInIncognito(const std::string& extension_id,
+bool ExtensionCanLoadInIncognito(const ResourceRequestInfo* info,
+ const std::string& extension_id,
ExtensionInfoMap* extension_info_map) {
- const Extension* extension =
- extension_info_map->extensions().GetByID(extension_id);
- // Only split-mode extensions can load in incognito profiles.
- return extension && extension->incognito_split_mode();
+ if (!extension_info_map->IsIncognitoEnabled(extension_id))
+ return false;
+
+ // Only allow incognito toplevel navigations to extension resources in
+ // split mode. In spanning mode, the extension must run in a single process,
+ // and an incognito tab prevents that.
+ if (info->GetResourceType() == ResourceType::MAIN_FRAME) {
+ const Extension* extension =
+ extension_info_map->extensions().GetByID(extension_id);
+ return extension && extension->incognito_split_mode();
+ }
+
+ return true;
}
// Returns true if an chrome-extension:// resource should be allowed to load.
@@ -189,14 +199,8 @@
return true;
}
- // 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 (is_incognito &&
- info->GetResourceType() == ResourceType::MAIN_FRAME &&
- !ExtensionCanLoadInIncognito(request->url().host(), extension_info_map)) {
- LOG(ERROR) << "Denying load of " << request->url().spec() << " from "
- << "incognito tab.";
+ if (is_incognito && !ExtensionCanLoadInIncognito(info, request->url().host(),
+ extension_info_map)) {
return false;
}
@@ -242,7 +246,6 @@
// TODO(mpcomplete): better error code.
if (!AllowExtensionResourceLoad(
request, is_incognito_, extension_info_map_)) {
- LOG(ERROR) << "disallowed in extension protocols";
return new net::URLRequestErrorJob(request, net::ERR_ADDRESS_UNREACHABLE);
}
diff --git a/chrome/browser/extensions/extension_protocols_unittest.cc b/chrome/browser/extensions/extension_protocols_unittest.cc
new file mode 100644
index 0000000..2437db4
--- /dev/null
+++ b/chrome/browser/extensions/extension_protocols_unittest.cc
@@ -0,0 +1,153 @@
+// Copyright (c) 2012 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 <string>
+
+#include "base/file_util.h"
+#include "base/message_loop.h"
+#include "chrome/browser/extensions/extension_info_map.h"
+#include "chrome/browser/extensions/extension_protocols.h"
+#include "chrome/common/url_constants.h"
+#include "content/public/browser/resource_request_info.h"
+#include "content/test/mock_resource_context.h"
+#include "content/test/test_browser_thread.h"
+#include "net/url_request/url_request.h"
+#include "net/url_request/url_request_status.h"
+#include "net/url_request/url_request_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using content::BrowserThread;
+
+namespace {
+
+scoped_refptr<Extension> CreateTestExtension(const std::string& name,
+ bool incognito_split_mode) {
+ DictionaryValue manifest;
+ manifest.SetString("name", name);
+ manifest.SetString("version", "1");
+ manifest.SetString("incognito", incognito_split_mode ? "split" : "spanning");
+
+ FilePath path;
+ EXPECT_TRUE(file_util::GetCurrentDirectory(&path));
+
+ std::string error;
+ scoped_refptr<Extension> extension(
+ Extension::Create(path, Extension::INTERNAL, manifest,
+ Extension::STRICT_ERROR_CHECKS, &error));
+ EXPECT_TRUE(extension.get()) << error;
+ return extension;
+}
+
+class ExtensionProtocolTest : public testing::Test {
+ public:
+ ExtensionProtocolTest()
+ : ui_thread_(BrowserThread::UI, &message_loop_),
+ file_thread_(BrowserThread::FILE, &message_loop_),
+ io_thread_(BrowserThread::IO, &message_loop_) {}
+
+ virtual void SetUp() {
+ extension_info_map_ = new ExtensionInfoMap();
+ net::URLRequestContext* request_context =
+ resource_context_.GetRequestContext();
+ old_factory_ = request_context->job_factory();
+ // Register an incognito extension protocol handler.
+ job_factory_.SetProtocolHandler(
+ chrome::kExtensionScheme,
+ CreateExtensionProtocolHandler(true, extension_info_map_));
+ request_context->set_job_factory(&job_factory_);
+ }
+
+ virtual void TearDown() {
+ net::URLRequestContext* request_context =
+ resource_context_.GetRequestContext();
+ request_context->set_job_factory(old_factory_);
+ }
+
+ void StartRequest(net::URLRequest* request,
+ ResourceType::Type resource_type) {
+ content::ResourceRequestInfo::AllocateForTesting(request,
+ resource_type,
+ &resource_context_);
+ request->set_context(resource_context_.GetRequestContext());
+ request->Start();
+ MessageLoop::current()->Run();
+ }
+
+ protected:
+ MessageLoopForIO message_loop_;
+ content::TestBrowserThread ui_thread_;
+ content::TestBrowserThread file_thread_;
+ content::TestBrowserThread io_thread_;
+ scoped_refptr<ExtensionInfoMap> extension_info_map_;
+ net::URLRequestJobFactory job_factory_;
+ const net::URLRequestJobFactory* old_factory_;
+ TestDelegate test_delegate_;
+ content::MockResourceContext resource_context_;
+};
+
+// Tests that making a chrome-extension request in an incognito context is
+// only allowed under the right circumstances (if the extension is allowed
+// in incognito, and it's either a non-main-frame request or a split-mode
+// extension).
+TEST_F(ExtensionProtocolTest, IncognitoRequest) {
+ struct TestCase {
+ // Inputs.
+ std::string name;
+ bool incognito_split_mode;
+ bool incognito_enabled;
+
+ // Expected results.
+ bool should_allow_main_frame_load;
+ bool should_allow_sub_frame_load;
+ } cases[] = {
+ {"spanning disabled", false, false, false, false},
+ {"split disabled", true, false, false, false},
+ {"spanning enabled", false, true, false, true},
+ {"split enabled", true, true, true, true},
+ };
+
+ for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) {
+ scoped_refptr<Extension> extension =
+ CreateTestExtension(cases[i].name, cases[i].incognito_split_mode);
+ extension_info_map_->AddExtension(
+ extension, base::Time::Now(), cases[i].incognito_enabled);
+
+ // First test a main frame request.
+ {
+ // It doesn't matter that the resource doesn't exist. If the resource
+ // is blocked, we should see ADDRESS_UNREACHABLE. Otherwise, the request
+ // should just fail because the file doesn't exist.
+ net::URLRequest request(extension->GetResourceURL("404.html"),
+ &test_delegate_);
+ StartRequest(&request, ResourceType::MAIN_FRAME);
+ EXPECT_EQ(net::URLRequestStatus::FAILED, request.status().status());
+
+ if (cases[i].should_allow_main_frame_load) {
+ EXPECT_EQ(net::ERR_FILE_NOT_FOUND, request.status().error()) <<
+ cases[i].name;
+ } else {
+ EXPECT_EQ(net::ERR_ADDRESS_UNREACHABLE, request.status().error()) <<
+ cases[i].name;
+ }
+ }
+
+ // Now do a subframe request.
+ {
+ net::URLRequest request(extension->GetResourceURL("404.html"),
+ &test_delegate_);
+ StartRequest(&request, ResourceType::SUB_FRAME);
+ EXPECT_EQ(net::URLRequestStatus::FAILED, request.status().status());
+
+ if (cases[i].should_allow_sub_frame_load) {
+ EXPECT_EQ(net::ERR_FILE_NOT_FOUND, request.status().error()) <<
+ cases[i].name;
+ } else {
+ EXPECT_EQ(net::ERR_ADDRESS_UNREACHABLE, request.status().error()) <<
+ cases[i].name;
+ }
+ }
+ }
+}
+
+} // namespace
