fix: crash when doing redirect navigation with webRequest listener (#21838)

* fix: pass navigation_ui_data to proxying factory

* fix: clone response instead of move in redirect
This commit is contained in:
Cheng Zhao 2020-01-22 10:31:10 +09:00 committed by GitHub
parent 36fac7bfec
commit fbc2f8344f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 33 additions and 6 deletions

View file

@ -44,6 +44,7 @@
#include "content/public/common/web_preferences.h"
#include "electron/buildflags/buildflags.h"
#include "electron/grit/electron_resources.h"
#include "extensions/browser/extension_navigation_ui_data.h"
#include "extensions/browser/extension_protocols.h"
#include "extensions/common/constants.h"
#include "net/base/escape.h"
@ -1093,6 +1094,17 @@ bool AtomBrowserClient::WillCreateURLLoaderFactory(
mojo::PendingRemote<network::mojom::URLLoaderFactory> target_factory_remote;
*factory_receiver = target_factory_remote.InitWithNewPipeAndPassReceiver();
// Required by WebRequestInfoInitParams.
//
// Note that in Electron we allow webRequest to capture requests sent from
// browser process, so creation of |navigation_ui_data| is different from
// Chromium which only does for renderer-initialized navigations.
std::unique_ptr<extensions::ExtensionNavigationUIData> navigation_ui_data;
if (navigation_id.has_value()) {
navigation_ui_data =
std::make_unique<extensions::ExtensionNavigationUIData>();
}
mojo::PendingReceiver<network::mojom::TrustedURLLoaderHeaderClient>
header_client_receiver;
if (header_client)
@ -1100,7 +1112,8 @@ bool AtomBrowserClient::WillCreateURLLoaderFactory(
new ProxyingURLLoaderFactory(
web_request.get(), protocol->intercept_handlers(), browser_context,
render_process_id, std::move(navigation_id), std::move(proxied_receiver),
render_process_id, std::move(navigation_ui_data),
std::move(navigation_id), std::move(proxied_receiver),
std::move(target_factory_remote), std::move(header_client_receiver),
type);

View file

@ -95,7 +95,9 @@ void ProxyingURLLoaderFactory::InProgressRequest::UpdateRequestInfo() {
request_for_info.request_initiator = original_initiator_;
info_.emplace(extensions::WebRequestInfoInitParams(
request_id_, factory_->render_process_id_, request_.render_frame_id,
nullptr, routing_id_, request_for_info, false,
factory_->navigation_ui_data_ ? factory_->navigation_ui_data_->DeepCopy()
: nullptr,
routing_id_, request_for_info, false,
!(options_ & network::mojom::kURLLoadOptionSynchronous),
factory_->IsForServiceWorkerScript(), factory_->navigation_id_));
@ -304,7 +306,7 @@ void ProxyingURLLoaderFactory::InProgressRequest::OnBeforeSendHeaders(
void ProxyingURLLoaderFactory::InProgressRequest::OnHeadersReceived(
const std::string& headers,
const net::IPEndPoint& endpoint,
const net::IPEndPoint& remote_endpoint,
OnHeadersReceivedCallback callback) {
if (!current_request_uses_header_client_) {
std::move(callback).Run(net::OK, base::nullopt, GURL());
@ -315,6 +317,7 @@ void ProxyingURLLoaderFactory::InProgressRequest::OnHeadersReceived(
current_response_ = network::mojom::URLResponseHead::New();
current_response_->headers =
base::MakeRefCounted<net::HttpResponseHeaders>(headers);
current_response_->remote_endpoint = remote_endpoint;
HandleResponseOrRedirectHeaders(
base::BindOnce(&InProgressRequest::ContinueToHandleOverrideHeaders,
weak_factory_.GetWeakPtr()));
@ -520,7 +523,7 @@ void ProxyingURLLoaderFactory::InProgressRequest::ContinueToResponseStarted(
proxied_client_receiver_.Resume();
factory_->web_request_api()->OnResponseStarted(&info_.value(), request_);
target_client_->OnReceiveResponse(std::move(current_response_));
target_client_->OnReceiveResponse(current_response_.Clone());
}
void ProxyingURLLoaderFactory::InProgressRequest::ContinueToBeforeRedirect(
@ -538,8 +541,7 @@ void ProxyingURLLoaderFactory::InProgressRequest::ContinueToBeforeRedirect(
factory_->web_request_api()->OnBeforeRedirect(&info_.value(), request_,
redirect_info.new_url);
target_client_->OnReceiveRedirect(redirect_info,
std::move(current_response_));
target_client_->OnReceiveRedirect(redirect_info, current_response_.Clone());
request_.url = redirect_info.new_url;
request_.method = redirect_info.new_method;
request_.site_for_cookies = redirect_info.new_site_for_cookies;
@ -675,6 +677,7 @@ ProxyingURLLoaderFactory::ProxyingURLLoaderFactory(
const HandlersMap& intercepted_handlers,
content::BrowserContext* browser_context,
int render_process_id,
std::unique_ptr<extensions::ExtensionNavigationUIData> navigation_ui_data,
base::Optional<int64_t> navigation_id,
network::mojom::URLLoaderFactoryRequest loader_request,
mojo::PendingRemote<network::mojom::URLLoaderFactory> target_factory_remote,
@ -685,6 +688,7 @@ ProxyingURLLoaderFactory::ProxyingURLLoaderFactory(
intercepted_handlers_(intercepted_handlers),
browser_context_(browser_context),
render_process_id_(render_process_id),
navigation_ui_data_(std::move(navigation_ui_data)),
navigation_id_(std::move(navigation_id)),
loader_factory_type_(loader_factory_type) {
target_factory_.Bind(std::move(target_factory_remote));

View file

@ -210,6 +210,7 @@ class ProxyingURLLoaderFactory
const HandlersMap& intercepted_handlers,
content::BrowserContext* browser_context,
int render_process_id,
std::unique_ptr<extensions::ExtensionNavigationUIData> navigation_ui_data,
base::Optional<int64_t> navigation_id,
network::mojom::URLLoaderFactoryRequest loader_request,
mojo::PendingRemote<network::mojom::URLLoaderFactory>
@ -268,6 +269,7 @@ class ProxyingURLLoaderFactory
content::BrowserContext* const browser_context_;
const int render_process_id_;
std::unique_ptr<extensions::ExtensionNavigationUIData> navigation_ui_data_;
base::Optional<int64_t> navigation_id_;
mojo::ReceiverSet<network::mojom::URLLoaderFactory> proxy_receivers_;
mojo::Remote<network::mojom::URLLoaderFactory> target_factory_;

View file

@ -121,6 +121,14 @@ describe('webRequest module', () => {
const { data } = await ajax(defaultURL)
expect(data).to.equal('/redirect')
})
it('does not crash for redirects', async () => {
ses.webRequest.onBeforeRequest((details, callback) => {
callback({ cancel: false })
})
await ajax(defaultURL + 'serverRedirect')
await ajax(defaultURL + 'serverRedirect')
})
})
describe('webRequest.onBeforeSendHeaders', () => {