Bug 1322355 - Cancel http:// channel when secure update (redirect) to https:// is vetoed to avoid duplicate OnStartRequest notification + added logs. r=michal
☠☠ backed out by bc401cae3a90 ☠ ☠
authorHonza Bambas <honzab.moz@firemni.cz>
Tue, 21 Feb 2017 11:30:00 -0500
changeset 373183 a93c0f43ccf8027aaa191fe644186769a00fd675
parent 373182 fc14157949746f4f5b902e2f1500589a5f60e401
child 373184 2c3443470801b458558cf6503eca739f5a618f72
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmichal
bugs1322355
milestone54.0a1
Bug 1322355 - Cancel http:// channel when secure update (redirect) to https:// is vetoed to avoid duplicate OnStartRequest notification + added logs. r=michal
netwerk/protocol/http/HttpBaseChannel.cpp
netwerk/protocol/http/HttpChannelParentListener.cpp
netwerk/protocol/http/nsHttpChannel.cpp
--- a/netwerk/protocol/http/HttpBaseChannel.cpp
+++ b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -2081,16 +2081,22 @@ HttpBaseChannel::GetRequestSucceeded(boo
   uint32_t status = mResponseHead->Status();
   *aValue = (status / 100 == 2);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 HttpBaseChannel::RedirectTo(nsIURI *targetURI)
 {
+  NS_ENSURE_ARG(targetURI);
+
+  nsAutoCString spec;
+  targetURI->GetAsciiSpec(spec);
+  LOG(("HttpBaseChannel::RedirectTo [this=%p, uri=%s]", this, spec.get()));
+
   // 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;
@@ -2964,16 +2970,18 @@ HttpBaseChannel::ReleaseListeners()
   mCallbacks = nullptr;
   mProgressSink = nullptr;
   mCompressListener = nullptr;
 }
 
 void
 HttpBaseChannel::DoNotifyListener()
 {
+  LOG(("HttpBaseChannel::DoNotifyListener this=%p", this));
+
   if (mListener) {
     MOZ_ASSERT(!mOnStartRequestCalled,
                "We should not call OnStartRequest twice");
 
     nsCOMPtr<nsIStreamListener> listener = mListener;
     listener->OnStartRequest(this, mListenerContext);
 
     mOnStartRequestCalled = true;
--- a/netwerk/protocol/http/HttpChannelParentListener.cpp
+++ b/netwerk/protocol/http/HttpChannelParentListener.cpp
@@ -25,16 +25,18 @@ namespace net {
 
 HttpChannelParentListener::HttpChannelParentListener(HttpChannelParent* aInitialChannel)
   : mNextListener(aInitialChannel)
   , mRedirectChannelId(0)
   , mSuspendedForDiversion(false)
   , mShouldIntercept(false)
   , mShouldSuspendIntercept(false)
 {
+  LOG(("HttpChannelParentListener::HttpChannelParentListener [this=%p, next=%p]",
+       this, aInitialChannel));
 }
 
 HttpChannelParentListener::~HttpChannelParentListener()
 {
 }
 
 //-----------------------------------------------------------------------------
 // HttpChannelParentListener::nsISupports
@@ -155,16 +157,19 @@ HttpChannelParentListener::GetInterface(
 
 NS_IMETHODIMP
 HttpChannelParentListener::AsyncOnChannelRedirect(
                                     nsIChannel *oldChannel,
                                     nsIChannel *newChannel,
                                     uint32_t redirectFlags,
                                     nsIAsyncVerifyRedirectCallback* callback)
 {
+  LOG(("HttpChannelParentListener::AsyncOnChannelRedirect [this=%p, old=%p, new=%p, flags=%u]",
+       this, oldChannel, newChannel, redirectFlags));
+
   nsresult rv;
 
   nsCOMPtr<nsIParentRedirectingChannel> activeRedirectingChannel =
       do_QueryInterface(mNextListener);
   if (!activeRedirectingChannel) {
     NS_ERROR("Channel got a redirect response, but doesn't implement "
              "nsIParentRedirectingChannel to handle it.");
     return NS_ERROR_NOT_IMPLEMENTED;
@@ -188,16 +193,19 @@ HttpChannelParentListener::AsyncOnChanne
 
 //-----------------------------------------------------------------------------
 // HttpChannelParentListener::nsIRedirectResultListener
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 HttpChannelParentListener::OnRedirectResult(bool succeeded)
 {
+  LOG(("HttpChannelParentListener::OnRedirectResult [this=%p, suc=%d]",
+       this, succeeded));
+
   nsresult rv;
 
   nsCOMPtr<nsIParentChannel> redirectChannel;
   if (mRedirectChannelId) {
     nsCOMPtr<nsIRedirectChannelRegistrar> registrar =
         do_GetService("@mozilla.org/redirectchannelregistrar;1", &rv);
     NS_ENSURE_SUCCESS(rv, rv);
 
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -1262,16 +1262,18 @@ EnsureMIMEOfScript(nsIURI* aURI, nsHttpR
     Telemetry::Accumulate(Telemetry::SCRIPT_BLOCK_INCORRECT_MIME, 0);
     return NS_OK;
 }
 
 
 nsresult
 nsHttpChannel::CallOnStartRequest()
 {
+    LOG(("nsHttpChannel::CallOnStartRequest [this=%p]", this));
+
     MOZ_RELEASE_ASSERT(!(mRequireCORSPreflight &&
                          mInterceptCache != INTERCEPTED) ||
                        mIsCorsPreflightDone,
                        "CORS preflight must have been finished by the time we "
                        "call OnStartRequest");
 
     nsresult rv = EnsureMIMEOfScript(mURI, mResponseHead, mLoadInfo);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -1355,17 +1357,17 @@ nsHttpChannel::CallOnStartRequest()
         if (NS_ERROR_FILE_TOO_BIG == rv) {
           // Don't throw the entry away, we will need it later.
           LOG(("  entry too big"));
         } else {
           NS_ENSURE_SUCCESS(rv, rv);
         }
     }
 
-    LOG(("  calling mListener->OnStartRequest\n"));
+    LOG(("  calling mListener->OnStartRequest [this=%p, listener=%p]\n", this, mListener.get()));
     if (mListener) {
         MOZ_ASSERT(!mOnStartRequestCalled,
                    "We should not call OsStartRequest twice");
         nsCOMPtr<nsIStreamListener> deleteProtector(mListener);
         rv = deleteProtector->OnStartRequest(this, mListenerContext);
         mOnStartRequestCalled = true;
         if (NS_FAILED(rv))
             return rv;
@@ -2147,16 +2149,18 @@ nsHttpChannel::ContinueProcessResponse1(
     // redirects, so we distinguish this codepath (a non-redirect that's
     // processing normally) by passing in a bogus error code.
     return ContinueProcessResponse2(NS_BINDING_FAILED);
 }
 
 nsresult
 nsHttpChannel::ContinueProcessResponse2(nsresult rv)
 {
+    LOG(("nsHttpChannel::ContinueProcessResponse1 [this=%p, rv=0x%08x]", this, 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;
@@ -2404,16 +2408,18 @@ nsHttpChannel::ProcessNormal()
     }
 
     return ContinueProcessNormal(NS_OK);
 }
 
 nsresult
 nsHttpChannel::ContinueProcessNormal(nsresult rv)
 {
+    LOG(("nsHttpChannel::ContinueProcessNormal [this=%p]", this));
+
     if (NS_FAILED(rv)) {
         // Fill the failure status here, we have failed to fall back, thus we
         // have to report our status as failed.
         mStatus = rv;
         DoNotifyListener();
         return rv;
     }
 
@@ -2641,36 +2647,38 @@ nsHttpChannel::StartRedirectChannelToURI
     }
 
     return rv;
 }
 
 nsresult
 nsHttpChannel::ContinueAsyncRedirectChannelToURI(nsresult rv)
 {
+    LOG(("nsHttpChannel::ContinueAsyncRedirectChannelToURI [this=%p]", this));
+
     // 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
+        // Cancel the channel here, the update to https had been vetoed
         // but from the security reasons we have to discard the whole channel
         // load.
-        mStatus = rv;
+        Cancel(rv);
     }
 
     if (mLoadGroup) {
         mLoadGroup->RemoveRequest(this, nullptr, mStatus);
     }
 
-    if (NS_FAILED(rv)) {
+    if (NS_FAILED(rv) && !mCachePump && !mTransactionPump) {
         // 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();
     }
 
     return rv;
 }
@@ -6742,16 +6750,18 @@ nsHttpChannel::ContinueOnStartRequest2(n
     }
 
     return ContinueOnStartRequest3(NS_OK);
 }
 
 nsresult
 nsHttpChannel::ContinueOnStartRequest3(nsresult result)
 {
+    LOG(("nsHttpChannel::ContinueOnStartRequest3 [this=%p]", this));
+
     if (mFallingBack)
         return NS_OK;
 
     return CallOnStartRequest();
 }
 
 NS_IMETHODIMP
 nsHttpChannel::OnStopRequest(nsIRequest *request, nsISupports *ctxt, nsresult status)