Bug 1683176 - Make sure mAuthChannel is released on main thread r=necko-reviewers,dragana

Differential Revision: https://phabricator.services.mozilla.com/D100243

extensions/auth/nsHttpNegotiateAuth.cpp

@@ -28,6 +28,7 @@
 #include "nsCOMPtr.h"
 #include "nsString.h"
 #include "nsNetCID.h"
+#include "nsProxyRelease.h"
 #include "plbase64.h"
 #include "plstr.h"
 #include "mozilla/Base64.h"
@@ -334,12 +335,12 @@ class GetNextTokenRunnable final : public mozilla::Runnable {
   ~GetNextTokenRunnable() override = default;
 
  public:
-  GetNextTokenRunnable(nsIHttpAuthenticableChannel* authChannel,
-                       const char* challenge, bool isProxyAuth,
-                       const char16_t* domain, const char16_t* username,
-                       const char16_t* password, nsISupports* sessionState,
-                       nsISupports* continuationState,
-                       GetNextTokenCompleteEvent* aCompleteEvent)
+  GetNextTokenRunnable(
+      nsMainThreadPtrHandle<nsIHttpAuthenticableChannel>& authChannel,
+      const char* challenge, bool isProxyAuth, const char16_t* domain,
+      const char16_t* username, const char16_t* password,
+      nsISupports* sessionState, nsISupports* continuationState,
+      GetNextTokenCompleteEvent* aCompleteEvent)
       : mozilla::Runnable("GetNextTokenRunnable"),
         mAuthChannel(authChannel),
         mChallenge(challenge),
@@ -409,7 +410,7 @@ class GetNextTokenRunnable final : public mozilla::Runnable {
   }
 
  private:
-  nsCOMPtr<nsIHttpAuthenticableChannel> mAuthChannel;
+  nsMainThreadPtrHandle<nsIHttpAuthenticableChannel> mAuthChannel;
   nsCString mChallenge;
   bool mIsProxyAuth;
   nsString mDomain;
@@ -435,9 +436,12 @@ nsHttpNegotiateAuth::GenerateCredentialsAsync(
   RefPtr<GetNextTokenCompleteEvent> cancelEvent =
       new GetNextTokenCompleteEvent(aCallback);
 
+  nsMainThreadPtrHandle<nsIHttpAuthenticableChannel> handle(
+      new nsMainThreadPtrHolder<nsIHttpAuthenticableChannel>(
+          "nsIHttpAuthenticableChannel", authChannel, false));
   nsCOMPtr<nsIRunnable> getNextTokenRunnable = new GetNextTokenRunnable(
-      authChannel, challenge, isProxyAuth, domain, username, password,
-      sessionState, continuationState, cancelEvent);
+      handle, challenge, isProxyAuth, domain, username, password, sessionState,
+      continuationState, cancelEvent);
 
   nsresult rv = NS_DispatchBackgroundTask(
       getNextTokenRunnable, nsIEventTarget::DISPATCH_EVENT_MAY_BLOCK);

netwerk/protocol/http/nsHttpChannel.cpp

@@ -2508,6 +2508,11 @@ nsresult nsHttpChannel::ContinueProcessResponse3(nsresult rv) {
             ("Suspending the transaction, asynchronously prompting for "
              "credentials"));
         mTransactionPump->Suspend();
+
+#ifdef DEBUG
+        // This is for test purposes only. See bug 1683176 for details.
+        gHttpHandler->OnTransactionSuspendedDueToAuthentication(this);
+#endif
         rv = NS_OK;
       } else if (NS_FAILED(rv)) {
         LOG(("ProcessAuthentication failed [rv=%" PRIx32 "]\n",

netwerk/protocol/http/nsHttpHandler.h

@@ -441,6 +441,12 @@ class nsHttpHandler final : public nsIHttpProtocolHandler,
     NotifyObservers(chan, NS_HTTP_ON_EXAMINE_CACHED_RESPONSE_TOPIC);
   }
 
+  // Called by the channel when the transaction pump is suspended because of
+  // trying to get credentials asynchronously.
+  void OnTransactionSuspendedDueToAuthentication(nsIHttpChannel* chan) {
+    NotifyObservers(chan, "http-on-transaction-suspended-authentication");
+  }
+
   // Generates the host:port string for use in the Host: header as well as the
   // CONNECT line for proxies. This handles IPv6 literals correctly.
   [[nodiscard]] static nsresult GenerateHostPort(const nsCString& host,

netwerk/test/unit/test_bug1683176.js

@@ -0,0 +1,104 @@
+// Test bug 1683176
+//
+// Summary:
+//   Test the case when a channel is cancelled when "negotiate" authentication
+//   is processing.
+//
+
+"use strict";
+
+ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
+
+let prefs;
+let httpserv;
+
+const { HttpServer } = ChromeUtils.import("resource://testing-common/httpd.js");
+
+XPCOMUtils.defineLazyGetter(this, "URL", function() {
+  return "http://localhost:" + httpserv.identity.primaryPort;
+});
+
+XPCOMUtils.defineLazyGetter(this, "PORT", function() {
+  return httpserv.identity.primaryPort;
+});
+
+function makeChan(url, loadingUrl) {
+  var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
+  var ssm = Cc["@mozilla.org/scriptsecuritymanager;1"].getService(
+    Ci.nsIScriptSecurityManager
+  );
+  var principal = ssm.createContentPrincipal(ios.newURI(loadingUrl), {});
+  return NetUtil.newChannel({
+    uri: url,
+    loadingPrincipal: principal,
+    securityFlags: Ci.nsILoadInfo.SEC_ALLOW_CROSS_ORIGIN_SEC_CONTEXT_IS_NULL,
+    contentPolicyType: Ci.nsIContentPolicy.TYPE_OTHER,
+  });
+}
+
+function authHandler(metadata, response) {
+  var body = "blablabla";
+
+  response.seizePower();
+  response.write("HTTP/1.1 401 Unauthorized\r\n");
+  response.write("WWW-Authenticate: Negotiate\r\n");
+  response.write("WWW-Authenticate: Basic realm=test\r\n");
+  response.write("\r\n");
+  response.write(body);
+  response.finish();
+}
+
+function setup() {
+  prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
+
+  prefs.setIntPref("network.auth.subresource-http-auth-allow", 2);
+  prefs.setStringPref("network.negotiate-auth.trusted-uris", "localhost");
+
+  httpserv = new HttpServer();
+  httpserv.registerPathHandler("/auth", authHandler);
+  httpserv.start(-1);
+}
+
+setup();
+registerCleanupFunction(async () => {
+  prefs.clearUserPref("network.auth.subresource-http-auth-allow");
+  prefs.clearUserPref("network.negotiate-auth.trusted-uris");
+  await httpserv.stop();
+});
+
+function makeChan(url) {
+  let chan = NetUtil.newChannel({
+    uri: url,
+    loadUsingSystemPrincipal: true,
+  }).QueryInterface(Ci.nsIHttpChannel);
+  return chan;
+}
+
+function channelOpenPromise(chan) {
+  return new Promise(resolve => {
+    let topic = "http-on-transaction-suspended-authentication";
+    let observer = {
+      QueryInterface: ChromeUtils.generateQI(["nsIObserver"]),
+      observe(aSubject, aTopic, aData) {
+        if (aTopic == topic) {
+          Services.obs.removeObserver(observer, topic);
+          let channel = aSubject.QueryInterface(Ci.nsIChannel);
+          channel.cancel(Cr.NS_BINDING_ABORTED);
+          resolve();
+        }
+      },
+    };
+    Services.obs.addObserver(observer, topic);
+
+    chan.asyncOpen(new ChannelListener(finish, null, CL_EXPECT_FAILURE));
+    function finish() {
+      resolve();
+    }
+  });
+}
+
+add_task(async function testCancelAuthentication() {
+  let chan = makeChan(URL + "/auth", URL);
+  await channelOpenPromise(chan);
+  Assert.equal(chan.status, Cr.NS_BINDING_ABORTED);
+});

netwerk/test/unit/xpcshell.ini

@@ -512,3 +512,5 @@ run-sequentially = node server exceptions dont replay well
 skip-if = asan || tsan || os == 'win' || os =='android'
 run-sequentially = node server exceptions dont replay well
 [test_httpssvc_https_upgrade.js]
+[test_bug1683176.js]
+skip-if = os == "android" || !debug