From 2d386ee8e7a6f487eec9a105300983126152b352 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Fri, 10 Dec 2021 13:11:58 -0800 Subject: [PATCH] browser(firefox): fix proxy auth redirect + resource redirect handling (#10854) This was breaking a vital invariant in our firefox network code - see comments. References #10095 --- browser_patches/firefox-beta/BUILD_NUMBER | 4 ++-- .../firefox-beta/juggler/NetworkObserver.js | 14 +++++++++++++- browser_patches/firefox/BUILD_NUMBER | 4 ++-- browser_patches/firefox/juggler/NetworkObserver.js | 14 +++++++++++++- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/browser_patches/firefox-beta/BUILD_NUMBER b/browser_patches/firefox-beta/BUILD_NUMBER index 6589aabad5..1a47f322ec 100644 --- a/browser_patches/firefox-beta/BUILD_NUMBER +++ b/browser_patches/firefox-beta/BUILD_NUMBER @@ -1,2 +1,2 @@ -1307 -Changed: lushnikov@chromium.org Mon Dec 6 15:17:29 PST 2021 +1308 +Changed: lushnikov@chromium.org Fri Dec 10 02:42:14 PST 2021 diff --git a/browser_patches/firefox-beta/juggler/NetworkObserver.js b/browser_patches/firefox-beta/juggler/NetworkObserver.js index c2669ca852..10ecca520b 100644 --- a/browser_patches/firefox-beta/juggler/NetworkObserver.js +++ b/browser_patches/firefox-beta/juggler/NetworkObserver.js @@ -104,7 +104,6 @@ class NetworkRequest { constructor(networkObserver, httpChannel, redirectedFrom) { this._networkObserver = networkObserver; this.httpChannel = httpChannel; - this._networkObserver._channelToRequest.set(this.httpChannel, this); const loadInfo = this.httpChannel.loadInfo; let browsingContext = loadInfo?.frameBrowsingContext || loadInfo?.browsingContext; @@ -133,6 +132,19 @@ class NetworkRequest { // use onStopRequest, but that will only happen after the last redirect has finished. redirectedFrom._sendOnRequestFinished(); } + // In case of proxy auth, we get two requests with the same channel: + // - one is pre-auth + // - second is with auth header. + // + // In this case, we create this NetworkRequest object with a `redirectedFrom` + // object, and they both share the same httpChannel. + // + // Since we want to maintain _channelToRequest map without clashes, + // we must call `_sendOnRequestFinished` **before** we update it with a new object + // here. + if (this._networkObserver._channelToRequest.has(this.httpChannel)) + throw new Error(`Internal Error: invariant is broken for _channelToRequest map`); + this._networkObserver._channelToRequest.set(this.httpChannel, this); this._pageNetwork = redirectedFrom ? redirectedFrom._pageNetwork : networkObserver._findPageNetwork(httpChannel); this._expectingInterception = false; diff --git a/browser_patches/firefox/BUILD_NUMBER b/browser_patches/firefox/BUILD_NUMBER index e77cba43eb..dd4ec122ca 100644 --- a/browser_patches/firefox/BUILD_NUMBER +++ b/browser_patches/firefox/BUILD_NUMBER @@ -1,2 +1,2 @@ -1308 -Changed: lushnikov@chromium.org Tue Dec 7 11:51:20 PST 2021 +1309 +Changed: lushnikov@chromium.org Fri Dec 10 02:42:14 PST 2021 diff --git a/browser_patches/firefox/juggler/NetworkObserver.js b/browser_patches/firefox/juggler/NetworkObserver.js index c2669ca852..10ecca520b 100644 --- a/browser_patches/firefox/juggler/NetworkObserver.js +++ b/browser_patches/firefox/juggler/NetworkObserver.js @@ -104,7 +104,6 @@ class NetworkRequest { constructor(networkObserver, httpChannel, redirectedFrom) { this._networkObserver = networkObserver; this.httpChannel = httpChannel; - this._networkObserver._channelToRequest.set(this.httpChannel, this); const loadInfo = this.httpChannel.loadInfo; let browsingContext = loadInfo?.frameBrowsingContext || loadInfo?.browsingContext; @@ -133,6 +132,19 @@ class NetworkRequest { // use onStopRequest, but that will only happen after the last redirect has finished. redirectedFrom._sendOnRequestFinished(); } + // In case of proxy auth, we get two requests with the same channel: + // - one is pre-auth + // - second is with auth header. + // + // In this case, we create this NetworkRequest object with a `redirectedFrom` + // object, and they both share the same httpChannel. + // + // Since we want to maintain _channelToRequest map without clashes, + // we must call `_sendOnRequestFinished` **before** we update it with a new object + // here. + if (this._networkObserver._channelToRequest.has(this.httpChannel)) + throw new Error(`Internal Error: invariant is broken for _channelToRequest map`); + this._networkObserver._channelToRequest.set(this.httpChannel, this); this._pageNetwork = redirectedFrom ? redirectedFrom._pageNetwork : networkObserver._findPageNetwork(httpChannel); this._expectingInterception = false;