From 8d9097c85cd381ff4d03c853cc20e70f7bdcabe0 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 20 Jan 2020 16:42:00 +0900 Subject: [PATCH 1/2] fix: pass navigation_ui_data to proxying factory --- shell/browser/atom_browser_client.cc | 15 ++++++++++++++- shell/browser/net/proxying_url_loader_factory.cc | 9 +++++++-- shell/browser/net/proxying_url_loader_factory.h | 2 ++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/shell/browser/atom_browser_client.cc b/shell/browser/atom_browser_client.cc index 9427cb5f34ed5..64c97827b32c0 100644 --- a/shell/browser/atom_browser_client.cc +++ b/shell/browser/atom_browser_client.cc @@ -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" @@ -1092,6 +1093,17 @@ bool AtomBrowserClient::WillCreateURLLoaderFactory( mojo::PendingRemote 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 navigation_ui_data; + if (navigation_id.has_value()) { + navigation_ui_data = + std::make_unique(); + } + mojo::PendingReceiver header_client_receiver; if (header_client) @@ -1099,7 +1111,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); diff --git a/shell/browser/net/proxying_url_loader_factory.cc b/shell/browser/net/proxying_url_loader_factory.cc index a144a767fbf17..19196fa5dd06a 100644 --- a/shell/browser/net/proxying_url_loader_factory.cc +++ b/shell/browser/net/proxying_url_loader_factory.cc @@ -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(headers); + current_response_->remote_endpoint = remote_endpoint; HandleResponseOrRedirectHeaders( base::BindOnce(&InProgressRequest::ContinueToHandleOverrideHeaders, weak_factory_.GetWeakPtr())); @@ -674,6 +677,7 @@ ProxyingURLLoaderFactory::ProxyingURLLoaderFactory( const HandlersMap& intercepted_handlers, content::BrowserContext* browser_context, int render_process_id, + std::unique_ptr navigation_ui_data, base::Optional navigation_id, network::mojom::URLLoaderFactoryRequest loader_request, mojo::PendingRemote target_factory_remote, @@ -684,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)); diff --git a/shell/browser/net/proxying_url_loader_factory.h b/shell/browser/net/proxying_url_loader_factory.h index 26897204b876f..65f367c1ac99c 100644 --- a/shell/browser/net/proxying_url_loader_factory.h +++ b/shell/browser/net/proxying_url_loader_factory.h @@ -210,6 +210,7 @@ class ProxyingURLLoaderFactory const HandlersMap& intercepted_handlers, content::BrowserContext* browser_context, int render_process_id, + std::unique_ptr navigation_ui_data, base::Optional navigation_id, network::mojom::URLLoaderFactoryRequest loader_request, mojo::PendingRemote @@ -268,6 +269,7 @@ class ProxyingURLLoaderFactory content::BrowserContext* const browser_context_; const int render_process_id_; + std::unique_ptr navigation_ui_data_; base::Optional navigation_id_; mojo::ReceiverSet proxy_receivers_; mojo::Remote target_factory_; From 2f1e71f82ccb58d11adca77ff5bd01890551f6f5 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 20 Jan 2020 17:15:20 +0900 Subject: [PATCH 2/2] fix: clone response instead of move in redirect --- shell/browser/net/proxying_url_loader_factory.cc | 5 ++--- spec-main/api-web-request-spec.ts | 8 ++++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/shell/browser/net/proxying_url_loader_factory.cc b/shell/browser/net/proxying_url_loader_factory.cc index 19196fa5dd06a..24a9b4fd33318 100644 --- a/shell/browser/net/proxying_url_loader_factory.cc +++ b/shell/browser/net/proxying_url_loader_factory.cc @@ -523,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( @@ -541,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; diff --git a/spec-main/api-web-request-spec.ts b/spec-main/api-web-request-spec.ts index e17a52c309114..4f0c31d0b476e 100644 --- a/spec-main/api-web-request-spec.ts +++ b/spec-main/api-web-request-spec.ts @@ -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', () => {