Bug 1598516 - Don't crash if NS_NewChannelInternal fails. r=mayhemer
authorMatt Woodrow <mwoodrow@mozilla.com>
Tue, 03 Dec 2019 23:54:51 +0000
changeset 505229 dfb2624a985788ebafb5e70e02b329ae073fa3c8
parent 505228 3422602420a0287741accfa844448e02f99c6212
child 505230 abece0ceac3dad6bec34f412d5a66e18393ffd08
push id36880
push usermalexandru@mozilla.com
push dateWed, 04 Dec 2019 09:56:40 +0000
treeherdermozilla-central@6989fcd6bab3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer
bugs1598516
milestone73.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1598516 - Don't crash if NS_NewChannelInternal fails. r=mayhemer Looks like this can sometimes fail with moz-extension URIs, so we shouldn't crash Differential Revision: https://phabricator.services.mozilla.com/D54249
dom/ipc/ContentChild.cpp
netwerk/ipc/DocumentChannelChild.cpp
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -3673,38 +3673,44 @@ mozilla::ipc::IPCResult ContentChild::Re
 
   nsCOMPtr<nsIChannel> newChannel;
   rv = NS_NewChannelInternal(getter_AddRefs(newChannel), aArgs.uri(), loadInfo,
                              nullptr,  // PerformanceStorage
                              nullptr,  // aLoadGroup
                              nullptr,  // aCallbacks
                              aArgs.newLoadFlags());
 
-  RefPtr<nsIChildChannel> childChannel = do_QueryObject(newChannel);
-  if (NS_FAILED(rv) || !childChannel) {
-    MOZ_DIAGNOSTIC_ASSERT(false, "NS_NewChannelInternal failed");
-    return IPC_OK();
-  }
-
   // This is used to report any errors back to the parent by calling
   // CrossProcessRedirectFinished.
   RefPtr<HttpChannelChild> httpChild = do_QueryObject(newChannel);
   auto scopeExit = MakeScopeExit([&]() {
     if (httpChild) {
       rv = httpChild->CrossProcessRedirectFinished(rv);
     }
-    nsCOMPtr<nsILoadInfo> loadInfo;
-    MOZ_ALWAYS_SUCCEEDS(newChannel->GetLoadInfo(getter_AddRefs(loadInfo)));
     Maybe<LoadInfoArgs> loadInfoArgs;
-    MOZ_ALWAYS_SUCCEEDS(
-        mozilla::ipc::LoadInfoToLoadInfoArgs(loadInfo, &loadInfoArgs));
+    if (newChannel && NS_SUCCEEDED(rv)) {
+      nsCOMPtr<nsILoadInfo> loadInfo;
+      MOZ_ALWAYS_SUCCEEDS(newChannel->GetLoadInfo(getter_AddRefs(loadInfo)));
+      MOZ_ALWAYS_SUCCEEDS(
+          mozilla::ipc::LoadInfoToLoadInfoArgs(loadInfo, &loadInfoArgs));
+    }
     aResolve(
         Tuple<const nsresult&, const Maybe<LoadInfoArgs>&>(rv, loadInfoArgs));
   });
 
+  if (NS_FAILED(rv)) {
+    return IPC_OK();
+  }
+
+  RefPtr<nsIChildChannel> childChannel = do_QueryObject(newChannel);
+  if (!childChannel) {
+    rv = NS_ERROR_UNEXPECTED;
+    return IPC_OK();
+  }
+
   if (httpChild) {
     rv = httpChild->SetChannelId(aArgs.channelId());
     if (NS_FAILED(rv)) {
       return IPC_OK();
     }
 
     rv = httpChild->SetOriginalURI(aArgs.originalURI());
     if (NS_FAILED(rv)) {
--- a/netwerk/ipc/DocumentChannelChild.cpp
+++ b/netwerk/ipc/DocumentChannelChild.cpp
@@ -305,32 +305,30 @@ IPCResult DocumentChannelChild::RecvRedi
   nsCOMPtr<nsIChannel> newChannel;
   nsresult rv =
       NS_NewChannelInternal(getter_AddRefs(newChannel), aArgs.uri(), loadInfo,
                             nullptr,     // PerformanceStorage
                             mLoadGroup,  // aLoadGroup
                             nullptr,     // aCallbacks
                             aArgs.newLoadFlags());
 
-  RefPtr<HttpChannelChild> httpChild = do_QueryObject(newChannel);
-  RefPtr<nsIChildChannel> childChannel = do_QueryObject(newChannel);
-  if (NS_FAILED(rv)) {
-    MOZ_DIAGNOSTIC_ASSERT(false, "NS_NewChannelInternal failed");
-    return IPC_OK();
-  }
-
   // This is used to report any errors back to the parent by calling
   // CrossProcessRedirectFinished.
   auto scopeExit = MakeScopeExit([&]() {
     Maybe<LoadInfoArgs> dummy;
     mRedirectResolver(
         Tuple<const nsresult&, const Maybe<LoadInfoArgs>&>(rv, dummy));
     mRedirectResolver = nullptr;
   });
 
+  if (NS_FAILED(rv)) {
+    return IPC_OK();
+  }
+
+  RefPtr<HttpChannelChild> httpChild = do_QueryObject(newChannel);
   if (httpChild) {
     rv = httpChild->SetChannelId(aArgs.channelId());
   }
   if (NS_FAILED(rv)) {
     return IPC_OK();
   }
 
   rv = newChannel->SetOriginalURI(aArgs.originalURI());
@@ -371,16 +369,17 @@ IPCResult DocumentChannelChild::RecvRedi
   // (ContentChild::RecvCrossProcessRedirect)? In that case there is no local
   // existing actor in the destination process... We really need all information
   // to go up to the parent, and then come down to the new child actor.
   if (nsCOMPtr<nsIWritablePropertyBag> bag = do_QueryInterface(newChannel)) {
     nsHashPropertyBag::CopyFrom(bag, aArgs.properties());
   }
 
   // connect parent.
+  nsCOMPtr<nsIChildChannel> childChannel = do_QueryInterface(newChannel);
   if (childChannel) {
     rv = childChannel->ConnectParent(
         aArgs.registrarId());  // creates parent channel
     if (NS_FAILED(rv)) {
       return IPC_OK();
     }
   }
   mRedirectChannel = newChannel;