Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Commit 88e9eb0

Browse files
author
Christoph Kerschbaumer
committed
Bug 1665062: HTTPS-Only: Upgraded website creating HTTP auth prompt gets interrupted by error-page r=necko-reviewers,dragana,JulianWels
Differential Revision: https://phabricator.services.mozilla.com/D91908
1 parent 0cc97d7 commit 88e9eb0

14 files changed

Lines changed: 180 additions & 5 deletions

‎dom/security/nsHTTPSOnlyUtils.cpp‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
#include "nsHTTPSOnlyUtils.h"
1313
#include "nsIConsoleService.h"
1414
#include "nsIHttpChannel.h"
15-
#include "nsIHttpChannel.h"
15+
#include "nsIHttpChannelInternal.h"
1616
#include "nsIHttpsOnlyModePermission.h"
1717
#include "nsIPermissionManager.h"
1818
#include "nsIPrincipal.h"
@@ -421,14 +421,21 @@ TestHTTPAnswerRunnable::OnStartRequest(nsIRequest* aRequest) {
421421
}
422422

423423
// Check if the original top-level channel which https-only is trying
424-
// to upgrade is already in progress. If it is, then all good, if not
424+
// to upgrade is already in progress or if the channel is an auth channel.
425+
// If it is in progress or Auth is in progress, then all good, if not
425426
// then let's cancel that channel so we can dispaly the exception page.
426427
nsCOMPtr<nsIChannel> httpsOnlyChannel = mDocumentLoadListener->GetChannel();
427428
if (httpsOnlyChannel) {
428429
nsCOMPtr<nsILoadInfo> loadInfo = httpsOnlyChannel->LoadInfo();
429-
uint32_t httpsOnlyStatus = loadInfo->GetHttpsOnlyStatus();
430-
if (!(httpsOnlyStatus &
431-
nsILoadInfo::HTTPS_ONLY_TOP_LEVEL_LOAD_IN_PROGRESS)) {
430+
uint32_t topLevelLoadInProgress =
431+
loadInfo->GetHttpsOnlyStatus() &
432+
nsILoadInfo::HTTPS_ONLY_TOP_LEVEL_LOAD_IN_PROGRESS;
433+
434+
nsCOMPtr<nsIHttpChannelInternal> httpChannelInternal =
435+
do_QueryInterface(httpsOnlyChannel);
436+
bool isAuthChannel = false;
437+
Unused << httpChannelInternal->GetIsAuthChannel(&isAuthChannel);
438+
if (!topLevelLoadInProgress && !isAuthChannel) {
432439
// Only really cancel the original top-level channel if it's
433440
// status is still NS_OK, otherwise it might have already
434441
// encountered some other error and was cancelled.

‎dom/security/test/https-only/file_http_background_auth_request.sjs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Custom *.sjs file specifically for the needs of Bug 1665062
2+
3+
function handleRequest(request, response) {
4+
// avoid confusing cache behaviors
5+
response.setHeader("Cache-Control", "no-cache", false);
6+
7+
if (request.scheme === "https") {
8+
response.setHeader("Content-Type", "text/html;charset=utf-8", false);
9+
response.setStatusLine(request.httpVersion, 401, "Unauthorized");
10+
response.setHeader("WWW-Authenticate", "Basic realm=\"bug1665062\"");
11+
return;
12+
}
13+
14+
// we should never get here; just in case, return something unexpected
15+
response.write("do'h");
16+
}

‎dom/security/test/https-only/mochitest.ini‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,5 @@ scheme=https
1414
fail-if = xorigin
1515
[test_http_background_request.html]
1616
support-files = file_http_background_request.sjs
17+
[test_http_background_auth_request.html]
18+
support-files = file_http_background_auth_request.sjs

‎dom/security/test/https-only/test_http_background_auth_request.html‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
<!DOCTYPE HTML>
2+
<html>
3+
<head>
4+
<title>Bug 1665062 - HTTPS-Only: Do not cancel channel if auth is in progress</title>
5+
<script src="/tests/SimpleTest/SimpleTest.js"></script>
6+
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
7+
</head>
8+
<body>
9+
10+
<script class="testbody" type="text/javascript">
11+
12+
/*
13+
* Description of the test:
14+
* We send a top-level request which results in a '401 - Unauthorized' and ensure that the
15+
* http background request does not accidentally treat that request as a potential timeout.
16+
* We make sure that ther HTTPS-Only Mode Error Page does *NOT* show up.
17+
*/
18+
19+
const {AppConstants} = SpecialPowers.Cu.import("resource://gre/modules/AppConstants.jsm", {});
20+
21+
SimpleTest.waitForExplicitFinish();
22+
SimpleTest.requestFlakyTimeout("When Auth is in progress, HTTPS-Only page should not show up");
23+
SimpleTest.requestLongerTimeout(10);
24+
25+
const EXPECTED_KICK_OFF_REQUEST =
26+
"http://test1.example.com/tests/dom/security/test/https-only/file_http_background_auth_request.sjs?foo";
27+
const EXPECTED_UPGRADE_REQUEST = EXPECTED_KICK_OFF_REQUEST.replace("http://", "https://");
28+
let EXPECTED_BG_REQUEST = "http://test1.example.com/";
29+
let requestCounter = 0;
30+
31+
function examiner() {
32+
SpecialPowers.addObserver(this, "specialpowers-http-notify-request");
33+
}
34+
examiner.prototype = {
35+
observe(subject, topic, data) {
36+
if (topic !== "specialpowers-http-notify-request") {
37+
return;
38+
}
39+
40+
// On Android we have other requests appear here as well. Let's make
41+
// sure we only evaluate requests triggered by the test.
42+
if (!data.startsWith("http://test1.example.com") &&
43+
!data.startsWith("https://test1.example.com")) {
44+
return;
45+
}
46+
++requestCounter;
47+
if (requestCounter == 1) {
48+
is(data, EXPECTED_KICK_OFF_REQUEST, "kick off request needs to be http");
49+
return;
50+
}
51+
if (requestCounter == 2) {
52+
is(data, EXPECTED_UPGRADE_REQUEST, "upgraded request needs to be https");
53+
return;
54+
}
55+
if (requestCounter == 3) {
56+
is(data, EXPECTED_BG_REQUEST, "background request needs to be http and no sensitive info");
57+
return;
58+
}
59+
ok(false, "we should never get here, but just in case");
60+
},
61+
remove() {
62+
SpecialPowers.removeObserver(this, "specialpowers-http-notify-request");
63+
}
64+
}
65+
window.AuthBackgroundRequestExaminer = new examiner();
66+
67+
// https-only top-level background request occurs after 3 seconds, hence
68+
// we use 4 seconds to make sure the background request did not happen.
69+
function resolveAfter4Seconds() {
70+
return new Promise(resolve => {
71+
setTimeout(() => {
72+
resolve();
73+
}, 4000);
74+
});
75+
}
76+
77+
async function runTests() {
78+
await SpecialPowers.pushPrefEnv({ set: [
79+
["dom.security.https_only_mode", true],
80+
["dom.security.https_only_mode_send_http_background_request", true],
81+
]});
82+
83+
let testWin = window.open(EXPECTED_KICK_OFF_REQUEST, "_blank");
84+
85+
// Give the Auth Process and background request some time before moving on.
86+
await resolveAfter4Seconds();
87+
88+
if (AppConstants.platform !== "android") {
89+
is(requestCounter, 3, "three requests total (kickoff, upgraded, background)");
90+
} else {
91+
// On Android, the auth request resolves and hence the background request
92+
// is not even kicked off - nevertheless, the error page should not appear!
93+
is(requestCounter, 2, "two requests total (kickoff, upgraded)");
94+
}
95+
96+
let wrappedWin = SpecialPowers.wrap(testWin);
97+
is(wrappedWin.document.body.innerHTML, "", "exception page should not be displayed");
98+
99+
testWin.close();
100+
101+
window.AuthBackgroundRequestExaminer.remove();
102+
SimpleTest.finish();
103+
}
104+
105+
runTests();
106+
107+
</script>
108+
</body>
109+
</html>

‎netwerk/protocol/http/ClassifierDummyChannel.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,11 @@ ClassifierDummyChannel::SetupFallbackChannel(const char* aFallbackKey) {
353353
return NS_ERROR_NOT_IMPLEMENTED;
354354
}
355355

356+
NS_IMETHODIMP
357+
ClassifierDummyChannel::GetIsAuthChannel(bool* aIsAuthChannel) {
358+
return NS_ERROR_NOT_IMPLEMENTED;
359+
}
360+
356361
NS_IMETHODIMP
357362
ClassifierDummyChannel::GetThirdPartyFlags(uint32_t* aThirdPartyFlags) {
358363
return NS_ERROR_NOT_IMPLEMENTED;

‎netwerk/protocol/http/HttpChannelChild.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2340,6 +2340,9 @@ HttpChannelChild::SetupFallbackChannel(const char* aFallbackKey) {
23402340
DROP_DEAD();
23412341
}
23422342

2343+
NS_IMETHODIMP
2344+
HttpChannelChild::GetIsAuthChannel(bool* aIsAuthChannel) { DROP_DEAD(); }
2345+
23432346
//-----------------------------------------------------------------------------
23442347
// HttpChannelChild::nsICacheInfoChannel
23452348
//-----------------------------------------------------------------------------

‎netwerk/protocol/http/HttpChannelChild.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ class HttpChannelChild final : public PHttpChannelChild,
101101
void DoDiagnosticAssertWhenOnStopNotCalledOnDestroy() override;
102102
// nsIHttpChannelInternal
103103
NS_IMETHOD SetupFallbackChannel(const char* aFallbackKey) override;
104+
NS_IMETHOD GetIsAuthChannel(bool* aIsAuthChannel) override;
104105
// nsISupportsPriority
105106
NS_IMETHOD SetPriority(int32_t value) override;
106107
// nsIClassOfService

‎netwerk/protocol/http/InterceptedHttpChannel.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,11 @@ InterceptedHttpChannel::SetupFallbackChannel(const char* aFallbackKey) {
552552
return NS_ERROR_NOT_IMPLEMENTED;
553553
}
554554

555+
NS_IMETHODIMP
556+
InterceptedHttpChannel::GetIsAuthChannel(bool* aIsAuthChannel) {
557+
return NS_ERROR_NOT_IMPLEMENTED;
558+
}
559+
555560
NS_IMETHODIMP
556561
InterceptedHttpChannel::SetPriority(int32_t aPriority) {
557562
mPriority = clamped<int32_t>(aPriority, INT16_MIN, INT16_MAX);

‎netwerk/protocol/http/InterceptedHttpChannel.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ class InterceptedHttpChannel final
159159
NS_IMETHOD
160160
SetupFallbackChannel(const char* aFallbackKey) override;
161161

162+
NS_IMETHOD
163+
GetIsAuthChannel(bool* aIsAuthChannel) override;
164+
162165
NS_IMETHOD
163166
SetPriority(int32_t aPriority) override;
164167

‎netwerk/protocol/http/TRRServiceChannel.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1275,6 +1275,11 @@ TRRServiceChannel::SetupFallbackChannel(const char* aFallbackKey) {
12751275
return NS_ERROR_NOT_IMPLEMENTED;
12761276
}
12771277

1278+
NS_IMETHODIMP
1279+
TRRServiceChannel::GetIsAuthChannel(bool* aIsAuthChannel) {
1280+
return NS_ERROR_NOT_IMPLEMENTED;
1281+
}
1282+
12781283
NS_IMETHODIMP
12791284
TRRServiceChannel::SetNotificationCallbacks(nsIInterfaceRequestor* aCallbacks) {
12801285
mCallbacks = aCallbacks;

0 commit comments

Comments
 (0)