Skip to content

Commit

Permalink
fix: do not cancel CORS preflight request on proxy auth. (#29266)
Browse files Browse the repository at this point in the history
* fix: do not cancel CORS preflight request on proxy auth.

If connecting via proxy, preflight request can receive 407
header response from proxy. This does not mean request
was finished even though it received headers (from proxy,
not the destination server), so prevent "completing"
and most importantly deleting it, which causes request
to be canceled in network layer. Just continue to monitor it
and await proper response from server. Also add circut breaker
to cancel request if proxy auth failed 3 times (for example
user keeps cancelling auth). This behavior happens only
when app registered WebRequest api listeners.

* Port chromium webrequest changes to electron code.

Move relevant parts of chromium WebRequestProxyingURLLoaderFactory from
https://chromium-review.googlesource.com/c/chromium/src/+/2011781
into electron ProxyingURLLoaderFactory.

* Update code to upstreamed version and remove retyr count failsafe.

Co-authored-by: Milan Burda <milan.burda@gmail.com>
  • Loading branch information
marekharanczyk and miniak committed Jun 21, 2021
1 parent d4a1b41 commit 507cbdc
Showing 1 changed file with 15 additions and 3 deletions.
18 changes: 15 additions & 3 deletions shell/browser/net/proxying_url_loader_factory.cc
Expand Up @@ -326,6 +326,14 @@ bool ProxyingURLLoaderFactory::IsForServiceWorkerScript() const {

void ProxyingURLLoaderFactory::InProgressRequest::OnLoaderCreated(
mojo::PendingReceiver<network::mojom::TrustedHeaderClient> receiver) {
// When CORS is involved there may be multiple network::URLLoader associated
// with this InProgressRequest, because CorsURLLoader may create a new
// network::URLLoader for the same request id in redirect handling - see
// CorsURLLoader::FollowRedirect. In such a case the old network::URLLoader
// is going to be detached fairly soon, so we don't need to take care of it.
// We need this explicit reset to avoid a DCHECK failure in mojo::Receiver.
header_client_receiver_.reset();

header_client_receiver_.Bind(std::move(receiver));
if (for_cors_preflight_) {
// In this case we don't have |target_loader_| and
Expand Down Expand Up @@ -600,13 +608,17 @@ void ProxyingURLLoaderFactory::InProgressRequest::
override_headers_ = nullptr;

if (for_cors_preflight_) {
// If this is for CORS preflight, there is no associated client. We notify
// the completion here, and deletes |this|.
// If this is for CORS preflight, there is no associated client.
info_->AddResponseInfoFromResourceResponse(*current_response_);
// Do not finish proxied preflight requests that require proxy auth.
// The request is not finished yet, give control back to network service
// which will start authentication process.
if (info_->response_code == net::HTTP_PROXY_AUTHENTICATION_REQUIRED)
return;
// We notify the completion here, and delete |this|.
factory_->web_request_api()->OnResponseStarted(&info_.value(), request_);
factory_->web_request_api()->OnCompleted(&info_.value(), request_, net::OK);

// Deletes |this|.
factory_->RemoveRequest(network_service_request_id_, request_id_);
return;
}
Expand Down

0 comments on commit 507cbdc

Please sign in to comment.