From 066fecfb1d2b12aa2359ade87ef2f036437cfaec Mon Sep 17 00:00:00 2001 From: Clark DuVall Date: Tue, 15 Jan 2019 02:30:16 +0000 Subject: [PATCH] Fix WebRequestProxyingURLLoaderFactory crash after redirect When OnReceiveRedirect() is called, we expect to not receive any more events for that request until FollowRedirect() has been called and the request is restarted with the new URL. We were not clearing the bindings in this case, so the URLLoader was staying alive and calling OnBeforeSendHeaders() on the redirected URL. This could then result in OnBeforeSendHeaders() being called twice, once for the original request which gets redirected in //net, and once for the request in the correct flow when it gets restarted after FollowRedirect(). I believe this is pretty similar to the issue described in http://crbug.com/882661, except this one has to do with normal redirects instead of webRequest generated redirects. The existing redirect tests catch this issue when using the 'extraHeaders' spec, so I made a version of them that runs with that enabled. Bug: 918761 Change-Id: Ifa551400e85c13ea3a297fba02a05deb5ccb1821 Reviewed-on: https://chromium-review.googlesource.com/c/1409915 Reviewed-by: Ken Rockot Commit-Queue: Clark DuVall Cr-Commit-Position: refs/heads/master@{#622685} --- .../api/web_request/web_request_apitest.cc | 16 + .../api_test/webrequest/test_redirects.js | 408 +++++++++--------- .../webrequest/test_subresource_redirects.js | 260 +++++------ ...web_request_proxying_url_loader_factory.cc | 24 +- 4 files changed, 362 insertions(+), 346 deletions(-) diff --git a/chrome/browser/extensions/api/web_request/web_request_apitest.cc b/chrome/browser/extensions/api/web_request/web_request_apitest.cc index f651b404e88ed..088ae5e7c94dd 100644 --- a/chrome/browser/extensions/api/web_request/web_request_apitest.cc +++ b/chrome/browser/extensions/api/web_request/web_request_apitest.cc @@ -556,6 +556,14 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, WebRequestRedirects) { << message_; } +IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, + WebRequestRedirectsWithExtraHeaders) { + ASSERT_TRUE(StartEmbeddedTestServer()); + ASSERT_TRUE(RunExtensionSubtestWithArg("webrequest", "test_redirects.html", + "useExtraHeaders")) + << message_; +} + // Tests that redirects from secure to insecure don't send the referrer header. IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, WebRequestRedirectsToInsecure) { @@ -588,6 +596,14 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, << message_; } +IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, + WebRequestSubresourceRedirectsWithExtraHeaders) { + ASSERT_TRUE(StartEmbeddedTestServer()); + ASSERT_TRUE(RunExtensionSubtestWithArg( + "webrequest", "test_subresource_redirects.html", "useExtraHeaders")) + << message_; +} + // Fails often on Windows dbg bots. http://crbug.com/177163 #if defined(OS_WIN) #define MAYBE_WebRequestNewTab DISABLED_WebRequestNewTab diff --git a/chrome/test/data/extensions/api_test/webrequest/test_redirects.js b/chrome/test/data/extensions/api_test/webrequest/test_redirects.js index d4c197794bba9..93e222328b43c 100644 --- a/chrome/test/data/extensions/api_test/webrequest/test_redirects.js +++ b/chrome/test/data/extensions/api_test/webrequest/test_redirects.js @@ -27,203 +27,211 @@ function assertRedirectFails(url) { }); } -runTests([ - function redirectToDataUrlOnHeadersReceived() { - var url = getServerURL('echo'); - var listener = function(details) { - return {redirectUrl: dataURL}; - }; - chrome.webRequest.onHeadersReceived.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, dataURL, function() { - chrome.webRequest.onHeadersReceived.removeListener(listener); - }); - }, - - function redirectToAboutUrlOnHeadersReceived() { - var url = getServerURL('echo'); - var listener = function(details) { - return {redirectUrl: aboutURL}; - }; - chrome.webRequest.onHeadersReceived.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, aboutURL, function() { - chrome.webRequest.onHeadersReceived.removeListener(listener); - }); - }, - - function redirectToNonWebAccessibleUrlOnHeadersReceived() { - var url = getServerURL('echo'); - var listener = function(details) { - return {redirectUrl: getURLNonWebAccessible()}; - }; - chrome.webRequest.onHeadersReceived.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, getURLNonWebAccessible(), function() { - chrome.webRequest.onHeadersReceived.removeListener(listener); - }); - }, - - function redirectToServerRedirectOnHeadersReceived() { - var url = getServerURL('echo'); - var redirectURL = getServerURL('server-redirect?' + getURLWebAccessible()); - var listener = function(details) { - return {redirectUrl: redirectURL}; - }; - chrome.webRequest.onHeadersReceived.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, getURLWebAccessible(), function() { - chrome.webRequest.onHeadersReceived.removeListener(listener); - }); - }, - - function serverRedirectThenExtensionRedirectOnHeadersReceived() { - var url_1 = getServerURL('echo'); - var url_2 = getURLWebAccessible(); - var serverRedirect = getServerURL('server-redirect?' + url_1); - var listener = function(details) { - return {redirectUrl: url_2}; - }; - chrome.webRequest.onHeadersReceived.addListener( - listener, - { urls: [url_1] }, - ["blocking"] - ); - - assertRedirectSucceeds(serverRedirect, url_2, function() { - chrome.webRequest.onHeadersReceived.removeListener(listener); - }); - }, - - function redirectToUnallowedServerRedirectOnHeadersReceived() { - var url = getServerURL('echo'); - var redirectURL = getServerURL('server-redirect?' + - getURLNonWebAccessible()); - var listener = function(details) { - return {redirectUrl: redirectURL}; - }; - chrome.webRequest.onHeadersReceived.addListener(listener, - {urls: [url]}, ['blocking']); - - // The page should be redirected to redirectURL, but not to the non web - // accessible URL. - assertRedirectSucceeds(url, redirectURL, function() { - chrome.webRequest.onHeadersReceived.removeListener(listener); - }); - }, - - function redirectToDataUrlOnBeforeRequest() { - var url = getServerURL('echo'); - var listener = function(details) { - return {redirectUrl: dataURL}; - }; - chrome.webRequest.onBeforeRequest.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, dataURL, function() { - chrome.webRequest.onBeforeRequest.removeListener(listener); - }); - }, - - function redirectToAboutUrlOnBeforeRequest() { - var url = getServerURL('echo'); - var listener = function(details) { - return {redirectUrl: aboutURL}; - }; - chrome.webRequest.onBeforeRequest.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, aboutURL, function() { - chrome.webRequest.onBeforeRequest.removeListener(listener); - }); - }, - - function redirectToNonWebAccessibleUrlOnBeforeRequest() { - var url = getServerURL('echo'); - var listener = function(details) { - return {redirectUrl: getURLNonWebAccessible()}; - }; - chrome.webRequest.onBeforeRequest.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, getURLNonWebAccessible(), function() { - chrome.webRequest.onBeforeRequest.removeListener(listener); - }); - }, - - function redirectToServerRedirectOnBeforeRequest() { - var url = getServerURL('echo'); - var redirectURL = getServerURL('server-redirect?' + getURLWebAccessible()); - var listener = function(details) { - return {redirectUrl: redirectURL}; - }; - chrome.webRequest.onBeforeRequest.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, getURLWebAccessible(), function() { - chrome.webRequest.onBeforeRequest.removeListener(listener); - }); - }, - - // A server redirect immediately followed by an extension redirect. - // Regression test for: - // - https://crbug.com/882661 - // - https://crbug.com/880741 - function serverRedirectThenExtensionRedirectOnBeforeRequest() { - var url_1 = getServerURL('echo'); - var url_2 = getURLWebAccessible(); - var serverRedirect = getServerURL('server-redirect?' + url_1); - var listener = function(details) { - return {redirectUrl: url_2}; - }; - chrome.webRequest.onBeforeRequest.addListener( - listener, - { urls: [url_1] }, - ["blocking"] - ); - - assertRedirectSucceeds(serverRedirect, url_2, function() { - chrome.webRequest.onBeforeRequest.removeListener(listener); - }); - }, - - function redirectToUnallowedServerRedirectOnBeforeRequest() { - var url = getServerURL('echo'); - var redirectURL = getServerURL('server-redirect?' + - getURLNonWebAccessible()); - var listener = function(details) { - return {redirectUrl: redirectURL}; - }; - 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() { - chrome.webRequest.onBeforeRequest.removeListener(listener); - }); - }, - - function redirectToAboutUrlWithServerRedirect() { - assertRedirectFails(getServerURL('server-redirect?' + aboutURL)); - }, - - function redirectToDataUrlWithServerRedirect() { - assertRedirectFails(getServerURL('server-redirect?' + dataURL)); - }, - - function redirectToNonWebAccessibleUrlWithServerRedirect() { - assertRedirectFails( - getServerURL('server-redirect?' + getURLNonWebAccessible())); - }, - - function redirectToWebAccessibleUrlWithServerRedirect() { - assertRedirectSucceeds( - getServerURL('server-redirect?' + getURLWebAccessible()), - getURLWebAccessible()); - }, -]); +chrome.test.getConfig(function(config) { + var onHeadersReceivedExtraInfoSpec = ['blocking']; + if (config.customArg === 'useExtraHeaders') + onHeadersReceivedExtraInfoSpec.push('extraHeaders'); + + runTests([ + function redirectToDataUrlOnHeadersReceived() { + var url = getServerURL('echo'); + var listener = function(details) { + return {redirectUrl: dataURL}; + }; + chrome.webRequest.onHeadersReceived.addListener(listener, + {urls: [url]}, onHeadersReceivedExtraInfoSpec); + + assertRedirectSucceeds(url, dataURL, function() { + chrome.webRequest.onHeadersReceived.removeListener(listener); + }); + }, + + function redirectToAboutUrlOnHeadersReceived() { + var url = getServerURL('echo'); + var listener = function(details) { + return {redirectUrl: aboutURL}; + }; + chrome.webRequest.onHeadersReceived.addListener(listener, + {urls: [url]}, onHeadersReceivedExtraInfoSpec); + + assertRedirectSucceeds(url, aboutURL, function() { + chrome.webRequest.onHeadersReceived.removeListener(listener); + }); + }, + + function redirectToNonWebAccessibleUrlOnHeadersReceived() { + var url = getServerURL('echo'); + var listener = function(details) { + return {redirectUrl: getURLNonWebAccessible()}; + }; + chrome.webRequest.onHeadersReceived.addListener(listener, + {urls: [url]}, onHeadersReceivedExtraInfoSpec); + + assertRedirectSucceeds(url, getURLNonWebAccessible(), function() { + chrome.webRequest.onHeadersReceived.removeListener(listener); + }); + }, + + function redirectToServerRedirectOnHeadersReceived() { + var url = getServerURL('echo'); + var redirectURL = getServerURL('server-redirect?' + + getURLWebAccessible()); + var listener = function(details) { + return {redirectUrl: redirectURL}; + }; + chrome.webRequest.onHeadersReceived.addListener(listener, + {urls: [url]}, onHeadersReceivedExtraInfoSpec); + + assertRedirectSucceeds(url, getURLWebAccessible(), function() { + chrome.webRequest.onHeadersReceived.removeListener(listener); + }); + }, + + function serverRedirectThenExtensionRedirectOnHeadersReceived() { + var url_1 = getServerURL('echo'); + var url_2 = getURLWebAccessible(); + var serverRedirect = getServerURL('server-redirect?' + url_1); + var listener = function(details) { + return {redirectUrl: url_2}; + }; + chrome.webRequest.onHeadersReceived.addListener( + listener, + { urls: [url_1] }, + ["blocking"] + ); + + assertRedirectSucceeds(serverRedirect, url_2, function() { + chrome.webRequest.onHeadersReceived.removeListener(listener); + }); + }, + + function redirectToUnallowedServerRedirectOnHeadersReceived() { + var url = getServerURL('echo'); + var redirectURL = getServerURL('server-redirect?' + + getURLNonWebAccessible()); + var listener = function(details) { + return {redirectUrl: redirectURL}; + }; + 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() { + chrome.webRequest.onHeadersReceived.removeListener(listener); + }); + }, + + function redirectToDataUrlOnBeforeRequest() { + var url = getServerURL('echo'); + var listener = function(details) { + return {redirectUrl: dataURL}; + }; + chrome.webRequest.onBeforeRequest.addListener(listener, + {urls: [url]}, ['blocking']); + + assertRedirectSucceeds(url, dataURL, function() { + chrome.webRequest.onBeforeRequest.removeListener(listener); + }); + }, + + function redirectToAboutUrlOnBeforeRequest() { + var url = getServerURL('echo'); + var listener = function(details) { + return {redirectUrl: aboutURL}; + }; + chrome.webRequest.onBeforeRequest.addListener(listener, + {urls: [url]}, ['blocking']); + + assertRedirectSucceeds(url, aboutURL, function() { + chrome.webRequest.onBeforeRequest.removeListener(listener); + }); + }, + + function redirectToNonWebAccessibleUrlOnBeforeRequest() { + var url = getServerURL('echo'); + var listener = function(details) { + return {redirectUrl: getURLNonWebAccessible()}; + }; + chrome.webRequest.onBeforeRequest.addListener(listener, + {urls: [url]}, ['blocking']); + + assertRedirectSucceeds(url, getURLNonWebAccessible(), function() { + chrome.webRequest.onBeforeRequest.removeListener(listener); + }); + }, + + function redirectToServerRedirectOnBeforeRequest() { + var url = getServerURL('echo'); + var redirectURL = getServerURL('server-redirect?' + + getURLWebAccessible()); + var listener = function(details) { + return {redirectUrl: redirectURL}; + }; + chrome.webRequest.onBeforeRequest.addListener(listener, + {urls: [url]}, ['blocking']); + + assertRedirectSucceeds(url, getURLWebAccessible(), function() { + chrome.webRequest.onBeforeRequest.removeListener(listener); + }); + }, + + // A server redirect immediately followed by an extension redirect. + // Regression test for: + // - https://crbug.com/882661 + // - https://crbug.com/880741 + function serverRedirectThenExtensionRedirectOnBeforeRequest() { + var url_1 = getServerURL('echo'); + var url_2 = getURLWebAccessible(); + var serverRedirect = getServerURL('server-redirect?' + url_1); + var listener = function(details) { + return {redirectUrl: url_2}; + }; + chrome.webRequest.onBeforeRequest.addListener( + listener, + { urls: [url_1] }, + ["blocking"] + ); + + assertRedirectSucceeds(serverRedirect, url_2, function() { + chrome.webRequest.onBeforeRequest.removeListener(listener); + }); + }, + + function redirectToUnallowedServerRedirectOnBeforeRequest() { + var url = getServerURL('echo'); + var redirectURL = getServerURL('server-redirect?' + + getURLNonWebAccessible()); + var listener = function(details) { + return {redirectUrl: redirectURL}; + }; + 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() { + chrome.webRequest.onBeforeRequest.removeListener(listener); + }); + }, + + function redirectToAboutUrlWithServerRedirect() { + assertRedirectFails(getServerURL('server-redirect?' + aboutURL)); + }, + + function redirectToDataUrlWithServerRedirect() { + assertRedirectFails(getServerURL('server-redirect?' + dataURL)); + }, + + function redirectToNonWebAccessibleUrlWithServerRedirect() { + assertRedirectFails( + getServerURL('server-redirect?' + getURLNonWebAccessible())); + }, + + function redirectToWebAccessibleUrlWithServerRedirect() { + assertRedirectSucceeds( + getServerURL('server-redirect?' + getURLWebAccessible()), + getURLWebAccessible()); + }, + ]); +}); diff --git a/chrome/test/data/extensions/api_test/webrequest/test_subresource_redirects.js b/chrome/test/data/extensions/api_test/webrequest/test_subresource_redirects.js index c3686c3fdb7a5..b845b8eb4ad38 100644 --- a/chrome/test/data/extensions/api_test/webrequest/test_subresource_redirects.js +++ b/chrome/test/data/extensions/api_test/webrequest/test_subresource_redirects.js @@ -46,129 +46,137 @@ function assertRedirectFails(url, callback) { }); } -runTests([ - function subresourceRedirectToDataUrlOnHeadersReceived() { - var url = getServerURL('echo'); - var listener = function(details) { - return {redirectUrl: dataURL}; - }; - chrome.webRequest.onHeadersReceived.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, dataURL, function() { - chrome.webRequest.onHeadersReceived.removeListener(listener); - }); - }, - - function subresourceRedirectToNonWebAccessibleUrlOnHeadersReceived() { - var url = getServerURL('echo'); - var listener = function(details) { - return {redirectUrl: getURLNonWebAccessible()}; - }; - chrome.webRequest.onHeadersReceived.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, getURLNonWebAccessible(), function() { - chrome.webRequest.onHeadersReceived.removeListener(listener); - }); - }, - - function subresourceRedirectToServerRedirectOnHeadersReceived() { - var url = getServerURL('echo'); - var redirectURL = getServerURL('server-redirect?' + getURLWebAccessible()); - var listener = function(details) { - return {redirectUrl: redirectURL}; - }; - chrome.webRequest.onHeadersReceived.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, getURLWebAccessible(), function() { - chrome.webRequest.onHeadersReceived.removeListener(listener); - }); - }, - - function subresourceRedirectToUnallowedServerRedirectOnHeadersReceived() { - var url = getServerURL('echo'); - var redirectURL = getServerURL('server-redirect?' + - getURLNonWebAccessible()); - var listener = function(details) { - return {redirectUrl: redirectURL}; - }; - chrome.webRequest.onHeadersReceived.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectFails(url, function() { - chrome.webRequest.onHeadersReceived.removeListener(listener); - }); - }, - - function subresourceRedirectToDataUrlOnBeforeRequest() { - var url = getServerURL('echo'); - var listener = function(details) { - return {redirectUrl: dataURL}; - }; - chrome.webRequest.onBeforeRequest.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, dataURL, function() { - chrome.webRequest.onBeforeRequest.removeListener(listener); - }); - }, - - function subresourceRedirectToNonWebAccessibleUrlOnBeforeRequest() { - var url = getServerURL('echo'); - var listener = function(details) { - return {redirectUrl: getURLNonWebAccessible()}; - }; - chrome.webRequest.onBeforeRequest.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, getURLNonWebAccessible(), function() { - chrome.webRequest.onBeforeRequest.removeListener(listener); - }); - }, - - function subresourceRedirectToServerRedirectOnBeforeRequest() { - var url = getServerURL('echo'); - var redirectURL = getServerURL('server-redirect?' + getURLWebAccessible()); - var listener = function(details) { - return {redirectUrl: redirectURL}; - }; - chrome.webRequest.onBeforeRequest.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectSucceeds(url, getURLWebAccessible(), function() { - chrome.webRequest.onBeforeRequest.removeListener(listener); - }); - }, - - function subresourceRedirectToUnallowedServerRedirectOnBeforeRequest() { - var url = getServerURL('echo'); - var redirectURL = getServerURL('server-redirect?' + - getURLNonWebAccessible()); - var listener = function(details) { - return {redirectUrl: redirectURL}; - }; - chrome.webRequest.onBeforeRequest.addListener(listener, - {urls: [url]}, ['blocking']); - - assertRedirectFails(url, function() { - chrome.webRequest.onBeforeRequest.removeListener(listener); - }); - }, - - function subresourceRedirectToDataUrlWithServerRedirect() { - assertRedirectFails(getServerURL('server-redirect?' + dataURL)); - }, - - function subresourceRedirectToNonWebAccessibleWithServerRedirect() { - assertRedirectFails( - getServerURL('server-redirect?' + getURLNonWebAccessible())); - }, - - function subresourceRedirectToWebAccessibleWithServerRedirect() { - assertRedirectSucceeds( - getServerURL('server-redirect?' + getURLWebAccessible()), - getURLWebAccessible()); - }, -]); +chrome.test.getConfig(function(config) { + var onHeadersReceivedExtraInfoSpec = ['blocking']; + if (config.customArg === 'useExtraHeaders') + onHeadersReceivedExtraInfoSpec.push('extraHeaders'); + + runTests([ + function subresourceRedirectToDataUrlOnHeadersReceived() { + var url = getServerURL('echo'); + var listener = function(details) { + return {redirectUrl: dataURL}; + }; + chrome.webRequest.onHeadersReceived.addListener(listener, + {urls: [url]}, onHeadersReceivedExtraInfoSpec); + + assertRedirectSucceeds(url, dataURL, function() { + chrome.webRequest.onHeadersReceived.removeListener(listener); + }); + }, + + function subresourceRedirectToNonWebAccessibleUrlOnHeadersReceived() { + var url = getServerURL('echo'); + var listener = function(details) { + return {redirectUrl: getURLNonWebAccessible()}; + }; + chrome.webRequest.onHeadersReceived.addListener(listener, + {urls: [url]}, onHeadersReceivedExtraInfoSpec); + + assertRedirectSucceeds(url, getURLNonWebAccessible(), function() { + chrome.webRequest.onHeadersReceived.removeListener(listener); + }); + }, + + function subresourceRedirectToServerRedirectOnHeadersReceived() { + var url = getServerURL('echo'); + var redirectURL = getServerURL('server-redirect?' + + getURLWebAccessible()); + var listener = function(details) { + return {redirectUrl: redirectURL}; + }; + chrome.webRequest.onHeadersReceived.addListener(listener, + {urls: [url]}, onHeadersReceivedExtraInfoSpec); + + assertRedirectSucceeds(url, getURLWebAccessible(), function() { + chrome.webRequest.onHeadersReceived.removeListener(listener); + }); + }, + + function subresourceRedirectToUnallowedServerRedirectOnHeadersReceived() { + var url = getServerURL('echo'); + var redirectURL = getServerURL('server-redirect?' + + getURLNonWebAccessible()); + var listener = function(details) { + return {redirectUrl: redirectURL}; + }; + chrome.webRequest.onHeadersReceived.addListener(listener, + {urls: [url]}, onHeadersReceivedExtraInfoSpec); + + assertRedirectFails(url, function() { + chrome.webRequest.onHeadersReceived.removeListener(listener); + }); + }, + + function subresourceRedirectToDataUrlOnBeforeRequest() { + var url = getServerURL('echo'); + var listener = function(details) { + return {redirectUrl: dataURL}; + }; + chrome.webRequest.onBeforeRequest.addListener(listener, + {urls: [url]}, ['blocking']); + + assertRedirectSucceeds(url, dataURL, function() { + chrome.webRequest.onBeforeRequest.removeListener(listener); + }); + }, + + function subresourceRedirectToNonWebAccessibleUrlOnBeforeRequest() { + var url = getServerURL('echo'); + var listener = function(details) { + return {redirectUrl: getURLNonWebAccessible()}; + }; + chrome.webRequest.onBeforeRequest.addListener(listener, + {urls: [url]}, ['blocking']); + + assertRedirectSucceeds(url, getURLNonWebAccessible(), function() { + chrome.webRequest.onBeforeRequest.removeListener(listener); + }); + }, + + function subresourceRedirectToServerRedirectOnBeforeRequest() { + var url = getServerURL('echo'); + var redirectURL = getServerURL('server-redirect?' + + getURLWebAccessible()); + var listener = function(details) { + return {redirectUrl: redirectURL}; + }; + chrome.webRequest.onBeforeRequest.addListener(listener, + {urls: [url]}, ['blocking']); + + assertRedirectSucceeds(url, getURLWebAccessible(), function() { + chrome.webRequest.onBeforeRequest.removeListener(listener); + }); + }, + + function subresourceRedirectToUnallowedServerRedirectOnBeforeRequest() { + var url = getServerURL('echo'); + var redirectURL = getServerURL('server-redirect?' + + getURLNonWebAccessible()); + var listener = function(details) { + return {redirectUrl: redirectURL}; + }; + chrome.webRequest.onBeforeRequest.addListener(listener, + {urls: [url]}, ['blocking']); + + assertRedirectFails(url, function() { + chrome.webRequest.onBeforeRequest.removeListener(listener); + }); + }, + + function subresourceRedirectToDataUrlWithServerRedirect() { + assertRedirectFails(getServerURL('server-redirect?' + dataURL)); + }, + + function subresourceRedirectToNonWebAccessibleWithServerRedirect() { + assertRedirectFails( + getServerURL('server-redirect?' + getURLNonWebAccessible())); + }, + + function subresourceRedirectToWebAccessibleWithServerRedirect() { + assertRedirectSucceeds( + getServerURL('server-redirect?' + getURLWebAccessible()), + getURLWebAccessible()); + }, + ]); +}); diff --git a/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc b/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc index 7f32a696abb29..96c2d0d29e2e4 100644 --- a/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc +++ b/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc @@ -292,18 +292,6 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::OnHeadersReceived( void WebRequestProxyingURLLoaderFactory::InProgressRequest:: HandleBeforeRequestRedirect() { - // The extension requested a redirect. Close the connection with the current - // URLLoader and inform the URLLoaderClient the WebRequest API generated a - // redirect. To load |redirect_url_|, a new URLLoader will be recreated - // after receiving FollowRedirect(). - - // Forgetting to close the connection with the current URLLoader caused - // bugs. The latter doesn't know anything about the redirect. Continuing - // the load with it gives unexpected results. See - // https://crbug.com/882661#c72. - proxied_client_binding_.Close(); - target_loader_.reset(); - constexpr int kInternalRedirectStatusCode = 307; net::RedirectInfo redirect_info; @@ -574,13 +562,7 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest:: redirect_info.new_method = request_.method; redirect_info.new_url = new_url; redirect_info.new_site_for_cookies = new_url; - - // These will get re-bound if a new request is initiated by - // |FollowRedirect()|. - proxied_client_binding_.Close(); - target_loader_.reset(); on_receive_response_received_ = false; - ContinueToBeforeRedirect(redirect_info, net::OK); return; } @@ -605,8 +587,10 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest:: info_->AddResponseInfoFromResourceResponse(current_response_); - if (proxied_client_binding_.is_bound()) - proxied_client_binding_.ResumeIncomingMethodCallProcessing(); + // These will get re-bound if a new request is initiated by + // |FollowRedirect()|. + proxied_client_binding_.Close(); + target_loader_.reset(); ExtensionWebRequestEventRouter::GetInstance()->OnBeforeRedirect( factory_->browser_context_, factory_->info_map_, &info_.value(),