From 584b8e3f26b6d751c807907ce3c480a698a961a4 Mon Sep 17 00:00:00 2001 From: "arv@chromium.org" Date: Sat, 10 Apr 2010 00:23:37 +0000 Subject: Allow extensions to access chrome://favicon/. An extension needs to add chrome://favicon/ to its permission to able to load favicon images. WebKit changes: https://bugs.webkit.org/show_bug.cgi?id=37228 BUG=37802 TEST=browser_tests.exe --gtest_filter=ExtensionApiTest.* Review URL: http://codereview.chromium.org/1610011 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@44167 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/child_process_security_policy.cc | 9 ++++++-- chrome/browser/child_process_security_policy.h | 4 ++++ chrome/browser/dom_ui/dom_ui_factory.cc | 2 +- .../extensions/extension_function_dispatcher.cc | 15 ++++++++++++ chrome/browser/extensions/permissions_apitest.cc | 4 ++++ chrome/browser/renderer_host/render_view_host.cc | 4 ++++ chrome/common/extensions/extension.cc | 22 ++++++++++++------ chrome/common/extensions/extension.h | 3 +++ chrome/common/extensions/url_pattern.cc | 1 + chrome/common/extensions/url_pattern_unittest.cc | 15 +++++++++++- chrome/common/url_constants.cc | 1 + chrome/common/url_constants.h | 1 + .../api_test/permissions/favicon/manifest.json | 7 ++++++ .../api_test/permissions/favicon/test.html | 27 ++++++++++++++++++++++ 14 files changed, 104 insertions(+), 11 deletions(-) create mode 100644 chrome/test/data/extensions/api_test/permissions/favicon/manifest.json create mode 100644 chrome/test/data/extensions/api_test/permissions/favicon/test.html diff --git a/chrome/browser/child_process_security_policy.cc b/chrome/browser/child_process_security_policy.cc index 615364e..1c4e22b 100644 --- a/chrome/browser/child_process_security_policy.cc +++ b/chrome/browser/child_process_security_policy.cc @@ -227,16 +227,21 @@ void ChildProcessSecurityPolicy::GrantUploadFile(int renderer_id, state->second->GrantUploadFile(file); } -void ChildProcessSecurityPolicy::GrantInspectElement(int renderer_id) { +void ChildProcessSecurityPolicy::GrantScheme(int renderer_id, + const std::string& scheme) { AutoLock lock(lock_); SecurityStateMap::iterator state = security_state_.find(renderer_id); if (state == security_state_.end()) return; + state->second->GrantScheme(scheme); +} + +void ChildProcessSecurityPolicy::GrantInspectElement(int renderer_id) { // The inspector is served from a chrome: URL. In order to run the // inspector, the renderer needs to be able to load chrome: URLs. - state->second->GrantScheme(chrome::kChromeUIScheme); + GrantScheme(renderer_id, chrome::kChromeUIScheme); } void ChildProcessSecurityPolicy::GrantDOMUIBindings(int renderer_id) { diff --git a/chrome/browser/child_process_security_policy.h b/chrome/browser/child_process_security_policy.h index 9cde10a..f81a259 100644 --- a/chrome/browser/child_process_security_policy.h +++ b/chrome/browser/child_process_security_policy.h @@ -69,6 +69,10 @@ class ChildProcessSecurityPolicy { // upload the file to the web. void GrantUploadFile(int renderer_id, const FilePath& file); + // Grants the renderer process the capability to access URLs of the provided + // scheme. + void GrantScheme(int renderer_id, const std::string& scheme); + // Whenever the browser processes commands the renderer to run web inspector, // it should call this method to grant the renderer process the capability to // run the inspector. diff --git a/chrome/browser/dom_ui/dom_ui_factory.cc b/chrome/browser/dom_ui/dom_ui_factory.cc index 658c2aa..de360b2 100644 --- a/chrome/browser/dom_ui/dom_ui_factory.cc +++ b/chrome/browser/dom_ui/dom_ui_factory.cc @@ -9,8 +9,8 @@ #include "chrome/browser/dom_ui/bookmarks_ui.h" #include "chrome/browser/dom_ui/downloads_ui.h" #include "chrome/browser/dom_ui/devtools_ui.h" -#include "chrome/browser/dom_ui/history_ui.h" #include "chrome/browser/dom_ui/filebrowse_ui.h" +#include "chrome/browser/dom_ui/history_ui.h" #include "chrome/browser/dom_ui/html_dialog_ui.h" #include "chrome/browser/dom_ui/mediaplayer_ui.h" #include "chrome/browser/dom_ui/net_internals_ui.h" diff --git a/chrome/browser/extensions/extension_function_dispatcher.cc b/chrome/browser/extensions/extension_function_dispatcher.cc index 7476cb0..8791bb0 100644 --- a/chrome/browser/extensions/extension_function_dispatcher.cc +++ b/chrome/browser/extensions/extension_function_dispatcher.cc @@ -6,10 +6,13 @@ #include "base/process_util.h" #include "base/singleton.h" +#include "base/ref_counted.h" #include "base/values.h" #include "chrome/browser/browser.h" #include "chrome/browser/browser_list.h" #include "chrome/browser/browser_window.h" +#include "chrome/browser/dom_ui/chrome_url_data_manager.h" +#include "chrome/browser/dom_ui/dom_ui_favicon_source.h" #include "chrome/browser/extensions/execute_code_in_tab_function.h" #include "chrome/browser/extensions/extension_accessibility_api.h" #include "chrome/browser/extensions/extension_bookmark_manager_api.h" @@ -304,6 +307,18 @@ ExtensionFunctionDispatcher::ExtensionFunctionDispatcher( bool incognito_enabled = profile()->GetExtensionsService()->IsIncognitoEnabled(extension); + // If the extension has permission to load chrome://favicon/ resources we need + // to make sure that the DOMUIFavIconSource is registered with the + // ChromeURLDataManager. + if (extension->HasHostPermission(GURL(chrome::kChromeUIFavIconURL))) { + DOMUIFavIconSource* favicon_source = new DOMUIFavIconSource(profile_); + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(Singleton::get(), + &ChromeURLDataManager::AddDataSource, + make_scoped_refptr(favicon_source))); + } + // Update the extension permissions. Doing this each time we create an EFD // ensures that new processes are informed of permissions for newly installed // extensions. diff --git a/chrome/browser/extensions/permissions_apitest.cc b/chrome/browser/extensions/permissions_apitest.cc index 55be326..82c4068 100644 --- a/chrome/browser/extensions/permissions_apitest.cc +++ b/chrome/browser/extensions/permissions_apitest.cc @@ -37,3 +37,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, ExperimentalPermissionsFail) { ASSERT_TRUE(RunExtensionTest("permissions/experimental_disabled")) << message_; } + +IN_PROC_BROWSER_TEST_F(ExtensionApiTest, FavIconPermission) { + ASSERT_TRUE(RunExtensionTest("permissions/favicon")) << message_; +} diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc index 81a9cd2..9eced8f 100644 --- a/chrome/browser/renderer_host/render_view_host.cc +++ b/chrome/browser/renderer_host/render_view_host.cc @@ -170,6 +170,10 @@ bool RenderViewHost::CreateRenderView( if (is_extensions_process) { ChildProcessSecurityPolicy::GetInstance()->GrantExtensionBindings( process()->id()); + + // Extensions may have permission to access chrome:// URLs. + ChildProcessSecurityPolicy::GetInstance()->GrantScheme( + process()->id(), chrome::kChromeUIScheme); } renderer_initialized_ = true; diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc index 7e60ee4..2e8b5bf 100644 --- a/chrome/common/extensions/extension.cc +++ b/chrome/common/extensions/extension.cc @@ -1345,9 +1345,11 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_key, return false; } - // Only accept http/https persmissions at the moment. - if ((pattern.scheme() != chrome::kHttpScheme) && - (pattern.scheme() != chrome::kHttpsScheme)) { + // We support http:// and https:// as well as chrome://favicon/. + if (!(pattern.scheme() == chrome::kHttpScheme || + pattern.scheme() == chrome::kHttpsScheme || + (pattern.scheme() == chrome::kChromeUIScheme && + pattern.host() == chrome::kChromeUIFavIconHost))) { *error = ExtensionErrorUtils::FormatErrorMessage( errors::kInvalidPermissionScheme, IntToString(i)); return false; @@ -1532,6 +1534,15 @@ Extension::Icons Extension::GetIconPathAllowLargerSize( return EXTENSION_ICON_LARGE; } +bool Extension::HasHostPermission(const GURL& url) const { + for (URLPatternList::const_iterator host = host_permissions_.begin(); + host != host_permissions_.end(); ++host) { + if (host->MatchesUrl(url)) + return true; + } + return false; +} + bool Extension::CanExecuteScriptOnHost(const GURL& url, std::string* error) const { // No extensions are allowed to execute script on the gallery because that @@ -1542,11 +1553,8 @@ bool Extension::CanExecuteScriptOnHost(const GURL& url, return false; } - for (URLPatternList::const_iterator host = host_permissions_.begin(); - host != host_permissions_.end(); ++host) { - if (host->MatchesUrl(url)) + if (HasHostPermission(url)) return true; - } if (error) { *error = ExtensionErrorUtils::FormatErrorMessage(errors::kCannotAccessPage, diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h index 165436a..7d6ac39 100644 --- a/chrome/common/extensions/extension.h +++ b/chrome/common/extensions/extension.h @@ -256,6 +256,9 @@ class Extension { // through content scripts and the hosts accessible through XHR. const std::set GetEffectiveHostPermissions() const; + // Whether the extension has access to the given URL. + bool HasHostPermission(const GURL& url) const; + // Returns true if the extension effectively has access to the user's browsing // history. There are several permissions that we group together into this // bucket. For example: tabs, bookmarks, and history. diff --git a/chrome/common/extensions/url_pattern.cc b/chrome/common/extensions/url_pattern.cc index ee40c54..e129903 100644 --- a/chrome/common/extensions/url_pattern.cc +++ b/chrome/common/extensions/url_pattern.cc @@ -15,6 +15,7 @@ static const char* kValidSchemes[] = { chrome::kHttpsScheme, chrome::kFileScheme, chrome::kFtpScheme, + chrome::kChromeUIScheme, }; static const char kPathSeparator[] = "/"; diff --git a/chrome/common/extensions/url_pattern_unittest.cc b/chrome/common/extensions/url_pattern_unittest.cc index 3eff503..8686a0f 100644 --- a/chrome/common/extensions/url_pattern_unittest.cc +++ b/chrome/common/extensions/url_pattern_unittest.cc @@ -16,7 +16,7 @@ TEST(URLPatternTest, ParseInvalid) { "http://foo.*.bar/baz", // must be first component "http:/bar", // scheme separator not found "foo://*", // invalid scheme - "chrome://*/*", // we don't support internal chrome URLs + "chrome-extenstions://*/*", // we don't support chrome extension URLs }; for (size_t i = 0; i < arraysize(kInvalidPatterns); ++i) { @@ -119,3 +119,16 @@ TEST(URLPatternTest, Match8) { EXPECT_TRUE(pattern.MatchesUrl( GURL("http://\xe1\x80\xbf/a\xc2\x81\xe1\xe1"))); }; + +// chrome:// +TEST(URLPatternTest, Match9) { + URLPattern pattern; + EXPECT_TRUE(pattern.Parse("chrome://favicon/*")); + EXPECT_EQ("chrome", pattern.scheme()); + EXPECT_EQ("favicon", pattern.host()); + EXPECT_FALSE(pattern.match_subdomains()); + EXPECT_EQ("/*", pattern.path()); + EXPECT_TRUE(pattern.MatchesUrl(GURL("chrome://favicon/http://google.com"))); + EXPECT_TRUE(pattern.MatchesUrl(GURL("chrome://favicon/https://google.com"))); + EXPECT_FALSE(pattern.MatchesUrl(GURL("chrome://history"))); +}; diff --git a/chrome/common/url_constants.cc b/chrome/common/url_constants.cc index 33b62ca..a42de1f9 100644 --- a/chrome/common/url_constants.cc +++ b/chrome/common/url_constants.cc @@ -59,6 +59,7 @@ const char kChromeUIDownloadsURL[] = "chrome://downloads/"; const char kChromeUIExtensionsURL[] = "chrome://extensions/"; const char kChromeUIHistoryURL[] = "chrome://history/"; const char kChromeUIPluginsURL[] = "chrome://plugins/"; +const char kChromeUIFavIconURL[] = "chrome://favicon/"; const char kChromeUIFileBrowseURL[] = "chrome://filebrowse/"; const char kChromeUIMediaplayerURL[] = "chrome://mediaplayer/"; const char kChromeUIIPCURL[] = "chrome://about/ipc"; diff --git a/chrome/common/url_constants.h b/chrome/common/url_constants.h index f6a3086..c35f26d 100644 --- a/chrome/common/url_constants.h +++ b/chrome/common/url_constants.h @@ -55,6 +55,7 @@ extern const char kChromeUIDownloadsURL[]; extern const char kChromeUIExtensionsURL[]; extern const char kChromeUIHistoryURL[]; extern const char kChromeUIPluginsURL[]; +extern const char kChromeUIFavIconURL[]; extern const char kChromeUIFileBrowseURL[]; extern const char kChromeUIMediaplayerURL[]; extern const char kChromeUIIPCURL[]; diff --git a/chrome/test/data/extensions/api_test/permissions/favicon/manifest.json b/chrome/test/data/extensions/api_test/permissions/favicon/manifest.json new file mode 100644 index 0000000..40e5806 --- /dev/null +++ b/chrome/test/data/extensions/api_test/permissions/favicon/manifest.json @@ -0,0 +1,7 @@ +{ + "name": "favicon test", + "version": "0.1", + "description": "Tests that we can load favicon images.", + "background_page": "test.html", + "permissions": ["chrome://favicon/"] +} diff --git a/chrome/test/data/extensions/api_test/permissions/favicon/test.html b/chrome/test/data/extensions/api_test/permissions/favicon/test.html new file mode 100644 index 0000000..fc98ccd8 --- /dev/null +++ b/chrome/test/data/extensions/api_test/permissions/favicon/test.html @@ -0,0 +1,27 @@ + + + + + -- cgit v1.1