Bug 1231512 - Allow nsIHttpChannel.redirectTo() work also on an open channel, r=jduell
--- a/netwerk/protocol/http/HttpBaseChannel.cpp
+++ b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -1773,24 +1773,25 @@ HttpBaseChannel::GetRequestSucceeded(boo
if (!mResponseHead)
return NS_ERROR_NOT_AVAILABLE;
uint32_t status = mResponseHead->Status();
*aValue = (status / 100 == 2);
return NS_OK;
}
NS_IMETHODIMP
-HttpBaseChannel::RedirectTo(nsIURI *newURI)
+HttpBaseChannel::RedirectTo(nsIURI *targetURI)
{
- // We can only redirect unopened channels
- ENSURE_CALLED_BEFORE_CONNECT();
-
- // The redirect is stored internally for use in AsyncOpen
- mAPIRedirectToURI = newURI;
-
+ // We cannot redirect after OnStartRequest of the listener
+ // has been called, since to redirect we have to switch channels
+ // and the dance with OnStartRequest et al has to start over.
+ // This would break the nsIStreamListener contract.
+ NS_ENSURE_FALSE(mOnStartRequestCalled, NS_ERROR_NOT_AVAILABLE);
+
+ mAPIRedirectToURI = targetURI;
return NS_OK;
}
NS_IMETHODIMP
HttpBaseChannel::GetSchedulingContextID(nsID *aSCID)
{
NS_ENSURE_ARG_POINTER(aSCID);
*aSCID = mSchedulingContextID;
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -1563,16 +1563,49 @@ nsHttpChannel::ProcessResponse()
// reset the authentication's current continuation state because our
// last authentication attempt has been completed successfully
mAuthProvider->Disconnect(NS_ERROR_ABORT);
mAuthProvider = nullptr;
LOG((" continuation state has been reset"));
}
+ if (mAPIRedirectToURI && !mCanceled) {
+ MOZ_ASSERT(!mOnStartRequestCalled);
+ nsCOMPtr<nsIURI> redirectTo;
+ mAPIRedirectToURI.swap(redirectTo);
+
+ PushRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse1);
+ rv = StartRedirectChannelToURI(redirectTo, nsIChannelEventSink::REDIRECT_TEMPORARY);
+ if (NS_SUCCEEDED(rv)) {
+ return NS_OK;
+ }
+ PopRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse1);
+ }
+
+ // Hack: ContinueProcessResponse1 uses NS_OK to detect successful
+ // redirects, so we distinguish this codepath (a non-redirect that's
+ // processing normally) by passing in a bogus error code.
+ return ContinueProcessResponse1(NS_BINDING_FAILED);
+}
+
+nsresult
+nsHttpChannel::ContinueProcessResponse1(nsresult rv)
+{
+ if (NS_SUCCEEDED(rv)) {
+ // redirectTo() has passed through, we don't want to go on with
+ // this channel. It will now be canceled by the redirect handling
+ // code that called this function.
+ return NS_OK;
+ }
+
+ rv = NS_OK;
+
+ uint32_t httpStatus = mResponseHead->Status();
+
bool successfulReval = false;
// handle different server response categories. Note that we handle
// caching or not caching of error pages in
// nsHttpResponseHead::MustValidate; if you change this switch, update that
// one
switch (httpStatus) {
case 200:
@@ -1605,29 +1638,29 @@ nsHttpChannel::ProcessResponse()
case 307:
case 308:
case 303:
#if 0
case 305: // disabled as a security measure (see bug 187996).
#endif
// don't store the response body for redirects
MaybeInvalidateCacheEntryForSubsequentGet();
- PushRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse);
+ PushRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse2);
rv = AsyncProcessRedirection(httpStatus);
if (NS_FAILED(rv)) {
- PopRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse);
+ PopRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse2);
LOG(("AsyncProcessRedirection failed [rv=%x]\n", rv));
// don't cache failed redirect responses.
if (mCacheEntry)
mCacheEntry->AsyncDoom(nullptr);
if (DoNotRender3xxBody(rv)) {
mStatus = rv;
DoNotifyListener();
} else {
- rv = ContinueProcessResponse(rv);
+ rv = ContinueProcessResponse2(rv);
}
}
break;
case 304:
rv = ProcessNotModified();
if (NS_FAILED(rv)) {
LOG(("ProcessNotModified failed [rv=%x]\n", rv));
mCacheInputStream.CloseAndRelease();
@@ -1692,33 +1725,33 @@ nsHttpChannel::ProcessResponse()
AccumulateCacheHitTelemetry(cacheDisposition);
Telemetry::Accumulate(Telemetry::HTTP_RESPONSE_VERSION,
mResponseHead->Version());
return rv;
}
nsresult
-nsHttpChannel::ContinueProcessResponse(nsresult rv)
+nsHttpChannel::ContinueProcessResponse2(nsresult rv)
{
bool doNotRender = DoNotRender3xxBody(rv);
if (rv == NS_ERROR_DOM_BAD_URI && mRedirectURI) {
bool isHTTP = false;
if (NS_FAILED(mRedirectURI->SchemeIs("http", &isHTTP)))
isHTTP = false;
if (!isHTTP && NS_FAILED(mRedirectURI->SchemeIs("https", &isHTTP)))
isHTTP = false;
if (!isHTTP) {
// This was a blocked attempt to redirect and subvert the system by
// redirecting to another protocol (perhaps javascript:)
// In that case we want to throw an error instead of displaying the
// non-redirected response body.
- LOG(("ContinueProcessResponse detected rejected Non-HTTP Redirection"));
+ LOG(("ContinueProcessResponse2 detected rejected Non-HTTP Redirection"));
doNotRender = true;
rv = NS_ERROR_CORRUPTED_CONTENT;
}
}
if (doNotRender) {
Cancel(rv);
DoNotifyListener();
@@ -1734,17 +1767,17 @@ nsHttpChannel::ContinueProcessResponse(n
if (mApplicationCacheForWrite) {
// Store response in the offline cache
InitOfflineCacheEntry();
CloseOfflineCacheEntry();
}
return NS_OK;
}
- LOG(("ContinueProcessResponse got failure result [rv=%x]\n", rv));
+ LOG(("ContinueProcessResponse2 got failure result [rv=%x]\n", rv));
if (mTransaction->ProxyConnectFailed()) {
return ProcessFailedProxyConnect(mRedirectType);
}
return ProcessNormal();
}
nsresult
nsHttpChannel::ProcessNormal()
@@ -2004,28 +2037,34 @@ nsHttpChannel::StartRedirectChannelToURI
}
return rv;
}
nsresult
nsHttpChannel::ContinueAsyncRedirectChannelToURI(nsresult rv)
{
- if (NS_SUCCEEDED(rv))
+ // Since we handle mAPIRedirectToURI also after on-examine-response handler
+ // rather drop it here to avoid any redirect loops, even just hypothetical.
+ mAPIRedirectToURI = nullptr;
+
+ if (NS_SUCCEEDED(rv)) {
rv = OpenRedirectChannel(rv);
+ }
if (NS_FAILED(rv)) {
// Fill the failure status here, the update to https had been vetoed
// but from the security reasons we have to discard the whole channel
// load.
mStatus = rv;
}
- if (mLoadGroup)
+ if (mLoadGroup) {
mLoadGroup->RemoveRequest(this, nullptr, mStatus);
+ }
if (NS_FAILED(rv)) {
// We have to manually notify the listener because there is not any pump
// that would call our OnStart/StopRequest after resume from waiting for
// the redirect callback.
DoNotifyListener();
}
@@ -5698,16 +5737,18 @@ nsHttpChannel::OnStartSignedPackageReque
//-----------------------------------------------------------------------------
// nsHttpChannel::nsIRequestObserver
//-----------------------------------------------------------------------------
NS_IMETHODIMP
nsHttpChannel::OnStartRequest(nsIRequest *request, nsISupports *ctxt)
{
+ nsresult rv;
+
PROFILER_LABEL("nsHttpChannel", "OnStartRequest",
js::ProfileEntry::Category::NETWORK);
if (!(mCanceled || NS_FAILED(mStatus))) {
// capture the request's status, so our consumers will know ASAP of any
// connection failures, etc - bug 93581
request->GetStatus(&mStatus);
}
@@ -5752,45 +5793,80 @@ nsHttpChannel::OnStartRequest(nsIRequest
}
// avoid crashing if mListener happens to be null...
if (!mListener) {
NS_NOTREACHED("mListener is null");
return NS_OK;
}
+ // before we start any content load, check for redirectTo being called
+ // this code is executed mainly before we start load from the cache
+ if (mAPIRedirectToURI && !mCanceled) {
+ nsAutoCString redirectToSpec;
+ mAPIRedirectToURI->GetAsciiSpec(redirectToSpec);
+ LOG((" redirectTo called with uri=%s", redirectToSpec.BeginReading()));
+
+ MOZ_ASSERT(!mOnStartRequestCalled);
+
+ nsCOMPtr<nsIURI> redirectTo;
+ mAPIRedirectToURI.swap(redirectTo);
+
+ PushRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest1);
+ rv = StartRedirectChannelToURI(redirectTo, nsIChannelEventSink::REDIRECT_TEMPORARY);
+ if (NS_SUCCEEDED(rv)) {
+ return NS_OK;
+ }
+ PopRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest1);
+ }
+
+ // Hack: ContinueOnStartRequest1 uses NS_OK to detect successful redirects,
+ // so we distinguish this codepath (a non-redirect that's processing
+ // normally) by passing in a bogus error code.
+ return ContinueOnStartRequest1(NS_BINDING_FAILED);
+}
+
+nsresult
+nsHttpChannel::ContinueOnStartRequest1(nsresult result)
+{
+ if (NS_SUCCEEDED(result)) {
+ // Redirect has passed through, we don't want to go on with this
+ // channel. It will now be canceled by the redirect handling code
+ // that called this function.
+ return NS_OK;
+ }
+
// on proxy errors, try to failover
if (mConnectionInfo->ProxyInfo() &&
(mStatus == NS_ERROR_PROXY_CONNECTION_REFUSED ||
mStatus == NS_ERROR_UNKNOWN_PROXY_HOST ||
mStatus == NS_ERROR_NET_TIMEOUT)) {
- PushRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest1);
+ PushRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest2);
if (NS_SUCCEEDED(ProxyFailover()))
return NS_OK;
- PopRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest1);
- }
-
- return ContinueOnStartRequest2(NS_OK);
-}
-
-nsresult
-nsHttpChannel::ContinueOnStartRequest1(nsresult result)
-{
- // Success indicates we passed ProxyFailover, in that case we must not continue
- // with this code chain.
- if (NS_SUCCEEDED(result))
- return NS_OK;
-
- return ContinueOnStartRequest2(result);
+ PopRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest2);
+ }
+
+ // Hack: ContinueOnStartRequest2 uses NS_OK to detect successful redirects,
+ // so we distinguish this codepath (a non-redirect that's processing
+ // normally) by passing in a bogus error code.
+ return ContinueOnStartRequest2(NS_BINDING_FAILED);
}
nsresult
nsHttpChannel::ContinueOnStartRequest2(nsresult result)
{
+ if (NS_SUCCEEDED(result)) {
+ // Redirect has passed through, we don't want to go on with this
+ // channel. It will now be canceled by the redirect handling code
+ // that called this function.
+ return NS_OK;
+ }
+
// on other request errors, try to fall back
if (NS_FAILED(mStatus)) {
PushRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest3);
bool waitingForRedirectCallback;
ProcessFallback(&waitingForRedirectCallback);
if (waitingForRedirectCallback)
return NS_OK;
PopRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest3);
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -267,17 +267,18 @@ private:
nsresult ContinueBeginConnectWithResult();
void ContinueBeginConnect();
nsresult Connect();
void SpeculativeConnect();
nsresult SetupTransaction();
void SetupTransactionSchedulingContext();
nsresult CallOnStartRequest();
nsresult ProcessResponse();
- nsresult ContinueProcessResponse(nsresult);
+ nsresult ContinueProcessResponse1(nsresult);
+ nsresult ContinueProcessResponse2(nsresult);
nsresult ProcessNormal();
nsresult ContinueProcessNormal(nsresult);
void ProcessAltService();
nsresult ProcessNotModified();
nsresult AsyncProcessRedirection(uint32_t httpStatus);
nsresult ContinueProcessRedirection(nsresult);
nsresult ContinueProcessRedirectionAfterFallback(nsresult);
nsresult ProcessFailedProxyConnect(uint32_t httpStatus);
--- a/netwerk/protocol/http/nsIHttpChannel.idl
+++ b/netwerk/protocol/http/nsIHttpChannel.idl
@@ -365,23 +365,41 @@ interface nsIHttpChannel : nsIChannel
*
* @throws NS_ERROR_NOT_AVAILABLE if called before the response
* has been received (before onStartRequest).
*/
boolean isPrivateResponse();
/**
* Instructs the channel to immediately redirect to a new destination.
- * Can only be called on channels not yet opened.
+ * Can only be called on channels that have not yet called their
+ * listener's OnStartRequest(). Generally that means the latest time
+ * this can be used is one of:
+ * "http-on-examine-response"
+ * "http-on-examine-merged-response"
+ * "http-on-examine-cached-response"
+ *
+ * When non-null URL is set before AsyncOpen:
+ * we attempt to redirect to the targetURI before we even start building
+ * and sending the request to the cache or the origin server.
+ * If the redirect is vetoed, we fail the channel.
+ *
+ * When set between AsyncOpen and first call to OnStartRequest being called:
+ * we attempt to redirect before we start delivery of network or cached
+ * response to the listener. If vetoed, we continue with delivery of
+ * the original content to the channel listener.
+ *
+ * When passed aTargetURI is null the channel behaves normally (can be
+ * rewritten).
*
* This method provides no explicit conflict resolution. The last
* caller to call it wins.
*
- * @throws NS_ERROR_ALREADY_OPENED if called after the channel
- * has been opened.
+ * @throws NS_ERROR_NOT_AVAILABLE if called after the channel has already
+ * started to deliver the content to its listener.
*/
- void redirectTo(in nsIURI aNewURI);
+ void redirectTo(in nsIURI aTargetURI);
/**
* Identifies the scheduling context for this load.
*/
[noscript] attribute nsID schedulingContextID;
};
copy from netwerk/test/unit/test_redirect_from_script.js
copy to netwerk/test/unit/test_redirect_from_script_after-open_passing.js
--- a/netwerk/test/unit/test_redirect_from_script.js
+++ b/netwerk/test/unit/test_redirect_from_script_after-open_passing.js
@@ -23,17 +23,17 @@
*
*/
Cu.import("resource://testing-common/httpd.js");
Cu.import("resource://gre/modules/NetUtil.jsm");
// the topic we observe to use the API. http-on-opening-request might also
// work for some purposes.
-redirectHook = "http-on-modify-request";
+redirectHook = "http-on-examine-response";
var httpServer = null, httpServer2 = null;
XPCOMUtils.defineLazyGetter(this, "port1", function() {
return httpServer.identity.primaryPort;
});
XPCOMUtils.defineLazyGetter(this, "port2", function() {
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -258,16 +258,17 @@ skip-if = os == "win"
# Bug 675039: test fails consistently on Android
fail-if = os == "android"
[test_redirect-caching_passing.js]
[test_redirect_canceled.js]
[test_redirect_failure.js]
# Bug 675039: test fails consistently on Android
fail-if = os == "android"
[test_redirect_from_script.js]
+[test_redirect_from_script_after-open_passing.js]
[test_redirect_passing.js]
[test_redirect_loop.js]
[test_redirect_baduri.js]
[test_redirect_different-protocol.js]
[test_reentrancy.js]
[test_reopen.js]
[test_resumable_channel.js]
[test_resumable_truncate.js]