Bug 1477592 - Prevent TCP connection for CONNECT-only HTTP channel with non-http proxy r=dragana
authorPaul Vitale <paul.m.vitale@gmail.com>
Sun, 22 Jul 2018 14:04:07 -0500
changeset 503193 298abb6be2dce482ecb8af9ec6ef78abf2c26e1d
parent 503192 9ea67e7f7a0f4c37a58202346634b771ed360986
child 503194 3d4dd037c4ceb41c227ef41270fefad15a8bb6f8
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana
bugs1477592
milestone65.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 1477592 - Prevent TCP connection for CONNECT-only HTTP channel with non-http proxy r=dragana This moves the check for a non-http proxy from the socket transport callbacks in nsHttpConnection to nsHttpChannel before the connection occurs. MozReview-Commit-ID: CJIozDInXWz
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpConnection.cpp
netwerk/test/unit/test_proxyconnect.js
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -6555,16 +6555,28 @@ nsHttpChannel::BeginConnect()
     }
     LOG(("host=%s port=%d\n", host.get(), port));
     LOG(("uri=%s\n", mSpec.get()));
 
     nsCOMPtr<nsProxyInfo> proxyInfo;
     if (mProxyInfo)
         proxyInfo = do_QueryInterface(mProxyInfo);
 
+    if (mCaps & NS_HTTP_CONNECT_ONLY) {
+        if (!proxyInfo) {
+            LOG(("return failure: no proxy for connect-only channel\n"));
+            return NS_ERROR_FAILURE;
+        }
+
+        if (!proxyInfo->IsHTTP() && !proxyInfo->IsHTTPS()) {
+            LOG(("return failure: non-http proxy for connect-only channel\n"));
+            return NS_ERROR_FAILURE;
+        }
+    }
+
     mRequestHead.SetHTTPS(isHttps);
     mRequestHead.SetOrigin(scheme, host, port);
 
     SetOriginHeader();
     SetDoNotTrack();
 
     OriginAttributes originAttributes;
     NS_GetOriginAttributes(this, originAttributes);
--- a/netwerk/protocol/http/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -1901,17 +1901,18 @@ nsHttpConnection::OnSocketWritable()
     const uint32_t maxWriteAttempts = 128;
     uint32_t writeAttempts = 0;
 
     mForceSendDuringFastOpenPending = false;
 
     if (mTransactionCaps & NS_HTTP_CONNECT_ONLY) {
         if (!mCompletedProxyConnect && !mProxyConnectStream) {
             // A CONNECT has been requested for this connection but will never
-            // be performed. Fail here to let request callbacks happen.
+            // be performed. This should never happen.
+            MOZ_ASSERT(false, "proxy connect will never happen");
             LOG(("return failure because proxy connect will never happen\n"));
             return NS_ERROR_FAILURE;
         }
 
         if (mCompletedProxyConnect) {
             // Don't need to check this each write attempt since it is only
             // updated after OnSocketWritable completes.
             // We've already done primary tls (if needed) and sent our CONNECT.
@@ -2081,17 +2082,18 @@ nsHttpConnection::OnSocketReadable()
     PRIntervalTime delta = now - mLastReadTime;
 
     // Reset mResponseTimeoutEnabled to stop response timeout checks.
     mResponseTimeoutEnabled = false;
 
     if ((mTransactionCaps & NS_HTTP_CONNECT_ONLY) &&
         !mCompletedProxyConnect && !mProxyConnectStream) {
         // A CONNECT has been requested for this connection but will never
-        // be performed. Fail here to let request callbacks happen.
+        // be performed. This should never happen.
+        MOZ_ASSERT(false, "proxy connect will never happen");
         LOG(("return failure because proxy connect will never happen\n"));
         return NS_ERROR_FAILURE;
     }
 
     if (mKeepAliveMask && (delta >= mMaxHangTime)) {
         LOG(("max hang time exceeded!\n"));
         // give the handler a chance to create a new persistent connection to
         // this host if we've been busy for too long.
--- a/netwerk/test/unit/test_proxyconnect.js
+++ b/netwerk/test/unit/test_proxyconnect.js
@@ -9,16 +9,21 @@
 // 3. Write data to the tunnel (and read server-side)
 // 4. Read data from the tunnel (and write server-side)
 // 5. done
 // test_connectonly_noproxy tests an http channel with only connect set but
 // no proxy configured.
 // 1. OnTransportAvailable callback NOT called (checked in step 2)
 // 2. StopRequest callback called
 // 3. done
+// test_connectonly_nonhttp tests an http channel with only connect set with a
+// non-http proxy.
+// 1. OnTransportAvailable callback NOT called (checked in step 2)
+// 2. StopRequest callback called
+// 3. done
 
 ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 // -1 then initialized with an actual port from the serversocket
 var socketserver_port = -1;
 
 const CC = Components.Constructor;
@@ -66,16 +71,17 @@ var listener = {
 
   onDataAvailable: function test_ODA() {
     do_throw("Should not get any data!");
   },
 
   onStopRequest: function test_onStopR(request, ctx, status) {
     if (state === STATE_COMPLETED) {
       Assert.equal(transportAvailable, false, 'transport available not called');
+      Assert.equal(status, 0x80004005, 'error code matches');
 
       nextTest();
       return;
     }
 
     Assert.equal(accepted, true, 'socket accepted');
     accepted = false;
   }
@@ -289,36 +295,53 @@ function test_connectonly() {
 function test_connectonly_noproxy() {
   clearPrefs()
   var chan = makeChan();
   chan.asyncOpen2(listener);
 
   do_test_pending();
 }
 
+function test_connectonly_nonhttp() {
+  clearPrefs()
+
+  Services.prefs.setCharPref("network.proxy.socks", "localhost")
+  Services.prefs.setIntPref("network.proxy.socks_port", socketserver_port)
+  Services.prefs.setCharPref("network.proxy.no_proxies_on", "")
+  Services.prefs.setIntPref("network.proxy.type", 1)
+
+  var chan = makeChan()
+  chan.asyncOpen2(listener)
+
+  do_test_pending()
+}
+
 function nextTest() {
   transportAvailable = false;
 
   if (tests.length == 0) {
     do_test_finished();
     return;
   }
 
   (tests.shift())();
   do_test_finished();
 }
 
 var tests = [
   test_connectonly,
-  // test_connectonly_noproxy,
+  test_connectonly_noproxy,
+  test_connectonly_nonhttp
 ];
 
 function clearPrefs() {
   Services.prefs.clearUserPref("network.proxy.ssl");
   Services.prefs.clearUserPref("network.proxy.ssl_port");
+  Services.prefs.clearUserPref("network.proxy.socks");
+  Services.prefs.clearUserPref("network.proxy.socks_port");
   Services.prefs.clearUserPref("network.proxy.no_proxies_on");
   Services.prefs.clearUserPref("network.proxy.type");
 }
 
 function run_test() {
   createProxy();
 
   registerCleanupFunction(clearPrefs);