Skip to content

Commit 5364c6a

Browse files
authored
browser(firefox): fix automatic http->https redirect (microsoft#3812)
browser(firefox): fix automatic http->https redirect Sometimes, Firefox does an automatic http->https redirect without hitting the network (e.g. for http://wikipedia.org). In this case, the http request is very strange: - it does not actually hit the network; - it is never intercepted; - we cannot access its response because there was no actual response. So, we had a bug where: - redirects inherited the original request's listener; - that listener was throwing an error. This lead to the error in the listeners onDataAvailable call chain, and original listener that renders the response was never called, resulting in an empty page. This change: - ignores the original request that did not hit the network; - does not inherit the listener; - adds try/catch around problematic calls.
1 parent 3c69f2a commit 5364c6a

File tree

3 files changed

+31
-14
lines changed

3 files changed

+31
-14
lines changed
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
1169
2-
Changed: [email protected] Tue Sep 1 17:07:52 PDT 2020
1+
1170
2+
Changed: [email protected] Tue Sep 8 22:17:56 PDT 2020

browser_patches/firefox/juggler/NetworkObserver.js

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,22 @@ class NetworkRequest {
131131
this.requestId = httpChannel.channelId + '';
132132
this.navigationId = httpChannel.isMainDocumentChannel ? this.requestId : undefined;
133133

134+
const internalCauseType = this.httpChannel.loadInfo ? this.httpChannel.loadInfo.internalContentPolicyType : Ci.nsIContentPolicy.TYPE_OTHER;
135+
this.channelKey = this.httpChannel.channelId + ':' + internalCauseType;
136+
134137
this._redirectedIndex = 0;
135-
if (redirectedFrom) {
138+
const ignoredRedirect = redirectedFrom && !redirectedFrom._sentOnResponse;
139+
if (ignoredRedirect) {
140+
// We just ignore redirect that did not hit the network before being redirected.
141+
// This happens, for example, for automatic http->https redirects.
142+
this.navigationId = redirectedFrom.navigationId;
143+
this.channelKey = redirectedFrom.channelKey;
144+
} else if (redirectedFrom) {
136145
this.redirectedFromId = redirectedFrom.requestId;
137146
this._redirectedIndex = redirectedFrom._redirectedIndex + 1;
138147
this.requestId = this.requestId + '-redirect' + this._redirectedIndex;
139148
this.navigationId = redirectedFrom.navigationId;
149+
this.channelKey = redirectedFrom.channelKey;
140150
// Finish previous request now. Since we inherit the listener, we could in theory
141151
// use onStopRequest, but that will only happen after the last redirect has finished.
142152
redirectedFrom._sendOnRequestFinished();
@@ -157,7 +167,7 @@ class NetworkRequest {
157167

158168
httpChannel.QueryInterface(Ci.nsITraceableChannel);
159169
this._originalListener = httpChannel.setNewListener(this);
160-
if (redirectedFrom && this._originalListener === redirectedFrom) {
170+
if (redirectedFrom) {
161171
// Listener is inherited for regular redirects, so we'd like to avoid
162172
// calling into previous NetworkRequest.
163173
this._originalListener = redirectedFrom._originalListener;
@@ -492,7 +502,7 @@ class NetworkRequest {
492502
navigationId: this.navigationId,
493503
cause: causeTypeToString(causeType),
494504
internalCause: causeTypeToString(internalCauseType),
495-
}, this.httpChannel.channelId + ':' + internalCauseType);
505+
}, this.channelKey);
496506
}
497507

498508
_sendOnResponse(fromCache) {
@@ -506,10 +516,20 @@ class NetworkRequest {
506516
return;
507517

508518
this.httpChannel.QueryInterface(Ci.nsIHttpChannelInternal);
519+
509520
const headers = [];
510-
this.httpChannel.visitResponseHeaders({
511-
visitHeader: (name, value) => headers.push({name, value}),
512-
});
521+
let status = 0;
522+
let statusText = '';
523+
try {
524+
status = this.httpChannel.responseStatus;
525+
statusText = this.httpChannel.responseStatusText;
526+
this.httpChannel.visitResponseHeaders({
527+
visitHeader: (name, value) => headers.push({name, value}),
528+
});
529+
} catch (e) {
530+
// Response headers, status and/or statusText are not available
531+
// when redirect did not actually hit the network.
532+
}
513533

514534
let remoteIPAddress = undefined;
515535
let remotePort = undefined;
@@ -527,8 +547,8 @@ class NetworkRequest {
527547
headers,
528548
remoteIPAddress,
529549
remotePort,
530-
status: this.httpChannel.responseStatus,
531-
statusText: this.httpChannel.responseStatusText,
550+
status,
551+
statusText,
532552
});
533553
}
534554

browser_patches/firefox/juggler/protocol/NetworkHandler.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ class NetworkHandler {
2323
this._requestInterception = false;
2424
this._eventListeners = [];
2525
this._pendingRequstWillBeSentEvents = new Set();
26-
this._requestIdToFrameId = new Map();
2726
}
2827

2928
async enable() {
@@ -126,9 +125,7 @@ class NetworkHandler {
126125
this._pendingRequstWillBeSentEvents.delete(pendingRequestPromise);
127126
return;
128127
}
129-
// Inherit frameId for redirects when details are not available.
130-
const frameId = details ? details.frameId : (eventDetails.redirectedFrom ? this._requestIdToFrameId.get(eventDetails.redirectedFrom) : undefined);
131-
this._requestIdToFrameId.set(eventDetails.requestId, frameId);
128+
const frameId = details ? details.frameId : undefined;
132129
const activity = this._ensureHTTPActivity(eventDetails.requestId);
133130
activity.request = {
134131
frameId,

0 commit comments

Comments
 (0)