From fca965cb98046ed900df9db493eb338155f162d4 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 30 Jun 2021 12:59:27 -0700 Subject: [PATCH] browser(firefox): properly handle HSTS redirects (#7391) When Firefox decides to perform an http->https redirect based on HSTS information, it issues an "internal" redirect and cancels the http request. Since there is no response for the http request, we forge 307 redirect in this case, following Chromium lead. The relevant code is in nsHttpChannel::StartRedirectChannelToHttps. --- browser_patches/firefox-beta/BUILD_NUMBER | 4 +-- .../firefox-beta/juggler/NetworkObserver.js | 34 +++++++++---------- browser_patches/firefox/BUILD_NUMBER | 4 +-- .../firefox/juggler/NetworkObserver.js | 34 +++++++++---------- 4 files changed, 38 insertions(+), 38 deletions(-) diff --git a/browser_patches/firefox-beta/BUILD_NUMBER b/browser_patches/firefox-beta/BUILD_NUMBER index d30fa46dce..a9cec8b8c5 100644 --- a/browser_patches/firefox-beta/BUILD_NUMBER +++ b/browser_patches/firefox-beta/BUILD_NUMBER @@ -1,2 +1,2 @@ -1266 -Changed: max@schmitt.mx Tue Jun 29 22:44:52 UTC 2021 +1267 +Changed: dgozman@gmail.com Tue Jun 30 16:15:40 PDT 2021 diff --git a/browser_patches/firefox-beta/juggler/NetworkObserver.js b/browser_patches/firefox-beta/juggler/NetworkObserver.js index ba65a9619b..d10eeff64a 100644 --- a/browser_patches/firefox-beta/juggler/NetworkObserver.js +++ b/browser_patches/firefox-beta/juggler/NetworkObserver.js @@ -123,15 +123,8 @@ class NetworkRequest { this.requestId = httpChannel.channelId + ''; this.navigationId = httpChannel.isMainDocumentChannel ? this.requestId : undefined; - const internalCauseType = this.httpChannel.loadInfo ? this.httpChannel.loadInfo.internalContentPolicyType : Ci.nsIContentPolicy.TYPE_OTHER; - this._redirectedIndex = 0; - const ignoredRedirect = redirectedFrom && !redirectedFrom._sentOnResponse; - if (ignoredRedirect) { - // We just ignore redirect that did not hit the network before being redirected. - // This happens, for example, for automatic http->https redirects. - this.navigationId = redirectedFrom.navigationId; - } else if (redirectedFrom) { + if (redirectedFrom) { this.redirectedFromId = redirectedFrom.requestId; this._redirectedIndex = redirectedFrom._redirectedIndex + 1; this.requestId = this.requestId + '-redirect' + this._redirectedIndex; @@ -513,7 +506,7 @@ class NetworkRequest { }, this._frameId); } - _sendOnResponse(fromCache) { + _sendOnResponse(fromCache, opt_statusCode, opt_statusText) { if (this._sentOnResponse) { // We can come here twice because of internal redirects, e.g. service workers. return; @@ -537,8 +530,8 @@ class NetworkRequest { }; const headers = []; - let status = 0; - let statusText = ''; + let status = opt_statusCode || 0; + let statusText = opt_statusText || ''; try { status = this.httpChannel.responseStatus; statusText = this.httpChannel.responseStatusText; @@ -666,14 +659,21 @@ class NetworkObserver { return; const oldHttpChannel = oldChannel.QueryInterface(Ci.nsIHttpChannel); const newHttpChannel = newChannel.QueryInterface(Ci.nsIHttpChannel); - if (!(flags & Ci.nsIChannelEventSink.REDIRECT_INTERNAL)) { - const previous = this._channelToRequest.get(oldHttpChannel); - if (previous) - this._expectRedirect(newHttpChannel.channelId + '', previous); - } else { - const request = this._channelToRequest.get(oldHttpChannel); + const request = this._channelToRequest.get(oldHttpChannel); + if (flags & Ci.nsIChannelEventSink.REDIRECT_INTERNAL) { if (request) request._onInternalRedirect(newHttpChannel); + } else if (flags & Ci.nsIChannelEventSink.REDIRECT_STS_UPGRADE) { + if (request) { + // This is an internal HSTS upgrade. The original http request is canceled, and a new + // equivalent https request is sent. We forge 307 redirect to follow Chromium here: + // https://source.chromium.org/chromium/chromium/src/+/main:net/url_request/url_request_http_job.cc;l=211 + request._sendOnResponse(false, 307, 'Temporary Redirect'); + this._expectRedirect(newHttpChannel.channelId + '', request); + } + } else { + if (request) + this._expectRedirect(newHttpChannel.channelId + '', request); } } diff --git a/browser_patches/firefox/BUILD_NUMBER b/browser_patches/firefox/BUILD_NUMBER index c909c0b8f3..715f52bd8e 100644 --- a/browser_patches/firefox/BUILD_NUMBER +++ b/browser_patches/firefox/BUILD_NUMBER @@ -1,2 +1,2 @@ -1273 -Changed: max@schmitt.mx Tue Jun 29 22:39:37 UTC 2021 +1274 +Changed: dgozman@gmail.com Tue Jun 30 16:15:40 PDT 2021 diff --git a/browser_patches/firefox/juggler/NetworkObserver.js b/browser_patches/firefox/juggler/NetworkObserver.js index ba65a9619b..d10eeff64a 100644 --- a/browser_patches/firefox/juggler/NetworkObserver.js +++ b/browser_patches/firefox/juggler/NetworkObserver.js @@ -123,15 +123,8 @@ class NetworkRequest { this.requestId = httpChannel.channelId + ''; this.navigationId = httpChannel.isMainDocumentChannel ? this.requestId : undefined; - const internalCauseType = this.httpChannel.loadInfo ? this.httpChannel.loadInfo.internalContentPolicyType : Ci.nsIContentPolicy.TYPE_OTHER; - this._redirectedIndex = 0; - const ignoredRedirect = redirectedFrom && !redirectedFrom._sentOnResponse; - if (ignoredRedirect) { - // We just ignore redirect that did not hit the network before being redirected. - // This happens, for example, for automatic http->https redirects. - this.navigationId = redirectedFrom.navigationId; - } else if (redirectedFrom) { + if (redirectedFrom) { this.redirectedFromId = redirectedFrom.requestId; this._redirectedIndex = redirectedFrom._redirectedIndex + 1; this.requestId = this.requestId + '-redirect' + this._redirectedIndex; @@ -513,7 +506,7 @@ class NetworkRequest { }, this._frameId); } - _sendOnResponse(fromCache) { + _sendOnResponse(fromCache, opt_statusCode, opt_statusText) { if (this._sentOnResponse) { // We can come here twice because of internal redirects, e.g. service workers. return; @@ -537,8 +530,8 @@ class NetworkRequest { }; const headers = []; - let status = 0; - let statusText = ''; + let status = opt_statusCode || 0; + let statusText = opt_statusText || ''; try { status = this.httpChannel.responseStatus; statusText = this.httpChannel.responseStatusText; @@ -666,14 +659,21 @@ class NetworkObserver { return; const oldHttpChannel = oldChannel.QueryInterface(Ci.nsIHttpChannel); const newHttpChannel = newChannel.QueryInterface(Ci.nsIHttpChannel); - if (!(flags & Ci.nsIChannelEventSink.REDIRECT_INTERNAL)) { - const previous = this._channelToRequest.get(oldHttpChannel); - if (previous) - this._expectRedirect(newHttpChannel.channelId + '', previous); - } else { - const request = this._channelToRequest.get(oldHttpChannel); + const request = this._channelToRequest.get(oldHttpChannel); + if (flags & Ci.nsIChannelEventSink.REDIRECT_INTERNAL) { if (request) request._onInternalRedirect(newHttpChannel); + } else if (flags & Ci.nsIChannelEventSink.REDIRECT_STS_UPGRADE) { + if (request) { + // This is an internal HSTS upgrade. The original http request is canceled, and a new + // equivalent https request is sent. We forge 307 redirect to follow Chromium here: + // https://source.chromium.org/chromium/chromium/src/+/main:net/url_request/url_request_http_job.cc;l=211 + request._sendOnResponse(false, 307, 'Temporary Redirect'); + this._expectRedirect(newHttpChannel.channelId + '', request); + } + } else { + if (request) + this._expectRedirect(newHttpChannel.channelId + '', request); } }