Bug 1333142 - audit asyncopen2 impls for callback release on failure r=dragana a=gchang
authorPatrick McManus <mcmanus@ducksong.com>
Wed, 08 Feb 2017 12:55:15 -0500
changeset 378402 dde4995469df1c12a38015558652d1c2d4774f1a
parent 378401 447a9c40939c591b7bda5ca763a69f1f32ac87d6
child 378403 1acbb1168e116fd67dacbd2f740f4c28cffc8a05
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana, gchang
bugs1333142
milestone53.0a2
Bug 1333142 - audit asyncopen2 impls for callback release on failure r=dragana a=gchang
image/decoders/icon/mac/nsIconChannelCocoa.mm
image/decoders/icon/win/nsIconChannel.cpp
modules/libjar/nsJARChannel.cpp
netwerk/base/nsBaseChannel.cpp
netwerk/protocol/wyciwyg/WyciwygChannelChild.cpp
netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp
uriloader/exthandler/nsExternalProtocolHandler.cpp
--- a/image/decoders/icon/mac/nsIconChannelCocoa.mm
+++ b/image/decoders/icon/mac/nsIconChannelCocoa.mm
@@ -225,41 +225,52 @@ nsIconChannel::AsyncOpen(nsIStreamListen
              mLoadInfo->GetSecurityMode() == 0 ||
              mLoadInfo->GetInitialSecurityCheckDone() ||
              (mLoadInfo->GetSecurityMode() == nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL &&
               nsContentUtils::IsSystemPrincipal(mLoadInfo->LoadingPrincipal())),
              "security flags in loadInfo but asyncOpen2() not called");
 
   nsCOMPtr<nsIInputStream> inStream;
   nsresult rv = MakeInputStream(getter_AddRefs(inStream), true);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_FAILED(rv)) {
+      mCallbacks = nullptr;
+      return rv;
+  }
 
   // Init our stream pump
   rv = mPump->Init(inStream, int64_t(-1), int64_t(-1), 0, 0, false);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_FAILED(rv)) {
+      mCallbacks = nullptr;
+      return rv;
+  }
 
   rv = mPump->AsyncRead(this, ctxt);
   if (NS_SUCCEEDED(rv)) {
     // Store our real listener
     mListener = aListener;
     // Add ourself to the load group, if available
     if (mLoadGroup) {
       mLoadGroup->AddRequest(this, nullptr);
     }
+  } else {
+      mCallbacks = nullptr;
   }
 
   return rv;
 }
 
 NS_IMETHODIMP
 nsIconChannel::AsyncOpen2(nsIStreamListener* aListener)
 {
   nsCOMPtr<nsIStreamListener> listener = aListener;
   nsresult rv = nsContentSecurityManager::doContentSecurityCheck(this, listener);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_FAILED(rv)) {
+      mCallbacks = nullptr;
+      return rv;
+  }
   return AsyncOpen(listener, nullptr);
 }
 
 nsresult
 nsIconChannel::MakeInputStream(nsIInputStream** _retval,
                                         bool aNonBlocking)
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;
--- a/image/decoders/icon/win/nsIconChannel.cpp
+++ b/image/decoders/icon/win/nsIconChannel.cpp
@@ -240,43 +240,51 @@ nsIconChannel::AsyncOpen(nsIStreamListen
              mLoadInfo->GetInitialSecurityCheckDone() ||
              (mLoadInfo->GetSecurityMode() == nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL &&
               nsContentUtils::IsSystemPrincipal(mLoadInfo->LoadingPrincipal())),
              "security flags in loadInfo but asyncOpen2() not called");
 
   nsCOMPtr<nsIInputStream> inStream;
   nsresult rv = MakeInputStream(getter_AddRefs(inStream), true);
   if (NS_FAILED(rv)) {
+    mCallbacks = nullptr;
     return rv;
   }
 
   // Init our streampump
   rv = mPump->Init(inStream, int64_t(-1), int64_t(-1), 0, 0, false);
   if (NS_FAILED(rv)) {
+    mCallbacks = nullptr;
     return rv;
   }
 
   rv = mPump->AsyncRead(this, ctxt);
   if (NS_SUCCEEDED(rv)) {
     // Store our real listener
     mListener = aListener;
     // Add ourself to the load group, if available
     if (mLoadGroup) {
       mLoadGroup->AddRequest(this, nullptr);
     }
+  } else {
+    mCallbacks = nullptr;
   }
+
   return rv;
 }
 
 NS_IMETHODIMP
 nsIconChannel::AsyncOpen2(nsIStreamListener* aListener)
 {
   nsCOMPtr<nsIStreamListener> listener = aListener;
   nsresult rv = nsContentSecurityManager::doContentSecurityCheck(this, listener);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_FAILED(rv)) {
+    mCallbacks = nullptr;
+    return rv;
+  }
   return AsyncOpen(listener, nullptr);
 }
 
 static DWORD
 GetSpecialFolderIcon(nsIFile* aFile, int aFolder,
                                   SHFILEINFOW* aSFI, UINT aInfoFlags)
 {
   DWORD shellResult = 0;
--- a/modules/libjar/nsJARChannel.cpp
+++ b/modules/libjar/nsJARChannel.cpp
@@ -772,27 +772,34 @@ nsJARChannel::AsyncOpen(nsIStreamListene
     mListenerContext = ctx;
     mIsPending = true;
 
     nsresult rv = LookupFile(true);
     if (NS_FAILED(rv)) {
         mIsPending = false;
         mListenerContext = nullptr;
         mListener = nullptr;
+        mCallbacks = nullptr;
+        mProgressSink = nullptr;
         return rv;
     }
 
     nsCOMPtr<nsIChannel> channel;
 
     if (!mJarFile) {
         // Not a local file...
 
         // Check preferences to see if all remote jar support should be disabled
         if (mBlockRemoteFiles) {
             mIsUnsafe = true;
+            mIsPending = false;
+            mListenerContext = nullptr;
+            mListener = nullptr;
+            mCallbacks = nullptr;
+            mProgressSink = nullptr;
             return NS_ERROR_UNSAFE_CONTENT_TYPE;
         }
 
         static bool reportedRemoteJAR = false;
         if (!reportedRemoteJAR) {
             reportedRemoteJAR = true;
             Telemetry::Accumulate(Telemetry::REMOTE_JAR_PROTOCOL_USED, 1);
         }
@@ -806,16 +813,18 @@ nsJARChannel::AsyncOpen(nsIStreamListene
                                    mLoadInfo,
                                    mLoadGroup,
                                    mCallbacks,
                                    loadFlags);
         if (NS_FAILED(rv)) {
             mIsPending = false;
             mListenerContext = nullptr;
             mListener = nullptr;
+            mCallbacks = nullptr;
+            mProgressSink = nullptr;
             return rv;
         }
         if (mLoadInfo && mLoadInfo->GetEnforceSecurity()) {
             rv = channel->AsyncOpen2(downloader);
         }
         else {
             rv = channel->AsyncOpen(downloader, nullptr);
         }
@@ -823,33 +832,43 @@ nsJARChannel::AsyncOpen(nsIStreamListene
     else {
         rv = OpenLocalFile();
     }
 
     if (NS_FAILED(rv)) {
         mIsPending = false;
         mListenerContext = nullptr;
         mListener = nullptr;
+        mCallbacks = nullptr;
+        mProgressSink = nullptr;
         return rv;
     }
 
     if (mLoadGroup)
         mLoadGroup->AddRequest(this, nullptr);
 
     mOpened = true;
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsJARChannel::AsyncOpen2(nsIStreamListener *aListener)
 {
   nsCOMPtr<nsIStreamListener> listener = aListener;
   nsresult rv = nsContentSecurityManager::doContentSecurityCheck(this, listener);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_FAILED(rv)) {
+      mIsPending = false;
+      mListenerContext = nullptr;
+      mListener = nullptr;
+      mCallbacks = nullptr;
+      mProgressSink = nullptr;
+      return rv;
+  }
+
   return AsyncOpen(listener, nullptr);
 }
 
 //-----------------------------------------------------------------------------
 // nsIJARChannel
 //-----------------------------------------------------------------------------
 NS_IMETHODIMP
 nsJARChannel::GetIsUnsafe(bool *isUnsafe)
--- a/netwerk/base/nsBaseChannel.cpp
+++ b/netwerk/base/nsBaseChannel.cpp
@@ -696,17 +696,20 @@ nsBaseChannel::AsyncOpen(nsIStreamListen
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsBaseChannel::AsyncOpen2(nsIStreamListener *aListener)
 {
   nsCOMPtr<nsIStreamListener> listener = aListener;
   nsresult rv = nsContentSecurityManager::doContentSecurityCheck(this, listener);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_FAILED(rv)) {
+    mCallbacks = nullptr;
+    return rv;
+  }
   return AsyncOpen(listener, nullptr);
 }
 
 //-----------------------------------------------------------------------------
 // nsBaseChannel::nsITransportEventSink
 
 NS_IMETHODIMP
 nsBaseChannel::OnTransportStatus(nsITransport *transport, nsresult status,
--- a/netwerk/protocol/wyciwyg/WyciwygChannelChild.cpp
+++ b/netwerk/protocol/wyciwyg/WyciwygChannelChild.cpp
@@ -641,16 +641,17 @@ WyciwygChannelChild::AsyncOpen(nsIStream
     mLoadGroup->AddRequest(this, nullptr);
   }
 
   URIParams originalURI;
   SerializeURI(mOriginalURI, originalURI);
 
   mozilla::dom::TabChild* tabChild = GetTabChild(this);
   if (MissingRequiredTabChild(tabChild, "wyciwyg")) {
+    mCallbacks = nullptr;
     return NS_ERROR_ILLEGAL_VALUE;
   }
 
   PBrowserOrId browser = static_cast<ContentChild*>(Manager()->Manager())
                          ->GetBrowserOrId(tabChild);
 
   SendAsyncOpen(originalURI, mLoadFlags, IPC::SerializedLoadContext(this), browser);
 
@@ -660,17 +661,20 @@ WyciwygChannelChild::AsyncOpen(nsIStream
   return NS_OK;
 }
 
 NS_IMETHODIMP
 WyciwygChannelChild::AsyncOpen2(nsIStreamListener *aListener)
 {
   nsCOMPtr<nsIStreamListener> listener = aListener;
   nsresult rv = nsContentSecurityManager::doContentSecurityCheck(this, listener);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_FAILED(rv)) {
+    mCallbacks = nullptr;
+    return rv;
+  }
   return AsyncOpen(listener, nullptr);
 }
 
 //-----------------------------------------------------------------------------
 // nsIWyciwygChannel
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
--- a/netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp
+++ b/netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp
@@ -441,16 +441,17 @@ nsWyciwygChannel::AsyncOpen(nsIStreamLis
   // mIsPending set to true since OnCacheEntryAvailable may be called
   // synchronously and fails when mIsPending found false.
   mIsPending = true;
   nsresult rv = OpenCacheEntry(mURI, nsICacheStorage::OPEN_READONLY |
                                      nsICacheStorage::CHECK_MULTITHREADED);
   if (NS_FAILED(rv)) {
     LOG(("nsWyciwygChannel::OpenCacheEntry failed [rv=%x]\n", rv));
     mIsPending = false;
+    mCallbacks = nullptr;
     return rv;
   }
 
   // There is no code path that would invoke the listener sooner than
   // we get to this line in case OnCacheEntryAvailable is invoked
   // synchronously.
   mListener = listener;
   mListenerContext = ctx;
@@ -461,17 +462,21 @@ nsWyciwygChannel::AsyncOpen(nsIStreamLis
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsWyciwygChannel::AsyncOpen2(nsIStreamListener *aListener)
 {
   nsCOMPtr<nsIStreamListener> listener = aListener;
   nsresult rv = nsContentSecurityManager::doContentSecurityCheck(this, listener);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_FAILED(rv)) {
+    mIsPending = false;
+    mCallbacks = nullptr;
+    return rv;
+  }
   return AsyncOpen(listener, nullptr);
 }
 
 //////////////////////////////////////////////////////////////////////////////
 // nsIWyciwygChannel
 //////////////////////////////////////////////////////////////////////////////
 
 nsresult
--- a/uriloader/exthandler/nsExternalProtocolHandler.cpp
+++ b/uriloader/exthandler/nsExternalProtocolHandler.cpp
@@ -220,17 +220,20 @@ NS_IMETHODIMP nsExtProtocolChannel::Asyn
 
   return OpenURL();
 }
 
 NS_IMETHODIMP nsExtProtocolChannel::AsyncOpen2(nsIStreamListener *aListener)
 {
   nsCOMPtr<nsIStreamListener> listener = aListener;
   nsresult rv = nsContentSecurityManager::doContentSecurityCheck(this, listener);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_FAILED(rv)) {
+    mCallbacks = nullptr;
+    return rv;
+  }
   return AsyncOpen(listener, nullptr);
 }
 
 NS_IMETHODIMP nsExtProtocolChannel::GetLoadFlags(nsLoadFlags *aLoadFlags)
 {
   *aLoadFlags = mLoadFlags;
   return NS_OK;
 }