Commit 92c6e42a authored by Clark DuVall's avatar Clark DuVall Committed by Commit Bot

Change WebRequest proxy redirect logic to match NavigationURLLoaderImpl

The safe redirect checking logic in NavigationURLLoaderImpl was changed
in http://crrev.com/c/1779310 to remove the check for web accessible
extensions resources. There was similar logic in the WebRequest proxy
that should be removed to match.

This was causing the network_service_web_request_proxy_browser_tests to
fail on the Mojo Linux bot. These tests run with a WebRequest proxy
forced on to test the logic:
https://ci.chromium.org/p/chromium/builders/ci/Mojo%20Linux/40022

Bug: 442579
Change-Id: I47a208d0586e0dc6389bb8bd4aeae2b3a61c78a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1853133
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: default avatarŁukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704757}
parent 5bfb61c3
......@@ -115,9 +115,9 @@ chrome.test.getConfig(function(config) {
chrome.webRequest.onHeadersReceived.addListener(listener,
{urls: [url]}, onHeadersReceivedExtraInfoSpec);
// The page should be redirected to redirectURL, but not to the non web
// accessible URL.
assertRedirectSucceeds(url, redirectURL, function() {
// The page should be redirected to the non web accessible URL, but this
// URL will not load.
assertRedirectSucceeds(url, getURLNonWebAccessible(), function() {
chrome.webRequest.onHeadersReceived.removeListener(listener);
});
},
......@@ -208,9 +208,9 @@ chrome.test.getConfig(function(config) {
chrome.webRequest.onBeforeRequest.addListener(listener,
{urls: [url]}, ['blocking']);
// The page should be redirected to redirectURL, but not to the non web
// accessible URL.
assertRedirectSucceeds(url, redirectURL, function() {
// The page should be redirected to the non web accessible URL, but this
// URL will not load.
assertRedirectSucceeds(url, getURLNonWebAccessible(), function() {
chrome.webRequest.onBeforeRequest.removeListener(listener);
});
},
......@@ -224,8 +224,9 @@ chrome.test.getConfig(function(config) {
},
function redirectToNonWebAccessibleUrlWithServerRedirect() {
assertRedirectFails(
getServerURL('server-redirect?' + getURLNonWebAccessible()));
assertRedirectSucceeds(
getServerURL('server-redirect?' + getURLNonWebAccessible()),
getURLNonWebAccessible());
},
function redirectToWebAccessibleUrlWithServerRedirect() {
......
......@@ -274,7 +274,8 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::OnReceiveRedirect(
const net::RedirectInfo& redirect_info,
network::mojom::URLResponseHeadPtr head) {
if (redirect_url_ != redirect_info.new_url &&
!IsRedirectSafe(request_.url, redirect_info.new_url)) {
!IsRedirectSafe(request_.url, redirect_info.new_url,
info_->is_navigation_request)) {
OnRequestError(
network::URLLoaderCompletionStatus(net::ERR_UNSAFE_REDIRECT));
return;
......@@ -842,8 +843,11 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::OnRequestError(
// Determines whether it is safe to redirect from |from_url| to |to_url|.
bool WebRequestProxyingURLLoaderFactory::InProgressRequest::IsRedirectSafe(
const GURL& from_url,
const GURL& to_url) {
if (to_url.SchemeIs(extensions::kExtensionScheme)) {
const GURL& to_url,
bool is_navigation_request) {
// For navigations, non-web accessible resources will be blocked by
// ExtensionNavigationThrottle.
if (!is_navigation_request && to_url.SchemeIs(extensions::kExtensionScheme)) {
const Extension* extension =
ExtensionRegistry::Get(factory_->browser_context_)
->enabled_extensions()
......
......@@ -118,7 +118,9 @@ class WebRequestProxyingURLLoaderFactory
void HandleResponseOrRedirectHeaders(
net::CompletionOnceCallback continuation);
void OnRequestError(const network::URLLoaderCompletionStatus& status);
bool IsRedirectSafe(const GURL& from_url, const GURL& to_url);
bool IsRedirectSafe(const GURL& from_url,
const GURL& to_url,
bool is_navigation_request);
void HandleBeforeRequestRedirect();
WebRequestProxyingURLLoaderFactory* const factory_;
......
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