diff --git a/shell/browser/api/atom_api_web_request.cc b/shell/browser/api/atom_api_web_request.cc index acac3564a4b7..48c7ea3389fd 100644 --- a/shell/browser/api/atom_api_web_request.cc +++ b/shell/browser/api/atom_api_web_request.cc @@ -332,6 +332,10 @@ void WebRequest::OnCompleted(extensions::WebRequestInfo* info, HandleSimpleEvent(kOnCompleted, info, request, net_error); } +void WebRequest::OnRequestWillBeDestroyed(extensions::WebRequestInfo* info) { + callbacks_.erase(info->id); +} + template void WebRequest::SetSimpleListener(gin::Arguments* args) { SetListener(event, &simple_listeners_, args); diff --git a/shell/browser/api/atom_api_web_request.h b/shell/browser/api/atom_api_web_request.h index bd7bbf62429a..38feefe17665 100644 --- a/shell/browser/api/atom_api_web_request.h +++ b/shell/browser/api/atom_api_web_request.h @@ -84,6 +84,7 @@ class WebRequest : public gin::Wrappable, public WebRequestAPI { void OnCompleted(extensions::WebRequestInfo* info, const network::ResourceRequest& request, int net_error) override; + void OnRequestWillBeDestroyed(extensions::WebRequestInfo* info) override; enum SimpleEvent { kOnSendHeaders, diff --git a/shell/browser/net/proxying_url_loader_factory.cc b/shell/browser/net/proxying_url_loader_factory.cc index accb63bea9d0..949cc2295a36 100644 --- a/shell/browser/net/proxying_url_loader_factory.cc +++ b/shell/browser/net/proxying_url_loader_factory.cc @@ -49,9 +49,10 @@ ProxyingURLLoaderFactory::InProgressRequest::InProgressRequest( target_client_(std::move(client)), current_response_(network::mojom::URLResponseHead::New()), proxied_client_binding_(this), - // TODO(zcbenz): We should always use "extraHeaders" mode to be compatible - // with old APIs. - has_any_extra_headers_listeners_(false) { + // Always use "extraHeaders" mode to be compatible with old APIs, except + // when the |request_id_| is zero, which is not supported in Chromium and + // only happens in Electron when the request is started from net module. + has_any_extra_headers_listeners_(network_service_request_id != 0) { // If there is a client error, clean up the request. target_client_.set_connection_error_handler(base::BindOnce( &ProxyingURLLoaderFactory::InProgressRequest::OnRequestError, @@ -60,7 +61,19 @@ ProxyingURLLoaderFactory::InProgressRequest::InProgressRequest( } ProxyingURLLoaderFactory::InProgressRequest::~InProgressRequest() { - // TODO(zcbenz): Do cleanup here. + // This is important to ensure that no outstanding blocking requests continue + // to reference state owned by this object. + if (info_) { + factory_->web_request_api()->OnRequestWillBeDestroyed(&info_.value()); + } + if (on_before_send_headers_callback_) { + std::move(on_before_send_headers_callback_) + .Run(net::ERR_ABORTED, base::nullopt); + } + if (on_headers_received_callback_) { + std::move(on_headers_received_callback_) + .Run(net::ERR_ABORTED, base::nullopt, base::nullopt); + } } void ProxyingURLLoaderFactory::InProgressRequest::Restart() { @@ -84,8 +97,7 @@ void ProxyingURLLoaderFactory::InProgressRequest::UpdateRequestInfo() { current_request_uses_header_client_ = factory_->url_loader_header_client_receiver_.is_bound() && - network_service_request_id_ != 0 && - false /* TODO(zcbenz): HasExtraHeadersListenerForRequest */; + network_service_request_id_ != 0 && has_any_extra_headers_listeners_; } void ProxyingURLLoaderFactory::InProgressRequest::RestartInternal() { @@ -722,6 +734,10 @@ void ProxyingURLLoaderFactory::CreateLoaderAndStart( // don't use it for identity here. const uint64_t web_request_id = ++g_request_id; + // Notes: Chromium assumes that requests with zero-ID would never use the + // "extraHeaders" code path, however in Electron requests started from + // the net module would have zero-ID because they do not have renderer process + // associated. if (request_id) network_request_id_to_web_request_id_.emplace(request_id, web_request_id); diff --git a/shell/browser/net/proxying_url_loader_factory.h b/shell/browser/net/proxying_url_loader_factory.h index 2ce6937840e6..068b9008759d 100644 --- a/shell/browser/net/proxying_url_loader_factory.h +++ b/shell/browser/net/proxying_url_loader_factory.h @@ -67,6 +67,7 @@ class WebRequestAPI { virtual void OnCompleted(extensions::WebRequestInfo* info, const network::ResourceRequest& request, int net_error) = 0; + virtual void OnRequestWillBeDestroyed(extensions::WebRequestInfo* info) = 0; }; // This class is responsible for following tasks when NetworkService is enabled: diff --git a/spec-main/api-web-request-spec.ts b/spec-main/api-web-request-spec.ts index 948dffd1c2b3..6cb29249672f 100644 --- a/spec-main/api-web-request-spec.ts +++ b/spec-main/api-web-request-spec.ts @@ -20,6 +20,9 @@ describe('webRequest module', () => { if (req.headers.accept === '*/*;test/header') { content += 'header/received' } + if (req.headers.origin === 'http://new-origin') { + content += 'new/origin' + } res.end(content) } }) @@ -145,6 +148,16 @@ describe('webRequest module', () => { expect(data).to.equal('/header/received') }) + it('can change CORS headers', async () => { + ses.webRequest.onBeforeSendHeaders((details, callback) => { + const requestHeaders = details.requestHeaders + requestHeaders.Origin = 'http://new-origin' + callback({ requestHeaders: requestHeaders }) + }) + const { data } = await ajax(defaultURL) + expect(data).to.equal('/new/origin') + }) + it('resets the whole headers', async () => { const requestHeaders = { Test: 'header' @@ -199,6 +212,16 @@ describe('webRequest module', () => { expect(headers).to.match(/^custom: Changed$/m) }) + it('can change CORS headers', async () => { + ses.webRequest.onHeadersReceived((details, callback) => { + const responseHeaders = details.responseHeaders! + responseHeaders['access-control-allow-origin'] = ['http://new-origin'] as any + callback({ responseHeaders: responseHeaders }) + }) + const { headers } = await ajax(defaultURL) + expect(headers).to.match(/^access-control-allow-origin: http:\/\/new-origin$/m) + }) + it('does not change header by default', async () => { ses.webRequest.onHeadersReceived((details, callback) => { callback({})