Bug 1921480 - Make internalRedirectTo transparent r=kershaw,jdescottes,necko-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D224008
This commit is contained in:
Sean
2024-10-07 13:00:03 +00:00
parent 9f8635a31f
commit acd71b3ea7
10 changed files with 40 additions and 45 deletions

View File

@@ -67,7 +67,8 @@ nsresult nsAsyncRedirectVerifyHelper::Init(
: GetCurrentSerialEventTarget();
if (!(flags & (nsIChannelEventSink::REDIRECT_INTERNAL |
nsIChannelEventSink::REDIRECT_STS_UPGRADE))) {
nsIChannelEventSink::REDIRECT_STS_UPGRADE |
nsIChannelEventSink::REDIRECT_TRANSPARENT))) {
nsCOMPtr<nsILoadInfo> loadInfo = oldChan->LoadInfo();
if (loadInfo->GetDontFollowRedirects()) {
ExplicitCallback(NS_BINDING_ABORTED);
@@ -242,6 +243,12 @@ nsAsyncRedirectVerifyHelper::Run() {
return NS_OK;
}
// If transparent, avoid notifying the observers.
if (mFlags & nsIChannelEventSink::REDIRECT_TRANSPARENT) {
ExplicitCallback(NS_OK);
return NS_OK;
}
// First, the global observer
NS_ASSERTION(gIOService, "Must have an IO service at this point");
LOG(("nsAsyncRedirectVerifyHelper::Run() calling gIOService..."));

View File

@@ -60,6 +60,14 @@ interface nsIChannelEventSink : nsISupports
*/
const unsigned long REDIRECT_AUTH_RETRY = 1 << 4;
/**
* This is a special-case internal redirect triggered by
* transparentRedirectTo. The URL bar and window.location.href
* must remain unchanged, preserving the original request URI,
* while the content is served from the redirected URI.
*/
const unsigned long REDIRECT_TRANSPARENT = 1 << 5;
/**
* Called when a redirect occurs. This may happen due to an HTTP 3xx status
* code. The purpose of this method is to notify the sink that a redirect

View File

@@ -2300,8 +2300,8 @@ HttpBaseChannel::RedirectTo(nsIURI* targetURI) {
}
NS_IMETHODIMP
HttpBaseChannel::InternalRedirectTo(nsIURI* targetURI) {
LOG(("HttpBaseChannel::InternalRedirectTo [this=%p]", this));
HttpBaseChannel::TransparentRedirectTo(nsIURI* targetURI) {
LOG(("HttpBaseChannel::TransparentRedirectTo [this=%p]", this));
RedirectTo(targetURI);
MOZ_ASSERT(mAPIRedirectTo, "How did this happen?");
mAPIRedirectTo->second() = true;

View File

@@ -229,7 +229,7 @@ class HttpBaseChannel : public nsHashPropertyBag,
NS_IMETHOD GetResponseStatusText(nsACString& aValue) override;
NS_IMETHOD GetRequestSucceeded(bool* aValue) override;
NS_IMETHOD RedirectTo(nsIURI* newURI) override;
NS_IMETHOD InternalRedirectTo(nsIURI* newURI) override;
NS_IMETHOD TransparentRedirectTo(nsIURI* newURI) override;
NS_IMETHOD UpgradeToSecure() override;
NS_IMETHOD GetRequestObserversCalled(bool* aCalled) override;
NS_IMETHOD SetRequestObserversCalled(bool aCalled) override;
@@ -732,7 +732,7 @@ class HttpBaseChannel : public nsHashPropertyBag,
nsCOMPtr<nsIProgressEventSink> mProgressSink;
nsCOMPtr<nsIReferrerInfo> mReferrerInfo;
// The first parameter is the URI we would like to redirect to
// The second parameter should be true if internal redirect otherwise false
// The second parameter should be true if trasparent redirect otherwise false
// mAPIRedirectTo is Nothing if and only if the URI is null.
mozilla::Maybe<mozilla::CompactPair<nsCOMPtr<nsIURI>, bool>> mAPIRedirectTo;
nsCOMPtr<nsIURI> mProxyURI;

View File

@@ -2586,7 +2586,7 @@ HttpChannelChild::RedirectTo(nsIURI* newURI) {
}
NS_IMETHODIMP
HttpChannelChild::InternalRedirectTo(nsIURI* newURI) {
HttpChannelChild::TransparentRedirectTo(nsIURI* newURI) {
return NS_ERROR_NOT_IMPLEMENTED;
}

View File

@@ -94,7 +94,7 @@ class HttpChannelChild final : public PHttpChannelChild,
const nsACString& aValue, bool aMerge) override;
NS_IMETHOD SetEmptyRequestHeader(const nsACString& aHeader) override;
NS_IMETHOD RedirectTo(nsIURI* newURI) override;
NS_IMETHOD InternalRedirectTo(nsIURI* newURI) override;
NS_IMETHOD TransparentRedirectTo(nsIURI* newURI) override;
NS_IMETHOD UpgradeToSecure() override;
NS_IMETHOD GetProtocolVersion(nsACString& aProtocolVersion) override;
void DoDiagnosticAssertWhenOnStopNotCalledOnDestroy() override;

View File

@@ -1089,12 +1089,13 @@ nsresult nsHttpChannel::Connect() {
LOG(("nsHttpChannel::Connect [this=%p]\n", this));
if (mAPIRedirectTo) {
LOG(("nsHttpChannel::Connect [internal=%d]\n", mAPIRedirectTo->second()));
LOG(("nsHttpChannel::Connect [transparent=%d]\n",
mAPIRedirectTo->second()));
nsresult rv = StartRedirectChannelToURI(
mAPIRedirectTo->first(),
mAPIRedirectTo->second() ? nsIChannelEventSink::REDIRECT_PERMANENT |
nsIChannelEventSink::REDIRECT_INTERNAL
nsIChannelEventSink::REDIRECT_TRANSPARENT
: nsIChannelEventSink::REDIRECT_PERMANENT);
mAPIRedirectTo = Nothing();
if (NS_SUCCEEDED(rv)) {
@@ -2783,7 +2784,7 @@ nsresult nsHttpChannel::ContinueProcessResponse2(nsresult rv) {
rv = StartRedirectChannelToURI(
mAPIRedirectTo->first(),
mAPIRedirectTo->second() ? nsIChannelEventSink::REDIRECT_TEMPORARY |
nsIChannelEventSink::REDIRECT_INTERNAL
nsIChannelEventSink::REDIRECT_TRANSPARENT
: nsIChannelEventSink::REDIRECT_TEMPORARY);
mAPIRedirectTo = Nothing();
if (NS_SUCCEEDED(rv)) {
@@ -3351,10 +3352,10 @@ void nsHttpChannel::HandleAsyncAPIRedirect() {
}
nsresult rv = StartRedirectChannelToURI(
mAPIRedirectTo->first(), mAPIRedirectTo->second()
? nsIChannelEventSink::REDIRECT_PERMANENT |
nsIChannelEventSink::REDIRECT_INTERNAL
: nsIChannelEventSink::REDIRECT_PERMANENT);
mAPIRedirectTo->first(),
mAPIRedirectTo->second() ? nsIChannelEventSink::REDIRECT_PERMANENT |
nsIChannelEventSink::REDIRECT_TRANSPARENT
: nsIChannelEventSink::REDIRECT_PERMANENT);
if (NS_FAILED(rv)) {
rv = ContinueAsyncRedirectChannelToURI(rv);
if (NS_FAILED(rv)) {
@@ -7957,7 +7958,7 @@ nsresult nsHttpChannel::ContinueOnStartRequest1(nsresult result) {
rv = StartRedirectChannelToURI(
mAPIRedirectTo->first(),
mAPIRedirectTo->second() ? nsIChannelEventSink::REDIRECT_TEMPORARY |
nsIChannelEventSink::REDIRECT_INTERNAL
nsIChannelEventSink::REDIRECT_TRANSPARENT
: nsIChannelEventSink::REDIRECT_TEMPORARY);
mAPIRedirectTo = Nothing();
if (NS_SUCCEEDED(rv)) {

View File

@@ -542,9 +542,9 @@ interface nsIHttpChannelInternal : nsISupports
/**
* Same as redirectTo in nsIHttpChannel, but handles internal redirect
* so that we add REDIRECT_INTERNAL to the flag.
* so that we add REDIRECT_TRANSPARENT to the flag.
*/
[must_use] void internalRedirectTo(in nsIURI aTargetURI);
[must_use] void transparentRedirectTo(in nsIURI aTargetURI);
/**

View File

@@ -29,10 +29,6 @@ function onBeforeConnect(callback) {
}
class EventSinkListener {
constructor(internalRedirect) {
this.internalRedirect = internalRedirect;
}
getInterface(iid) {
if (iid.equals(Ci.nsIChannelEventSink)) {
return this;
@@ -40,13 +36,8 @@ class EventSinkListener {
throw Components.Exception("", Cr.NS_ERROR_NO_INTERFACE);
}
asyncOnChannelRedirect(oldChan, newChan, flags, callback) {
Assert.ok(
!!(flags & Ci.nsIChannelEventSink.REDIRECT_INTERNAL) ===
this.internalRedirect,
`REDIRECT_INTERNAL flag should ${
this.internalRedirect ? "be" : "not be"
} set`
);
// if transparent, asyncOnChannelRedirect should not be called.
Assert.ok(false);
callback.onRedirectVerifyCallback(Cr.NS_OK);
}
}
@@ -56,7 +47,7 @@ EventSinkListener.prototype.QueryInterface = ChromeUtils.generateQI([
"nsIChannelEventSink",
]);
async function test_redirect(internalRedirect) {
add_task(async function test_transparent_redirect() {
var server = new HttpServer();
await server.start(-1);
registerCleanupFunction(async () => {
@@ -72,11 +63,7 @@ async function test_redirect(internalRedirect) {
chan.suspend();
Promise.resolve().then(() => {
try {
if (internalRedirect) {
chan.internalRedirectTo(Services.io.newURI(successUrl));
} else {
chan.redirectTo(Services.io.newURI(successUrl));
}
chan.transparentRedirectTo(Services.io.newURI(successUrl));
} catch (e) {
do_throw(e);
}
@@ -88,7 +75,7 @@ async function test_redirect(internalRedirect) {
uri: baseUrl,
loadUsingSystemPrincipal: true,
}).QueryInterface(Ci.nsIHttpChannelInternal);
let listener = new EventSinkListener(internalRedirect);
let listener = new EventSinkListener();
chan.notificationCallbacks = listener;
await new Promise(resolve => {
@@ -104,12 +91,4 @@ async function test_redirect(internalRedirect) {
)
);
});
}
add_task(async function test_internal_redirect() {
await test_redirect(true);
});
add_task(async function test_normal_redirect() {
await test_redirect(false);
});

View File

@@ -853,8 +853,6 @@ run-sequentially = "node server exceptions dont replay well"
["test_inhibit_caching.js"]
["test_internal_redirect.js"]
["test_ioservice.js"]
["test_large_port.js"]
@@ -1171,6 +1169,8 @@ firefox-appdir = "browser"
["test_trackingProtection_annotateChannels.js"]
["test_transparent_redirect.js"]
["test_trr.js"]
head = "head_channels.js head_cache.js head_cookies.js head_servers.js head_trr.js head_http3.js trr_common.js"
run-sequentially = "very high failure rate in parallel"