Bug 1522093 - Make sure to not reuse a connection to a proxy that returns an error response we can't gracefully handle, r=kershaw
authorHonza Bambas <honzab.moz@firemni.cz>
Tue, 12 Feb 2019 22:44:30 +0000
changeset 516712 6de9408be22f0be3ba084bbbe050f5832fb50bd2
parent 516711 ba745ab8ec4a6a6369285026a1a7e8ba3bee5457
child 516713 eb177b6fdd37d20d50188624ce33457f41d04c37
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskershaw
bugs1522093
milestone67.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 1522093 - Make sure to not reuse a connection to a proxy that returns an error response we can't gracefully handle, r=kershaw Differential Revision: https://phabricator.services.mozilla.com/D19403
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpTransaction.cpp
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -1844,16 +1844,22 @@ nsresult nsHttpChannel::ProcessFailedPro
       break;
     // Confused proxy server or malicious response
     default:
       rv = NS_ERROR_PROXY_CONNECTION_REFUSED;
       break;
   }
   LOG(("Cancelling failed proxy CONNECT [this=%p httpStatus=%u]\n", this,
        httpStatus));
+
+  // Make sure the connection is thrown away as it can be in a bad state
+  // and the proxy may just hang on the next request.
+  MOZ_ASSERT(mTransaction);
+  mTransaction->DontReuseConnection();
+
   Cancel(rv);
   {
     nsresult rv = CallOnStartRequest();
     if (NS_FAILED(rv)) {
       LOG(("CallOnStartRequest failed [this=%p httpStatus=%u rv=%08x]\n", this,
            httpStatus, static_cast<uint32_t>(rv)));
     }
   }
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -1154,17 +1154,31 @@ void nsHttpTransaction::Close(nsresult r
       if (mResponseHead->Version() == HttpVersion::v0_9) {
         // Reject 0 byte HTTP/0.9 Responses - bug 423506
         LOG(("nsHttpTransaction::Close %p 0 Byte 0.9 Response", this));
         reason = NS_ERROR_NET_RESET;
       }
     }
 
     // honor the sticky connection flag...
-    if (mCaps & NS_HTTP_STICKY_CONNECTION) relConn = false;
+    if (mCaps & NS_HTTP_STICKY_CONNECTION) {
+      LOG(("  keeping the connection because of STICKY_CONNECTION flag"));
+      relConn = false;
+    }
+
+    // if the proxy connection has failed, we want the connection be held
+    // to allow the upper layers (think nsHttpChannel) to close it when
+    // the failure is unrecoverable.
+    // we can't just close it here, because mProxyConnectFailed is to a general
+    // flag and is also set for e.g. 407 which doesn't mean to kill the
+    // connection, specifically when connection oriented auth may be involved.
+    if (mProxyConnectFailed) {
+      LOG(("  keeping the connection because of mProxyConnectFailed"));
+      relConn = false;
+    }
   }
 
   // mTimings.responseEnd is normally recorded based on the end of a
   // HTTP delimiter such as chunked-encodings or content-length. However,
   // EOF or an error still require an end time be recorded.
   if (TimingEnabled()) {
     const TimingStruct timings = Timings();
     if (timings.responseEnd.IsNull() && !timings.responseStart.IsNull()) {