Bug 1585236 - Have a preference to not be conservative when conneting a proxy, r=dragana a=lizzard
authorHonza Bambas <honzab.moz@firemni.cz>
Wed, 09 Oct 2019 16:24:00 +0000
changeset 555669 588266ed71ed5b9996030eaa2e9aba1daafd3b44
parent 555668 fb229825695eb91e4a92797a2b7b9a176b7fded3
child 555670 dbeffc0408158c980245d1be7dd5440af1e99377
push id2179
push userbtara@mozilla.com
push dateTue, 29 Oct 2019 23:10:30 +0000
treeherdermozilla-release@dbeffc040815 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana, lizzard
bugs1585236
milestone70.0.1
Bug 1585236 - Have a preference to not be conservative when conneting a proxy, r=dragana a=lizzard Differential Revision: https://phabricator.services.mozilla.com/D47973
modules/libpref/init/all.js
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
netwerk/protocol/http/nsHttpHandler.cpp
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -1546,16 +1546,23 @@ pref("network.http.version", "1.1");    
 // pref("network.http.version", "0.9");   // it'll work too if you're crazy
 // keep-alive option is effectively obsolete. Nevertheless it'll work with
 // some older 1.0 servers:
 
 pref("network.http.proxy.version", "1.1");    // default
 // pref("network.http.proxy.version", "1.0"); // uncomment this out in case of problems
                                               // (required if using junkbuster proxy)
 
+// Whether we should respect the BE_CONSERVATIVE (aka nsIHttpChannelInternal.beConservative)
+// flag when connecting to a proxy.  If the configured proxy accepts only TLS 1.3, system
+// requests like updates will not pass through.  Setting this pref to false will fix that
+// problem.
+// Default at true to preserve the behavior we had before for backward compat.
+pref("network.http.proxy.respect-be-conservative", true);
+
 // this preference can be set to override the socket type used for normal
 // HTTP traffic.  an empty value indicates the normal TCP/IP socket type.
 pref("network.http.default-socket-type", "");
 
 // There is a problem with some IIS7 servers that don't close the connection
 // properly after it times out (bug #491541). Default timeout on IIS7 is
 // 120 seconds. We need to reuse or drop the connection within this time.
 // We set the timeout a little shorter to keep a reserve for cases when
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -121,16 +121,17 @@ nsHttpConnectionMgr::nsHttpConnectionMgr
       mThrottleEnabled(false),
       mThrottleVersion(2),
       mThrottleSuspendFor(0),
       mThrottleResumeFor(0),
       mThrottleReadLimit(0),
       mThrottleReadInterval(0),
       mThrottleHoldTime(0),
       mThrottleMaxTime(0),
+      mBeConservativeForProxy(true),
       mIsShuttingDown(false),
       mNumActiveConns(0),
       mNumIdleConns(0),
       mNumSpdyActiveConns(0),
       mNumHalfOpenConns(0),
       mTimeOfNextWakeUp(UINT64_MAX),
       mPruningNoTraffic(false),
       mTimeoutTickArmed(false),
@@ -2980,16 +2981,19 @@ void nsHttpConnectionMgr::OnMsgUpdatePar
       mThrottleReadInterval = value;
       break;
     case THROTTLING_HOLD_TIME:
       mThrottleHoldTime = value;
       break;
     case THROTTLING_MAX_TIME:
       mThrottleMaxTime = TimeDuration::FromMilliseconds(value);
       break;
+    case PROXY_BE_CONSERVATIVE:
+      mBeConservativeForProxy = !!value;
+      break;
     default:
       MOZ_ASSERT_UNREACHABLE("unexpected parameter name");
   }
 }
 
 // nsHttpConnectionMgr::nsConnectionEntry
 nsHttpConnectionMgr::nsConnectionEntry::~nsConnectionEntry() {
   LOG(("nsConnectionEntry::~nsConnectionEntry this=%p", this));
@@ -3990,16 +3994,34 @@ nsHttpConnectionMgr::nsHalfOpenSocket::n
 nsHttpConnectionMgr::nsHalfOpenSocket::~nsHalfOpenSocket() {
   MOZ_ASSERT(!mStreamOut);
   MOZ_ASSERT(!mBackupStreamOut);
   LOG(("Destroying nsHalfOpenSocket [this=%p]\n", this));
 
   if (mEnt) mEnt->RemoveHalfOpen(this);
 }
 
+bool nsHttpConnectionMgr::BeConservativeIfProxied(nsIProxyInfo* proxy) {
+  if (mBeConservativeForProxy) {
+    // The pref says to be conservative for proxies.
+    return true;
+  }
+
+  if (!proxy) {
+    // There is no proxy, so be conservative by default.
+    return true;
+  }
+
+  // Be conservative only if there is no proxy host set either.
+  // This logic was copied from nsSSLIOLayerAddToSocket.
+  nsAutoCString proxyHost;
+  proxy->GetHost(proxyHost);
+  return proxyHost.IsEmpty();
+}
+
 nsresult nsHttpConnectionMgr::nsHalfOpenSocket::SetupStreams(
     nsISocketTransport** transport, nsIAsyncInputStream** instream,
     nsIAsyncOutputStream** outstream, bool isBackup) {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
   MOZ_ASSERT(mEnt);
   nsresult rv;
   nsTArray<nsCString> socketTypes;
@@ -4062,17 +4084,18 @@ nsresult nsHttpConnectionMgr::nsHalfOpen
   if (ci->GetPrivate() || ci->GetIsolated()) {
     tmpFlags |= nsISocketTransport::NO_PERMANENT_STORAGE;
   }
 
   if (ci->GetLessThanTls13()) {
     tmpFlags |= nsISocketTransport::DONT_TRY_ESNI;
   }
 
-  if ((mCaps & NS_HTTP_BE_CONSERVATIVE) || ci->GetBeConservative()) {
+  if (((mCaps & NS_HTTP_BE_CONSERVATIVE) || ci->GetBeConservative()) &&
+      gHttpHandler->ConnMgr()->BeConservativeIfProxied(ci->ProxyInfo())) {
     LOG(("Setting Socket to BE_CONSERVATIVE"));
     tmpFlags |= nsISocketTransport::BE_CONSERVATIVE;
   }
 
   if (mCaps & NS_HTTP_DISABLE_IPV4) {
     tmpFlags |= nsISocketTransport::DISABLE_IPV4;
   } else if (mCaps & NS_HTTP_DISABLE_IPV6) {
     tmpFlags |= nsISocketTransport::DISABLE_IPV6;
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -60,17 +60,18 @@ class nsHttpConnectionMgr final : public
     MAX_PERSISTENT_CONNECTIONS_PER_PROXY,
     MAX_REQUEST_DELAY,
     THROTTLING_ENABLED,
     THROTTLING_SUSPEND_FOR,
     THROTTLING_RESUME_FOR,
     THROTTLING_READ_LIMIT,
     THROTTLING_READ_INTERVAL,
     THROTTLING_HOLD_TIME,
-    THROTTLING_MAX_TIME
+    THROTTLING_MAX_TIME,
+    PROXY_BE_CONSERVATIVE
   };
 
   //-------------------------------------------------------------------------
   // NOTE: functions below may only be called on the main thread.
   //-------------------------------------------------------------------------
 
   nsHttpConnectionMgr();
 
@@ -560,16 +561,17 @@ class nsHttpConnectionMgr final : public
   bool mThrottleEnabled;
   uint32_t mThrottleVersion;
   uint32_t mThrottleSuspendFor;
   uint32_t mThrottleResumeFor;
   uint32_t mThrottleReadLimit;
   uint32_t mThrottleReadInterval;
   uint32_t mThrottleHoldTime;
   TimeDuration mThrottleMaxTime;
+  bool mBeConservativeForProxy;
   Atomic<bool, mozilla::Relaxed> mIsShuttingDown;
 
   //-------------------------------------------------------------------------
   // NOTE: these members are only accessed on the socket transport thread
   //-------------------------------------------------------------------------
 
   MOZ_MUST_USE bool ProcessPendingQForEntry(nsConnectionEntry*,
                                             bool considerAll);
@@ -813,16 +815,20 @@ class nsHttpConnectionMgr final : public
       nsConnectionEntry* ent, nsAHttpTransaction* trans);
 
   // When current active tab is changed, this function uses
   // |previousWindowId| to select background transactions and
   // mCurrentTopLevelOuterContentWindowId| to select foreground transactions.
   // Then, it notifies selected transactions' connection of the new active tab
   // id.
   void NotifyConnectionOfWindowIdChange(uint64_t previousWindowId);
+
+  // A test if be-conservative should be used when proxy is setup for the
+  // connection
+  bool BeConservativeIfProxied(nsIProxyInfo* proxy);
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsHttpConnectionMgr::nsHalfOpenSocket,
                               NS_HALFOPENSOCKET_IID)
 
 }  // namespace net
 }  // namespace mozilla
 
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -1324,16 +1324,26 @@ void nsHttpHandler::PrefsChanged(const c
       if (httpVersion.EqualsLiteral("1.1"))
         mProxyHttpVersion = HttpVersion::v1_1;
       else
         mProxyHttpVersion = HttpVersion::v1_0;
       // it does not make sense to issue a HTTP/0.9 request to a proxy server
     }
   }
 
+  if (PREF_CHANGED(HTTP_PREF("proxy.respect-be-conservative"))) {
+    rv =
+        Preferences::GetBool(HTTP_PREF("proxy.respect-be-conservative"), &cVar);
+    if (NS_SUCCEEDED(rv) && mConnMgr) {
+      Unused << mConnMgr->UpdateParam(
+          nsHttpConnectionMgr::PROXY_BE_CONSERVATIVE,
+          static_cast<int32_t>(cVar));
+    }
+  }
+
   if (PREF_CHANGED(HTTP_PREF("qos"))) {
     rv = Preferences::GetInt(HTTP_PREF("qos"), &val);
     if (NS_SUCCEEDED(rv)) mQoSBits = (uint8_t)clamped(val, 0, 0xff);
   }
 
   if (PREF_CHANGED(HTTP_PREF("accept-encoding"))) {
     nsAutoCString acceptEncodings;
     rv = Preferences::GetCString(HTTP_PREF("accept-encoding"), acceptEncodings);