Bug 1345893 - Handle Suspend() called on an HTTP channel from http-on-modify-request handler. r=mayhemer, a=ritu
authorShane Caraveo <scaraveo@mozilla.com>
Fri, 14 Apr 2017 10:00:05 -0700
changeset 578483 3a747480ead177402f8a1b9dc6cfa249401c5da6
parent 578482 b388642b066467743b3f920e79d7080ef1798858
child 578484 c7b064f6b93aa0af228a53052c72cfc8760b1b92
push id58939
push userbmo:cku@mozilla.com
push dateTue, 16 May 2017 04:17:59 +0000
reviewersmayhemer, ritu
bugs1345893
milestone52.1.2
Bug 1345893 - Handle Suspend() called on an HTTP channel from http-on-modify-request handler. r=mayhemer, a=ritu MozReview-Commit-ID: LbW9zgnPzmu
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
netwerk/test/unit/test_suspend_channel_on_modified.js
netwerk/test/unit/xpcshell.ini
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -5969,16 +5969,62 @@ nsHttpChannel::BeginConnect()
     // mCustomAuthHeader is set in AsyncOpen if we find Authorization header
     mAuthProvider->AddAuthorizationHeaders(mCustomAuthHeader);
 
     // notify "http-on-modify-request" observers
     CallOnModifyRequestObservers();
 
     SetLoadGroupUserAgentOverride();
 
+    // Check if request was cancelled during on-modify-request or on-useragent.
+    if (mCanceled) {
+        return mStatus;
+    }
+
+    if (mSuspendCount) {
+        LOG(("Waiting until resume BeginConnect [this=%p]\n", this));
+        MOZ_ASSERT(!mCallOnResume);
+        mCallOnResume = &nsHttpChannel::HandleBeginConnectContinue;
+        return NS_OK;
+    }
+
+    return BeginConnectContinue();
+}
+
+void
+nsHttpChannel::HandleBeginConnectContinue()
+{
+    NS_PRECONDITION(!mCallOnResume, "How did that happen?");
+    nsresult rv;
+
+    if (mSuspendCount) {
+        LOG(("Waiting until resume BeginConnect [this=%p]\n", this));
+        mCallOnResume = &nsHttpChannel::HandleBeginConnectContinue;
+        return;
+    }
+
+    LOG(("nsHttpChannel::HandleBeginConnectContinue [this=%p]\n", this));
+    rv = BeginConnectContinue();
+    if (NS_FAILED(rv)) {
+        CloseCacheEntry(false);
+        Unused << AsyncAbort(rv);
+    }
+}
+
+nsresult
+nsHttpChannel::BeginConnectContinue()
+{
+    nsresult rv;
+
+    // Check if request was cancelled during suspend AFTER on-modify-request or
+    // on-useragent.
+    if (mCanceled) {
+        return mStatus;
+    }
+
     // Check to see if we should redirect this channel elsewhere by
     // nsIHttpChannel.redirectTo API request
     if (mAPIRedirectToURI) {
         return AsyncCall(&nsHttpChannel::HandleAsyncAPIRedirect);
     }
     // Check to see if this principal exists on local blocklists.
     RefPtr<nsChannelClassifier> channelClassifier = new nsChannelClassifier();
     if (mLoadFlags & LOAD_CLASSIFY_URI) {
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -278,16 +278,18 @@ public:
 protected:
     virtual ~nsHttpChannel();
 
 private:
     typedef nsresult (nsHttpChannel::*nsContinueRedirectionFunc)(nsresult result);
 
     bool     RequestIsConditional();
     nsresult BeginConnect();
+    void     HandleBeginConnectContinue();
+    MOZ_MUST_USE nsresult BeginConnectContinue();
     nsresult ContinueBeginConnectWithResult();
     void     ContinueBeginConnect();
     nsresult Connect();
     void     SpeculativeConnect();
     nsresult SetupTransaction();
     void     SetupTransactionRequestContext();
     nsresult CallOnStartRequest();
     nsresult ProcessResponse();
new file mode 100644
--- /dev/null
+++ b/netwerk/test/unit/test_suspend_channel_on_modified.js
@@ -0,0 +1,175 @@
+// This file tests async handling of a channel suspended in http-on-modify-request.
+
+var CC = Components.Constructor;
+
+Cu.import("resource://testing-common/httpd.js");
+Cu.import("resource://gre/modules/NetUtil.jsm");
+
+var obs = Cc["@mozilla.org/observer-service;1"]
+            .getService(Ci.nsIObserverService);
+
+var ios = Cc["@mozilla.org/network/io-service;1"]
+            .getService(Components.interfaces.nsIIOService);
+
+// baseUrl is always the initial connection attempt and is handled by
+// failResponseHandler since every test expects that request will either be
+// redirected or cancelled.
+var baseUrl;
+
+function failResponseHandler(metadata, response)
+{
+  var text = "failure response";
+  response.setHeader("Content-Type", "text/plain", false);
+  response.bodyOutputStream.write(text, text.length);
+  do_check_true(false, "Received request when we shouldn't.");
+}
+
+function successResponseHandler(metadata, response)
+{
+  var text = "success response";
+  response.setHeader("Content-Type", "text/plain", false);
+  response.bodyOutputStream.write(text, text.length);
+  do_check_true(true, "Received expected request.");
+}
+
+function onModifyListener(callback) {
+  obs.addObserver({
+    observe: function(subject, topic, data) {
+      var obs = Cc["@mozilla.org/observer-service;1"].getService();
+      obs = obs.QueryInterface(Ci.nsIObserverService);
+      obs.removeObserver(this, "http-on-modify-request");
+      callback(subject.QueryInterface(Ci.nsIHttpChannel));
+    }
+  }, "http-on-modify-request", false);
+}
+
+function startChannelRequest(baseUrl, flags, expectedResponse=null) {
+  var chan = NetUtil.newChannel({
+    uri: baseUrl,
+    loadUsingSystemPrincipal: true
+  });
+  chan.asyncOpen2(new ChannelListener((request, data, context) => {
+    if (expectedResponse) {
+      do_check_eq(data, expectedResponse);
+    } else {
+      do_check_true(!!!data, "no response");
+    }
+    do_execute_soon(run_next_test)
+  }, null, flags));
+}
+
+
+add_test(function testSimpleRedirect() {
+  onModifyListener(chan => {
+    chan.redirectTo(ios.newURI(`${baseUrl}/success`));
+  });
+  startChannelRequest(baseUrl, undefined, "success response");
+});
+
+add_test(function testSimpleCancel() {
+  onModifyListener(chan => {
+    chan.cancel(Cr.NS_BINDING_ABORTED);
+  });
+  startChannelRequest(baseUrl, CL_EXPECT_FAILURE);
+});
+
+add_test(function testSimpleCancelRedirect() {
+  onModifyListener(chan => {
+    chan.redirectTo(ios.newURI(`${baseUrl}/fail`));
+    chan.cancel(Cr.NS_BINDING_ABORTED);
+  });
+  startChannelRequest(baseUrl, CL_EXPECT_FAILURE);
+});
+
+// Test a request that will get redirected asynchronously.  baseUrl should
+// not be requested, we should receive the request for the redirectedUrl.
+add_test(function testAsyncRedirect() {
+  onModifyListener(chan => {
+    // Suspend the channel then yield to make this async.
+    chan.suspend();
+    Promise.resolve().then(() => {
+      chan.redirectTo(ios.newURI(`${baseUrl}/success`));
+      chan.resume();
+    });
+  });
+  startChannelRequest(baseUrl, undefined, "success response");
+});
+
+add_test(function testSyncRedirect() {
+  onModifyListener(chan => {
+    chan.suspend();
+    chan.redirectTo(ios.newURI(`${baseUrl}/success`));
+    Promise.resolve().then(() => {
+      chan.resume();
+    });
+  });
+  startChannelRequest(baseUrl, undefined, "success response");
+});
+
+add_test(function testAsyncCancel() {
+  onModifyListener(chan => {
+    // Suspend the channel then yield to make this async.
+    chan.suspend();
+    Promise.resolve().then(() => {
+      chan.cancel(Cr.NS_BINDING_ABORTED);
+      chan.resume();
+    });
+  });
+  startChannelRequest(baseUrl, CL_EXPECT_FAILURE);
+});
+
+add_test(function testSyncCancel() {
+  onModifyListener(chan => {
+    chan.suspend();
+    chan.cancel(Cr.NS_BINDING_ABORTED);
+    Promise.resolve().then(() => {
+      chan.resume();
+    });
+  });
+  startChannelRequest(baseUrl, CL_EXPECT_FAILURE);
+});
+
+// Test request that will get redirected and cancelled asynchronously,
+// ensure no connection is made.
+add_test(function testAsyncCancelRedirect() {
+  onModifyListener(chan => {
+    // Suspend the channel then yield to make this async.
+    chan.suspend();
+    Promise.resolve().then(() => {
+      chan.cancel(Cr.NS_BINDING_ABORTED);
+      chan.redirectTo(ios.newURI(`${baseUrl}/fail`));
+      chan.resume();
+    });
+  });
+  startChannelRequest(baseUrl, CL_EXPECT_FAILURE);
+});
+
+// Test a request that will get cancelled synchronously, ensure async redirect
+// is not made.
+add_test(function testSyncCancelRedirect() {
+  onModifyListener(chan => {
+    chan.suspend();
+    chan.cancel(Cr.NS_BINDING_ABORTED);
+    Promise.resolve().then(() => {
+      chan.redirectTo(ios.newURI(`${baseUrl}/fail`));
+      chan.resume();
+    });
+  });
+  startChannelRequest(baseUrl, CL_EXPECT_FAILURE);
+});
+
+function run_test() {
+  var httpServer = new HttpServer();
+  httpServer.registerPathHandler("/", failResponseHandler);
+  httpServer.registerPathHandler("/fail", failResponseHandler);
+  httpServer.registerPathHandler("/success", successResponseHandler);
+  httpServer.start(-1);
+
+  baseUrl = `http://localhost:${httpServer.identity.primaryPort}`;
+
+  run_next_test();
+
+  do_register_cleanup(function(){
+    httpServer.stop(() => {});
+  });
+}
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -345,16 +345,17 @@ skip-if = os == "android" || (os == "win
 reason = bug 1190674
 firefox-appdir = browser
 [test_tls_server_multiple_clients.js]
 # The local cert service used by this test is not currently shipped on Android
 skip-if = os == "android"
 [test_1073747.js]
 [test_safeoutputstream_append.js]
 [test_suspend_channel_before_connect.js]
+[test_suspend_channel_on_modified.js]
 [test_inhibit_caching.js]
 [test_dns_disable_ipv4.js]
 [test_dns_disable_ipv6.js]
 [test_bug1195415.js]
 [test_cookie_blacklist.js]
 [test_getHost.js]
 [test_bug412457.js]
 [test_bug464591.js]