diff --git a/shell/browser/net/electron_url_loader_factory.cc b/shell/browser/net/electron_url_loader_factory.cc index 9ce6d2030a74..ae8259dbd136 100644 --- a/shell/browser/net/electron_url_loader_factory.cc +++ b/shell/browser/net/electron_url_loader_factory.cc @@ -170,6 +170,83 @@ void OnWrite(std::unique_ptr write_data, MojoResult result) { } // namespace +ElectronURLLoaderFactory::RedirectedRequest::RedirectedRequest( + const net::RedirectInfo& redirect_info, + mojo::PendingReceiver loader_receiver, + int32_t request_id, + uint32_t options, + const network::ResourceRequest& request, + mojo::PendingRemote client, + const net::MutableNetworkTrafficAnnotationTag& traffic_annotation, + mojo::PendingRemote target_factory_remote) + : redirect_info_(redirect_info), + request_id_(request_id), + options_(options), + request_(request), + client_(std::move(client)), + traffic_annotation_(traffic_annotation) { + loader_receiver_.Bind(std::move(loader_receiver)); + loader_receiver_.set_disconnect_handler( + base::BindOnce(&ElectronURLLoaderFactory::RedirectedRequest::DeleteThis, + base::Unretained(this))); + target_factory_remote_.Bind(std::move(target_factory_remote)); + target_factory_remote_.set_disconnect_handler(base::BindOnce( + &ElectronURLLoaderFactory::RedirectedRequest::OnTargetFactoryError, + base::Unretained(this))); +} + +ElectronURLLoaderFactory::RedirectedRequest::~RedirectedRequest() = default; + +void ElectronURLLoaderFactory::RedirectedRequest::FollowRedirect( + const std::vector& removed_headers, + const net::HttpRequestHeaders& modified_headers, + const net::HttpRequestHeaders& modified_cors_exempt_headers, + const absl::optional& new_url) { + // Update |request_| with info from the redirect, so that it's accurate + // The following references code in WorkerScriptLoader::FollowRedirect + bool should_clear_upload = false; + net::RedirectUtil::UpdateHttpRequest( + request_.url, request_.method, redirect_info_, removed_headers, + modified_headers, &request_.headers, &should_clear_upload); + request_.cors_exempt_headers.MergeFrom(modified_cors_exempt_headers); + for (const std::string& name : removed_headers) + request_.cors_exempt_headers.RemoveHeader(name); + + if (should_clear_upload) + request_.request_body = nullptr; + + request_.url = redirect_info_.new_url; + request_.method = redirect_info_.new_method; + request_.site_for_cookies = redirect_info_.new_site_for_cookies; + request_.referrer = GURL(redirect_info_.new_referrer); + request_.referrer_policy = redirect_info_.new_referrer_policy; + + // Create a new loader to process the redirect and destroy this one + target_factory_remote_->CreateLoaderAndStart( + loader_receiver_.Unbind(), request_id_, options_, request_, + std::move(client_), traffic_annotation_); + + DeleteThis(); +} + +void ElectronURLLoaderFactory::RedirectedRequest::OnTargetFactoryError() { + // Can't create a new loader at this point, so the request can't continue + mojo::Remote client_remote( + std::move(client_)); + client_remote->OnComplete( + network::URLLoaderCompletionStatus(net::ERR_FAILED)); + client_remote.reset(); + + DeleteThis(); +} + +void ElectronURLLoaderFactory::RedirectedRequest::DeleteThis() { + loader_receiver_.reset(); + target_factory_remote_.reset(); + + delete this; +} + // static mojo::PendingRemote ElectronURLLoaderFactory::Create(ProtocolType type, @@ -202,12 +279,18 @@ void ElectronURLLoaderFactory::CreateLoaderAndStart( mojo::PendingRemote client, const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - mojo::PendingRemote proxy_factory; + + // |StartLoading| is used for both intercepted and registered protocols, + // and on redirects it needs a factory to use to create a loader for the + // new request. So in this case, this factory is the target factory. + mojo::PendingRemote target_factory; + this->Clone(target_factory.InitWithNewPipeAndPassReceiver()); + handler_.Run( request, base::BindOnce(&ElectronURLLoaderFactory::StartLoading, std::move(loader), request_id, options, request, std::move(client), - traffic_annotation, std::move(proxy_factory), type_)); + traffic_annotation, std::move(target_factory), type_)); } // static @@ -230,7 +313,7 @@ void ElectronURLLoaderFactory::StartLoading( const network::ResourceRequest& request, mojo::PendingRemote client, const net::MutableNetworkTrafficAnnotationTag& traffic_annotation, - mojo::PendingRemote proxy_factory, + mojo::PendingRemote target_factory, ProtocolType type, gin::Arguments* args) { // Send network error when there is no argument passed. @@ -280,13 +363,6 @@ void ElectronURLLoaderFactory::StartLoading( request.url.Resolve(location), net::RedirectUtil::GetReferrerPolicyHeader(head->headers.get()), false); - network::ResourceRequest new_request = request; - new_request.method = redirect_info.new_method; - new_request.url = redirect_info.new_url; - new_request.site_for_cookies = redirect_info.new_site_for_cookies; - new_request.referrer = GURL(redirect_info.new_referrer); - new_request.referrer_policy = redirect_info.new_referrer_policy; - DCHECK(client.is_valid()); mojo::Remote client_remote( @@ -294,33 +370,15 @@ void ElectronURLLoaderFactory::StartLoading( client_remote->OnReceiveRedirect(redirect_info, std::move(head)); - // Unbound client, so it an be passed to sub-methods - client = client_remote.Unbind(); - // When the redirection comes from an intercepted scheme (which has - // |proxy_factory| passed), we ask the proxy factory to create a loader - // for new URL, otherwise we call |StartLoadingHttp|, which creates - // loader with default factory. - // - // Note that when handling requests for intercepted scheme, creating loader - // with default factory (i.e. calling StartLoadingHttp) would bypass the - // ProxyingURLLoaderFactory, we have to explicitly use the proxy factory to - // create loader so it is possible to have handlers of intercepted scheme - // getting called recursively, which is a behavior expected in protocol - // module. - // - // I'm not sure whether this is an intended behavior in Chromium. - if (proxy_factory.is_valid()) { - mojo::Remote proxy_factory_remote( - std::move(proxy_factory)); + // Bind the URLLoader receiver and wait for a FollowRedirect request, or for + // the remote to disconnect, which will happen if the request is aborted. + // That may happen when the redirect is to a different scheme, which will + // cause the URL loader to be destroyed and a new one created using the + // factory for that scheme. + new RedirectedRequest(redirect_info, std::move(loader), request_id, options, + request, client_remote.Unbind(), traffic_annotation, + std::move(target_factory)); - proxy_factory_remote->CreateLoaderAndStart( - std::move(loader), request_id, options, new_request, - std::move(client), traffic_annotation); - } else { - StartLoadingHttp(std::move(loader), new_request, std::move(client), - traffic_annotation, - gin::Dictionary::CreateEmpty(args->isolate())); - } return; } @@ -360,7 +418,7 @@ void ElectronURLLoaderFactory::StartLoading( } StartLoading(std::move(loader), request_id, options, request, std::move(client), traffic_annotation, - std::move(proxy_factory), type, args); + std::move(target_factory), type, args); break; } } diff --git a/shell/browser/net/electron_url_loader_factory.h b/shell/browser/net/electron_url_loader_factory.h index 7d26e3328573..b31be2a7cb52 100644 --- a/shell/browser/net/electron_url_loader_factory.h +++ b/shell/browser/net/electron_url_loader_factory.h @@ -8,6 +8,7 @@ #include #include #include +#include #include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_remote.h" @@ -15,9 +16,11 @@ #include "mojo/public/cpp/bindings/remote.h" #include "net/url_request/url_request_job_factory.h" #include "services/network/public/cpp/self_deleting_url_loader_factory.h" +#include "services/network/public/mojom/url_loader.mojom.h" #include "services/network/public/mojom/url_loader_factory.mojom.h" #include "services/network/public/mojom/url_response_head.mojom.h" #include "shell/common/gin_helper/dictionary.h" +#include "third_party/abseil-cpp/absl/types/optional.h" namespace electron { @@ -43,6 +46,52 @@ using HandlersMap = // Implementation of URLLoaderFactory. class ElectronURLLoaderFactory : public network::SelfDeletingURLLoaderFactory { public: + // This class binds a URLLoader receiver in the case of a redirect, waiting + // for |FollowRedirect| to be called at which point the new request will be + // started, and the receiver will be unbound letting a new URLLoader bind it + class RedirectedRequest : public network::mojom::URLLoader { + public: + RedirectedRequest( + const net::RedirectInfo& redirect_info, + mojo::PendingReceiver loader_receiver, + int32_t request_id, + uint32_t options, + const network::ResourceRequest& request, + mojo::PendingRemote client, + const net::MutableNetworkTrafficAnnotationTag& traffic_annotation, + mojo::PendingRemote + target_factory_remote); + ~RedirectedRequest() override; + + // network::mojom::URLLoader: + void FollowRedirect( + const std::vector& removed_headers, + const net::HttpRequestHeaders& modified_headers, + const net::HttpRequestHeaders& modified_cors_exempt_headers, + const absl::optional& new_url) override; + void SetPriority(net::RequestPriority priority, + int32_t intra_priority_value) override {} + void PauseReadingBodyFromNet() override {} + void ResumeReadingBodyFromNet() override {} + + void OnTargetFactoryError(); + void DeleteThis(); + + private: + net::RedirectInfo redirect_info_; + + mojo::Receiver loader_receiver_{this}; + int32_t request_id_; + uint32_t options_; + network::ResourceRequest request_; + mojo::PendingRemote client_; + net::MutableNetworkTrafficAnnotationTag traffic_annotation_; + + mojo::Remote target_factory_remote_; + + DISALLOW_COPY_AND_ASSIGN(RedirectedRequest); + }; + static mojo::PendingRemote Create( ProtocolType type, const ProtocolHandler& handler); @@ -64,7 +113,7 @@ class ElectronURLLoaderFactory : public network::SelfDeletingURLLoaderFactory { const network::ResourceRequest& request, mojo::PendingRemote client, const net::MutableNetworkTrafficAnnotationTag& traffic_annotation, - mojo::PendingRemote proxy_factory, + mojo::PendingRemote target_factory, ProtocolType type, gin::Arguments* args); diff --git a/spec-main/api-protocol-spec.ts b/spec-main/api-protocol-spec.ts index 119fbe8cf07f..4f92a046b33f 100644 --- a/spec-main/api-protocol-spec.ts +++ b/spec-main/api-protocol-spec.ts @@ -120,6 +120,24 @@ describe('protocol module', () => { const r = await ajax(protocolName + '://fake-host'); expect(r.data).to.equal(text); }); + + it('can redirect to the same scheme', async () => { + registerStringProtocol(protocolName, (request, callback) => { + if (request.url === `${protocolName}://fake-host/redirect`) { + callback({ + statusCode: 302, + headers: { + Location: `${protocolName}://fake-host` + } + }); + } else { + expect(request.url).to.equal(`${protocolName}://fake-host`); + callback('redirected'); + } + }); + const r = await ajax(`${protocolName}://fake-host/redirect`); + expect(r.data).to.equal('redirected'); + }); }); describe('protocol.unregisterProtocol', () => { diff --git a/spec-main/api-web-request-spec.ts b/spec-main/api-web-request-spec.ts index 2eb0d7b3f5c2..472e9a011d4a 100644 --- a/spec-main/api-web-request-spec.ts +++ b/spec-main/api-web-request-spec.ts @@ -181,6 +181,42 @@ describe('webRequest module', () => { expect(data).to.equal('/header/received'); }); + it('can change the request headers on a custom protocol redirect', async () => { + protocol.registerStringProtocol('custom-scheme', (req, callback) => { + if (req.url === 'custom-scheme://fake-host/redirect') { + callback({ + statusCode: 302, + headers: { + Location: 'custom-scheme://fake-host' + } + }); + } else { + let content = ''; + if (req.headers.Accept === '*/*;test/header') { + content = 'header-received'; + } + callback(content); + } + }); + + // Note that we need to do navigation every time after a protocol is + // registered or unregistered, otherwise the new protocol won't be + // recognized by current page when NetworkService is used. + await contents.loadFile(path.join(__dirname, 'fixtures', 'pages', 'jquery.html')); + + try { + ses.webRequest.onBeforeSendHeaders((details, callback) => { + const requestHeaders = details.requestHeaders; + requestHeaders.Accept = '*/*;test/header'; + callback({ requestHeaders: requestHeaders }); + }); + const { data } = await ajax('custom-scheme://fake-host/redirect'); + expect(data).to.equal('header-received'); + } finally { + protocol.unregisterProtocol('custom-scheme'); + } + }); + it('can change request origin', async () => { ses.webRequest.onBeforeSendHeaders((details, callback) => { const requestHeaders = details.requestHeaders;