Bug 1355858 - Blacklist hosts from spdy who are very poorly behaved. r=dragana

In certain cases (such as the case from bug 1050329, where a server claims to speak h2, but really doesn't), we will end up trying every connection to that server as h2 before falling back to http/1.1. Once we know a server is very badly behaved, it doesn't make sense to keep trying h2 (at least for the current browsing session). This adds an in-memory blacklist of origins & conninfos to not try h2 on, so we don't waste round trips trying h2, failing, and then dialing back with http/1.1 except for the first connection to an origin.

Depends on D8436

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

--HG--
extra : moz-landing-system : lando

netwerk/protocol/http/Http2Session.cpp

@@ -3170,6 +3170,13 @@ Http2Session::WriteSegmentsAgain(nsAHttpSegmentWriter *writer,
         mInputFrameType != FRAME_TYPE_SETTINGS) {
       LOG3(("First Frame Type Must Be Settings\n"));
       mPeerFailedHandshake = true;
+
+      // Don't allow any more h2 connections to this host
+      RefPtr<nsHttpConnectionInfo> ci = ConnectionInfo();
+      if (ci) {
+        gHttpHandler->BlacklistSpdy(ci);
+      }
+
       // Go through and re-start all of our transactions with h2 disabled.
       for (auto iter = mStreamTransactionHash.Iter(); !iter.Done(); iter.Next()) {
         nsAutoPtr<Http2Stream>& stream = iter.Data();

netwerk/protocol/http/nsHttpChannel.cpp

@@ -6536,6 +6536,11 @@ nsHttpChannel::BeginConnect()
     OriginAttributes originAttributes;
     NS_GetOriginAttributes(this, originAttributes);
 
+    RefPtr<nsHttpConnectionInfo> connInfo =
+        new nsHttpConnectionInfo(host, port, EmptyCString(), mUsername,
+                                 proxyInfo, originAttributes, isHttps);
+    mAllowAltSvc = (mAllowAltSvc && !gHttpHandler->IsSpdyBlacklisted(connInfo));
+
     RefPtr<AltSvcMapping> mapping;
     if (!mConnectionInfo && mAllowAltSvc && // per channel
         !(mLoadFlags & LOAD_FRESH_CONNECTION) &&
@@ -6589,11 +6594,18 @@ nsHttpChannel::BeginConnect()
     } else {
         LOG(("nsHttpChannel %p Using default connection info", this));
 
-        mConnectionInfo = new nsHttpConnectionInfo(host, port, EmptyCString(), mUsername, proxyInfo,
-                                                   originAttributes, isHttps);
+        mConnectionInfo = connInfo;
         Telemetry::Accumulate(Telemetry::HTTP_TRANSACTION_USE_ALTSVC, false);
     }
 
+    // Need to re-ask the handler, since mConnectionInfo may not be the connInfo
+    // we used earlier
+    if (gHttpHandler->IsSpdyBlacklisted(mConnectionInfo)) {
+        mAllowSpdy = 0;
+        mCaps |= NS_HTTP_DISALLOW_SPDY;
+        mConnectionInfo->SetNoSpdy(true);
+    }
+
     mAuthProvider = new nsHttpChannelAuthProvider();
     rv = mAuthProvider->Init(this);
     if (NS_FAILED(rv)) {

netwerk/protocol/http/nsHttpConnectionMgr.cpp

@@ -1954,6 +1954,9 @@ nsHttpConnectionMgr::ProcessNewTransaction(nsHttpTransaction *trans)
         trans->SetConnection(nullptr);
         rv = DispatchTransaction(ent, trans, conn);
     } else {
+        if (!ent->AllowSpdy()) {
+            trans->DisableSpdy();
+        }
         pendingTransInfo = new PendingTransactionInfo(trans);
         rv = TryDispatchTransaction(ent, !!trans->TunnelProvider(), pendingTransInfo);
     }
@@ -5112,6 +5115,7 @@ nsHttpConnectionMgr::nsHalfOpenSocket::OnTransportStatus(nsITransport *trans,
         gHttpHandler->IsSpdyEnabled() &&
         gHttpHandler->CoalesceSpdy() &&
         mEnt && mEnt->mConnInfo && mEnt->mConnInfo->EndToEndSSL() &&
+        mEnt->AllowSpdy() &&
         !mEnt->mConnInfo->UsingProxy() &&
         mEnt->mCoalescingKeys.IsEmpty()) {
 
@@ -5266,6 +5270,7 @@ nsHttpConnectionMgr::
 nsConnectionEntry::nsConnectionEntry(nsHttpConnectionInfo *ci)
     : mConnInfo(ci)
     , mUsingSpdy(false)
+    , mCanUseSpdy(true)
     , mPreferIPv4(false)
     , mPreferIPv6(false)
     , mUsedForConnection(false)
@@ -5402,6 +5407,42 @@ nsConnectionEntry::RemoveHalfOpen(nsHalfOpenSocket *halfOpen)
     }
 }
 
+void
+nsHttpConnectionMgr::BlacklistSpdy(const nsHttpConnectionInfo *ci)
+{
+    LOG(("nsHttpConnectionMgr::BlacklistSpdy blacklisting ci %s", ci->HashKey().BeginReading()));
+    nsConnectionEntry *ent = mCT.GetWeak(ci->HashKey());
+    if (!ent) {
+        LOG(("nsHttpConnectionMgr::BlacklistSpdy no entry found?!"));
+        return;
+    }
+
+    ent->DisallowSpdy();
+}
+
+void
+nsHttpConnectionMgr::
+nsConnectionEntry::DisallowSpdy()
+{
+    mCanUseSpdy = false;
+
+    // If we have any spdy connections, we want to go ahead and close them when
+    // they're done so we can free up some connections.
+    for (uint32_t i = 0; i < mActiveConns.Length(); ++i) {
+        if (mActiveConns[i]->UsingSpdy()) {
+            mActiveConns[i]->DontReuse();
+        }
+    }
+    for (uint32_t i = 0; i < mIdleConns.Length(); ++i) {
+        if (mIdleConns[i]->UsingSpdy()) {
+            mIdleConns[i]->DontReuse();
+        }
+    }
+
+    // Can't coalesce if we're not using spdy
+    mCoalescingKeys.Clear();
+}
+
 void
 nsHttpConnectionMgr::
 nsConnectionEntry::RecordIPFamilyPreference(uint16_t family)

netwerk/protocol/http/nsHttpConnectionMgr.h

@@ -255,6 +255,8 @@ public:
         return mCurrentTopLevelOuterContentWindowId;
     }
 
+    void BlacklistSpdy(const nsHttpConnectionInfo *ci);
+
 private:
     virtual ~nsHttpConnectionMgr();
 
@@ -310,6 +312,13 @@ private:
         // connection is currently using spdy.
         bool mUsingSpdy : 1;
 
+        // Determines whether or not we can continue to use spdy-enabled
+        // connections in the future. This is generally set to false either when
+        // some connection here encounters connection-based auth (such as NTLM)
+        // or when some connection here encounters a server that is totally
+        // busted (such as it fails to properly perform the h2 handshake).
+        bool mCanUseSpdy : 1;
+
         // Flags to remember our happy-eyeballs decision.
         // Reset only by Ctrl-F5 reload.
         // True when we've first connected an IPv4 server for this host,
@@ -327,6 +336,9 @@ private:
 
         bool mDoNotDestroy : 1;
 
+        bool AllowSpdy() const { return mCanUseSpdy; }
+        void DisallowSpdy();
+
         // Set the IP family preference flags according the connected family
         void RecordIPFamilyPreference(uint16_t family);
         // Resets all flags to their default values

netwerk/protocol/http/nsHttpHandler.cpp

@@ -2789,5 +2789,18 @@ nsHttpHandler::IsBeforeLastActiveTabLoadOptimization(TimeStamp const &when)
          when <= mLastActiveTabLoadOptimizationHit;
 }
 
+void
+nsHttpHandler::BlacklistSpdy(const nsHttpConnectionInfo *ci)
+{
+    mConnMgr->BlacklistSpdy(ci);
+    mBlacklistedSpdyOrigins.PutEntry(ci->GetOrigin());
+}
+
+bool
+nsHttpHandler::IsSpdyBlacklisted(const nsHttpConnectionInfo *ci)
+{
+    return mBlacklistedSpdyOrigins.Contains(ci->GetOrigin());
+}
+
 } // namespace net
 } // namespace mozilla

netwerk/protocol/http/nsHttpHandler.h

@@ -734,6 +734,11 @@ private:
 
 public:
     MOZ_MUST_USE nsresult NewChannelId(uint64_t& channelId);
+
+    void BlacklistSpdy(const nsHttpConnectionInfo *ci);
+    MOZ_MUST_USE bool IsSpdyBlacklisted(const nsHttpConnectionInfo *ci);
+private:
+    nsTHashtable<nsCStringHashKey> mBlacklistedSpdyOrigins;
 };
 
 extern StaticRefPtr<nsHttpHandler> gHttpHandler;

netwerk/protocol/http/nsHttpTransaction.h

@@ -138,6 +138,8 @@ public:
     void DispatchedAsBlocking();
     void RemoveDispatchedAsBlocking();
 
+    void DisableSpdy() override;
+
     nsHttpTransaction *QueryHttpTransaction() override { return this; }
 
     Http2PushedStream *GetPushedStream() { return mPushedStream; }
@@ -223,7 +225,6 @@ private:
 
     bool ResponseTimeoutEnabled() const final;
 
-    void DisableSpdy() override;
     void ReuseConnectionOnRestartOK(bool reuseOk) override { mReuseOnRestart = reuseOk; }
 
     // Called right after we parsed the response head.  Checks for connection based