Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(7656)

Unified Diff: chrome/browser/net/chrome_extensions_network_delegate.cc

Issue 2870843003: Complete UI thread blob/filesystem URL blocking and remove IO thread check.
Patch Set: Remove unnecessary headers Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/net/chrome_extensions_network_delegate.cc
diff --git a/chrome/browser/net/chrome_extensions_network_delegate.cc b/chrome/browser/net/chrome_extensions_network_delegate.cc
index bc3330ee94fd7835150577c716e0e213a7abc116..27977b84f1e8fe1e9257fc970d4987ca14e72207 100644
--- a/chrome/browser/net/chrome_extensions_network_delegate.cc
+++ b/chrome/browser/net/chrome_extensions_network_delegate.cc
@@ -11,16 +11,12 @@
#include "net/base/net_errors.h"
#if BUILDFLAG(ENABLE_EXTENSIONS)
-#include "base/debug/alias.h"
-#include "base/debug/dump_without_crashing.h"
-#include "base/strings/string_util.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/extensions/api/proxy/proxy_api.h"
#include "chrome/browser/extensions/event_router_forwarder.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/renderer_host/chrome_navigation_ui_data.h"
#include "content/public/browser/browser_thread.h"
-#include "content/public/browser/child_process_security_policy.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/resource_request_info.h"
#include "content/public/common/browser_side_navigation_policy.h"
@@ -171,59 +167,6 @@ int ChromeExtensionsNetworkDelegateImpl::OnBeforeURLRequest(
net::URLRequest* request,
const net::CompletionCallback& callback,
GURL* new_url) {
- const content::ResourceRequestInfo* info =
- content::ResourceRequestInfo::ForRequest(request);
- const GURL& url(request->url());
-
- // Block top-level navigations to blob: or filesystem: URLs with extension
- // origin from non-extension processes. See https://crbug.com/645028.
- //
- // TODO(alexmos): This check is redundant with the one in
- // ExtensionNavigationThrottle::WillStartRequest, which was introduced in
- // M56. This check is reintroduced temporarily to tighten this blocking for
- // apps with a "webview" permission on M55/54 (see https://crbug.com/656752).
- // It will be removed after it's merged. Unlike the check in
- // ExtensionNavigationThrottle, this check is incompatible with PlzNavigate
- // and is disabled for that mode.
- bool is_nested_url = url.SchemeIsFileSystem() || url.SchemeIsBlob();
- bool is_navigation =
- info && content::IsResourceTypeFrame(info->GetResourceType());
- if (is_nested_url && is_navigation && info->IsMainFrame()) {
- // Nested conditional so we don't always pay the GURL -> Origin conversion.
- url::Origin origin = url::Origin(url);
- if (origin.scheme() == extensions::kExtensionScheme &&
- !extension_info_map_->process_map().Contains(info->GetChildID()) &&
- !content::IsBrowserSideNavigationEnabled()) {
- // Relax this restriction for apps that use <webview>. See
- // https://crbug.com/652077.
- const extensions::Extension* extension =
- extension_info_map_->extensions().GetByID(origin.host());
- bool has_webview_permission =
- extension &&
- extension->permissions_data()->HasAPIPermission(
- extensions::APIPermission::kWebView);
- // Check whether the request is coming from a <webview> guest process via
- // ChildProcessSecurityPolicy. A guest process should have already been
- // granted permission to request |origin| when its WebContents was
- // created. See https://crbug.com/656752.
- auto* policy = content::ChildProcessSecurityPolicy::GetInstance();
- bool from_guest =
- policy->HasSpecificPermissionForOrigin(info->GetChildID(), origin);
alexmos 2017/05/10 17:12:19 Can't remove this from the API, unfortunately, bec
- if (!has_webview_permission || !from_guest) {
- // TODO(alexmos): Temporary instrumentation to find any regressions for
- // this blocking. Remove after verifying that this is not breaking any
- // legitimate use cases.
- char origin_copy[256];
- base::strlcpy(origin_copy, origin.Serialize().c_str(),
- arraysize(origin_copy));
- base::debug::Alias(&origin_copy);
- base::debug::Alias(&from_guest);
- base::debug::DumpWithoutCrashing();
alexmos 2017/05/10 17:12:19 I checked the stats on this, and it's rare (40 ins
- return net::ERR_ABORTED;
- }
- }
- }
-
return ExtensionWebRequestEventRouter::GetInstance()->OnBeforeRequest(
profile_, extension_info_map_.get(), request, callback, new_url);
}

Powered by Google App Engine
This is Rietveld 408576698