Commit 7bb43d67 authored by ananta's avatar ananta Committed by Commit bot

PlzNavigate: Fix the FindInPageControllerTest.SearchWithinSpecialURL browser test.

This test was initiating a navigation to the chrome\test\data folder and performing a
number of find requests on the page returned. With PlzNavigate the request to navigate
to chrome\test\data folder would get blocked in ResourceLoader::OnReceivedRedirect because
the process id (-1) did not have access to the url being navigated to. In the non PlzNavigate
case this is not an issue because the child process is given access to the URL during
navigation.

Proposed fix for PlzNavigate is to not do the ChildProcessSecurityPolicy check in
the ResourceLoader::OnReceivedRedirect function and instead do this check on the UI thread
in the NavigationRequest::OnRedirectChecksComplete function. Additionally we also grant
access to the URL in the NavigatorImpl::RequestNavigation function.

The above test is now enabled for Windows.

BUG=175711

Disabled due to crbug.com/175711
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2436253002
Cr-Commit-Position: refs/heads/master@{#427279}
parent 13e00c15
...@@ -301,8 +301,8 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FindInPageFormsTextAreas) { ...@@ -301,8 +301,8 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FindInPageFormsTextAreas) {
// Verify search for text within special URLs such as chrome:history, // Verify search for text within special URLs such as chrome:history,
// chrome://downloads, data directory // chrome://downloads, data directory
#if defined(OS_WIN) || defined(OS_MACOSX) #if defined(OS_MACOSX)
// Disabled due to crbug.com/175711 and http://crbug.com/419987 // Disabled due to http://crbug.com/419987
#define MAYBE_SearchWithinSpecialURL \ #define MAYBE_SearchWithinSpecialURL \
DISABLED_SearchWithinSpecialURL DISABLED_SearchWithinSpecialURL
#else #else
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <utility> #include <utility>
#include "content/browser/child_process_security_policy_impl.h"
#include "content/browser/devtools/render_frame_devtools_agent_host.h" #include "content/browser/devtools/render_frame_devtools_agent_host.h"
#include "content/browser/frame_host/frame_tree.h" #include "content/browser/frame_host/frame_tree.h"
#include "content/browser/frame_host/frame_tree_node.h" #include "content/browser/frame_host/frame_tree_node.h"
...@@ -15,6 +16,7 @@ ...@@ -15,6 +16,7 @@
#include "content/browser/frame_host/navigator.h" #include "content/browser/frame_host/navigator.h"
#include "content/browser/frame_host/navigator_impl.h" #include "content/browser/frame_host/navigator_impl.h"
#include "content/browser/loader/navigation_url_loader.h" #include "content/browser/loader/navigation_url_loader.h"
#include "content/browser/renderer_host/render_process_host_impl.h"
#include "content/browser/service_worker/service_worker_context_wrapper.h" #include "content/browser/service_worker/service_worker_context_wrapper.h"
#include "content/browser/service_worker/service_worker_navigation_handle.h" #include "content/browser/service_worker/service_worker_navigation_handle.h"
#include "content/browser/site_instance_impl.h" #include "content/browser/site_instance_impl.h"
...@@ -333,8 +335,19 @@ void NavigationRequest::OnRequestRedirected( ...@@ -333,8 +335,19 @@ void NavigationRequest::OnRequestRedirected(
common_params_.method = redirect_info.new_method; common_params_.method = redirect_info.new_method;
common_params_.referrer.url = GURL(redirect_info.new_referrer); common_params_.referrer.url = GURL(redirect_info.new_referrer);
// TODO(clamy): Have CSP + security upgrade checks here. // For non browser initiated navigations we need to check if the source has
// access to the URL. We always allow browser initiated requests.
// TODO(clamy): Kill the renderer if FilterURL fails? // TODO(clamy): Kill the renderer if FilterURL fails?
GURL url = common_params_.url;
if (!browser_initiated_ && source_site_instance()) {
source_site_instance()->GetProcess()->FilterURL(false, &url);
// FilterURL sets the URL to about:blank if the CSP checks prevent the
// renderer from accessing it.
if ((url == url::kAboutBlankURL) && (url != common_params_.url)) {
frame_tree_node_->ResetNavigationRequest(false);
return;
}
}
// It's safe to use base::Unretained because this NavigationRequest owns the // It's safe to use base::Unretained because this NavigationRequest owns the
// NavigationHandle where the callback will be stored. // NavigationHandle where the callback will be stored.
......
...@@ -223,6 +223,7 @@ specific_include_rules = { ...@@ -223,6 +223,7 @@ specific_include_rules = {
"+content/common/content_export.h", "+content/common/content_export.h",
"+content/public/browser/resource_controller.h", "+content/public/browser/resource_controller.h",
"+content/public/browser/resource_dispatcher_host_login_delegate.h", "+content/public/browser/resource_dispatcher_host_login_delegate.h",
"+content/public/common/browser_side_navigation_policy.h",
"+content/public/common/resource_response.h", "+content/public/common/resource_response.h",
"+content/public/common/resource_type.h", "+content/public/common/resource_type.h",
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "content/browser/ssl/ssl_client_auth_handler.h" #include "content/browser/ssl/ssl_client_auth_handler.h"
#include "content/browser/ssl/ssl_manager.h" #include "content/browser/ssl/ssl_manager.h"
#include "content/public/browser/resource_dispatcher_host_login_delegate.h" #include "content/public/browser/resource_dispatcher_host_login_delegate.h"
#include "content/public/common/browser_side_navigation_policy.h"
#include "content/public/common/content_client.h" #include "content/public/common/content_client.h"
#include "content/public/common/content_switches.h" #include "content/public/common/content_switches.h"
#include "content/public/common/process_type.h" #include "content/public/common/process_type.h"
...@@ -256,14 +257,21 @@ void ResourceLoader::OnReceivedRedirect(net::URLRequest* unused, ...@@ -256,14 +257,21 @@ void ResourceLoader::OnReceivedRedirect(net::URLRequest* unused,
ResourceRequestInfoImpl* info = GetRequestInfo(); ResourceRequestInfoImpl* info = GetRequestInfo();
if (!ChildProcessSecurityPolicyImpl::GetInstance()->CanRequestURL( // With PlzNavigate for frame navigations this check is done in the
info->GetChildID(), redirect_info.new_url)) { // NavigationRequest::OnReceivedRedirect() function.
DVLOG(1) << "Denied unauthorized request for " bool check_handled_elsewhere = IsBrowserSideNavigationEnabled() &&
<< redirect_info.new_url.possibly_invalid_spec(); IsResourceTypeFrame(info->GetResourceType());
// Tell the renderer that this request was disallowed. if (!check_handled_elsewhere) {
Cancel(); if (!ChildProcessSecurityPolicyImpl::GetInstance()->CanRequestURL(
return; info->GetChildID(), redirect_info.new_url)) {
DVLOG(1) << "Denied unauthorized request for "
<< redirect_info.new_url.possibly_invalid_spec();
// Tell the renderer that this request was disallowed.
Cancel();
return;
}
} }
if (delegate_->HandleExternalProtocol(this, redirect_info.new_url)) { if (delegate_->HandleExternalProtocol(this, redirect_info.new_url)) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment