From 52277c872b368db45bc6d2f2bd9a4b7a0eb2ce8d Mon Sep 17 00:00:00 2001 From: rob Date: Sun, 7 Feb 2016 09:28:57 -0800 Subject: Add frameId to chrome.tabs.executeScript/insertCSS - Re-implemented https://codereview.chromium.org/952473002/, with all checks at the browser side instead of just the renderer. - Use the last committed URL for checking permissions instead of the visible URL. As a result, executeScript/insertCSS will now succeed in frames where the currently committed page is scriptable by extensions, but the target of the pending navigation is is not. - ExecuteScript: Do not send IPC to non-live frames (they're not going to reply anyway). - Include URL in the error if the extension has the tabs permission (follow-up to TODO from https://codereview.chromium.org/1414223005). BUG=63979,551626 TEST=./browser_tests --gtest_filter=ExecuteScriptApiTest.* CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation Review URL: https://codereview.chromium.org/1628423002 Cr-Commit-Position: refs/heads/master@{#374057} --- .../renderer/programmatic_script_injector.cc | 33 +++++++++++++++------- extensions/renderer/programmatic_script_injector.h | 10 ++++--- 2 files changed, 29 insertions(+), 14 deletions(-) (limited to 'extensions/renderer') diff --git a/extensions/renderer/programmatic_script_injector.cc b/extensions/renderer/programmatic_script_injector.cc index 13f0a20..e19a4c9 100644 --- a/extensions/renderer/programmatic_script_injector.cc +++ b/extensions/renderer/programmatic_script_injector.cc @@ -13,14 +13,15 @@ #include "extensions/common/error_utils.h" #include "extensions/common/extension_messages.h" #include "extensions/common/manifest_constants.h" +#include "extensions/common/permissions/api_permission.h" #include "extensions/common/permissions/permissions_data.h" #include "extensions/renderer/injection_host.h" +#include "extensions/renderer/renderer_extension_registry.h" #include "extensions/renderer/script_context.h" #include "third_party/WebKit/public/platform/WebString.h" #include "third_party/WebKit/public/web/WebDocument.h" #include "third_party/WebKit/public/web/WebLocalFrame.h" #include "third_party/WebKit/public/web/WebScriptSource.h" -#include "url/origin.h" namespace extensions { @@ -31,8 +32,10 @@ ProgrammaticScriptInjector::ProgrammaticScriptInjector( url_( ScriptContext::GetDataSourceURLForFrame(render_frame->GetWebFrame())), finished_(false) { - effective_url_ = ScriptContext::GetEffectiveDocumentURL( - render_frame->GetWebFrame(), url_, params.match_about_blank); + if (url_.SchemeIs(url::kAboutScheme)) { + origin_for_about_error_ = + render_frame->GetWebFrame()->securityOrigin().toString().utf8(); + } } ProgrammaticScriptInjector::~ProgrammaticScriptInjector() { @@ -130,16 +133,15 @@ void ProgrammaticScriptInjector::OnWillNotInject( std::string error; switch (reason) { case NOT_ALLOWED: - if (url_.SchemeIs(url::kAboutScheme)) { + if (!CanShowUrlInError()) { + error = manifest_errors::kCannotAccessPage; + } else if (!origin_for_about_error_.empty()) { error = ErrorUtils::FormatErrorMessage( manifest_errors::kCannotAccessAboutUrl, url_.spec(), - url::Origin(effective_url_).Serialize()); + origin_for_about_error_); } else { - // TODO(?) It would be nice to show kCannotAccessPageWithUrl here if - // this is triggered by an extension with tabs permission. See - // https://codereview.chromium.org/1414223005/diff/1/extensions/ - // common/manifest_constants.cc#newcode269 - error = manifest_errors::kCannotAccessPage; + error = ErrorUtils::FormatErrorMessage( + manifest_errors::kCannotAccessPageWithUrl, url_.spec()); } break; case EXTENSION_REMOVED: // no special error here. @@ -149,6 +151,17 @@ void ProgrammaticScriptInjector::OnWillNotInject( Finish(error, render_frame); } +bool ProgrammaticScriptInjector::CanShowUrlInError() const { + if (params_->host_id.type() != HostID::EXTENSIONS) + return false; + const Extension* extension = + RendererExtensionRegistry::Get()->GetByID(params_->host_id.id()); + if (!extension) + return false; + return extension->permissions_data()->active_permissions().HasAPIPermission( + APIPermission::kTab); +} + UserScript::RunLocation ProgrammaticScriptInjector::GetRunLocation() const { return static_cast(params_->run_at); } diff --git a/extensions/renderer/programmatic_script_injector.h b/extensions/renderer/programmatic_script_injector.h index 159ff1c..a7af9ff 100644 --- a/extensions/renderer/programmatic_script_injector.h +++ b/extensions/renderer/programmatic_script_injector.h @@ -50,6 +50,9 @@ class ProgrammaticScriptInjector : public ScriptInjector { void OnWillNotInject(InjectFailureReason reason, content::RenderFrame* render_frame) override; + // Whether it is safe to include information about the URL in error messages. + bool CanShowUrlInError() const; + // Return the run location for this injector. UserScript::RunLocation GetRunLocation() const; @@ -63,10 +66,9 @@ class ProgrammaticScriptInjector : public ScriptInjector { // The url of the frame into which we are injecting. GURL url_; - // The URL of the frame's origin. This is usually identical to |url_|, but - // could be different for e.g. about:blank URLs. Do not use this value to make - // security decisions, to avoid race conditions (e.g. due to navigation). - GURL effective_url_; + // The serialization of the frame's origin if the frame is an about:-URL. This + // is used to provide user-friendly messages. + std::string origin_for_about_error_; // The results of the script execution. base::ListValue results_; -- cgit v1.1