Bug 1355858 - Blacklist hosts from spdy who are very poorly behaved. r=dragana
authorNicholas Hurley <hurley@mozilla.com>
Thu, 25 Oct 2018 20:52:59 +0000
changeset 443053 54a37700d0e65b0a5a30aa920ae41b35cbeb7633
parent 443052 8ee3ec2214429e42c473844b86c8c1cbd0d94d15
child 443054 491959b9aaf798e151d2a6188efd8582e7629072
push id34935
push userccoroiu@mozilla.com
push dateFri, 26 Oct 2018 04:43:55 +0000
treeherdermozilla-central@d0b577458d53 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana
bugs1355858, 1050329
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 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
netwerk/protocol/http/Http2Session.cpp
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
netwerk/protocol/http/nsHttpHandler.cpp
netwerk/protocol/http/nsHttpHandler.h
netwerk/protocol/http/nsHttpTransaction.h
--- a/netwerk/protocol/http/Http2Session.cpp
+++ b/netwerk/protocol/http/Http2Session.cpp
@@ -3165,16 +3165,23 @@ Http2Session::WriteSegmentsAgain(nsAHttp
             mExpectedPushPromiseID));
       return SessionError(PROTOCOL_ERROR);
     }
 
     if (mDownstreamState == BUFFERING_OPENING_SETTINGS &&
         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();
         stream->Transaction()->DisableSpdy();
         CloseStream(stream, NS_ERROR_NET_RESET);
       }
       mStreamTransactionHash.Clear();
       return SessionError(PROTOCOL_ERROR);
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -6531,16 +6531,21 @@ nsHttpChannel::BeginConnect()
     mRequestHead.SetOrigin(scheme, host, port);
 
     SetOriginHeader();
     SetDoNotTrack();
 
     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) &&
         AltSvcMapping::AcceptableProxy(proxyInfo) &&
         (scheme.EqualsLiteral("http") || scheme.EqualsLiteral("https")) &&
         (mapping = gHttpHandler->GetAltServiceMapping(scheme,
                                                       host, port,
                                                       mPrivateBrowsing,
@@ -6584,21 +6589,28 @@ nsHttpChannel::BeginConnect()
         Telemetry::Accumulate(Telemetry::HTTP_TRANSACTION_USE_ALTSVC, true);
         Telemetry::Accumulate(Telemetry::HTTP_TRANSACTION_USE_ALTSVC_OE, !isHttps);
     } else if (mConnectionInfo) {
         LOG(("nsHttpChannel %p Using channel supplied connection info", this));
         Telemetry::Accumulate(Telemetry::HTTP_TRANSACTION_USE_ALTSVC, false);
     } 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)) {
         return rv;
     }
 
     // check to see if authorization headers should be included
     // mCustomAuthHeader is set in AsyncOpen if we find Authorization header
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -1949,16 +1949,19 @@ nsHttpConnectionMgr::ProcessNewTransacti
             MOZ_ASSERT(!conn->IsExperienced());
 
             AddActiveConn(conn, ent); // make it active
         }
 
         trans->SetConnection(nullptr);
         rv = DispatchTransaction(ent, trans, conn);
     } else {
+        if (!ent->AllowSpdy()) {
+            trans->DisableSpdy();
+        }
         pendingTransInfo = new PendingTransactionInfo(trans);
         rv = TryDispatchTransaction(ent, !!trans->TunnelProvider(), pendingTransInfo);
     }
 
     if (NS_SUCCEEDED(rv)) {
         LOG(("  ProcessNewTransaction Dispatch Immediately trans=%p\n", trans));
         return rv;
     }
@@ -5107,16 +5110,17 @@ nsHttpConnectionMgr::nsHalfOpenSocket::O
     // for this entry before then make the hash key if our dns lookup
     // just completed. We can't do coalescing if using a proxy because the
     // ip addresses are not available to the client.
 
     if (status == NS_NET_STATUS_CONNECTING_TO &&
         gHttpHandler->IsSpdyEnabled() &&
         gHttpHandler->CoalesceSpdy() &&
         mEnt && mEnt->mConnInfo && mEnt->mConnInfo->EndToEndSSL() &&
+        mEnt->AllowSpdy() &&
         !mEnt->mConnInfo->UsingProxy() &&
         mEnt->mCoalescingKeys.IsEmpty()) {
 
         nsCOMPtr<nsIDNSRecord> dnsRecord(do_GetInterface(mSocketTransport));
         nsTArray<NetAddr> addressSet;
         nsresult rv = NS_ERROR_NOT_AVAILABLE;
         if (dnsRecord) {
             rv = dnsRecord->GetAddresses(addressSet);
@@ -5261,16 +5265,17 @@ ConnectionHandle::TopLevelOuterContentWi
 }
 
 // nsConnectionEntry
 
 nsHttpConnectionMgr::
 nsConnectionEntry::nsConnectionEntry(nsHttpConnectionInfo *ci)
     : mConnInfo(ci)
     , mUsingSpdy(false)
+    , mCanUseSpdy(true)
     , mPreferIPv4(false)
     , mPreferIPv6(false)
     , mUsedForConnection(false)
     , mDoNotDestroy(false)
 {
     MOZ_COUNT_CTOR(nsConnectionEntry);
 
     if (mConnInfo->FirstHopSSL()) {
@@ -5398,16 +5403,52 @@ nsConnectionEntry::RemoveHalfOpen(nsHalf
         if (NS_FAILED(rv)) {
             LOG(("nsHttpConnectionMgr::nsConnectionEntry::RemoveHalfOpen\n"
                  "    failed to process pending queue\n"));
         }
     }
 }
 
 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)
 {
   LOG(("nsConnectionEntry::RecordIPFamilyPreference %p, af=%u", this, family));
 
   if (family == PR_AF_INET && !mPreferIPv6) {
     mPreferIPv4 = true;
   }
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -250,16 +250,18 @@ public:
     // NOTE: relatively expensive to call, there are two hashtable lookups.
     bool IsConnEntryUnderPressure(nsHttpConnectionInfo*);
 
     uint64_t CurrentTopLevelOuterContentWindowId()
     {
         return mCurrentTopLevelOuterContentWindowId;
     }
 
+    void BlacklistSpdy(const nsHttpConnectionInfo *ci);
+
 private:
     virtual ~nsHttpConnectionMgr();
 
     class nsHalfOpenSocket;
     class PendingTransactionInfo;
 
     // nsConnectionEntry
     //
@@ -305,16 +307,23 @@ private:
         //
         nsTArray<nsCString> mCoalescingKeys;
 
         // To have the UsingSpdy flag means some host with the same connection
         // entry has done NPN=spdy/* at some point. It does not mean every
         // 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,
         // initially false.
         bool mPreferIPv4 : 1;
         // True when we've first connected an IPv6 server for this host,
         // initially false.
         bool mPreferIPv6 : 1;
@@ -322,16 +331,19 @@ private:
         // True if this connection entry has initiated a socket
         bool mUsedForConnection : 1;
 
         // Try using TCP Fast Open.
         bool mUseFastOpen : 1;
 
         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
         void ResetIPFamilyPreference();
         // True iff there is currently an established IP family preference
         bool PreferenceKnown() const;
 
         // Return the count of pending transactions for all window ids.
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -2784,10 +2784,23 @@ bool
 nsHttpHandler::IsBeforeLastActiveTabLoadOptimization(TimeStamp const &when)
 {
   MutexAutoLock lock(mLastActiveTabLoadOptimizationLock);
 
   return !mLastActiveTabLoadOptimizationHit.IsNull() &&
          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
--- a/netwerk/protocol/http/nsHttpHandler.h
+++ b/netwerk/protocol/http/nsHttpHandler.h
@@ -729,16 +729,21 @@ private:
     // value from ipc onstoprequest arguments.  This is a sufficent way of passing
     // it down to the content process, since the value will be used only after
     // onstoprequest notification coming from an http channel.
     Mutex mLastActiveTabLoadOptimizationLock;
     TimeStamp mLastActiveTabLoadOptimizationHit;
 
 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;
 
 //-----------------------------------------------------------------------------
 // nsHttpsHandler - thin wrapper to distinguish the HTTP handler from the
 //                  HTTPS handler (even though they share the same impl).
 //-----------------------------------------------------------------------------
--- a/netwerk/protocol/http/nsHttpTransaction.h
+++ b/netwerk/protocol/http/nsHttpTransaction.h
@@ -133,16 +133,18 @@ public:
     const TimeStamp GetPendingTime() { return mPendingTime; }
 
     // overload of nsAHttpTransaction::RequestContext()
     nsIRequestContext *RequestContext() override { return mRequestContext.get(); }
     void SetRequestContext(nsIRequestContext *aRequestContext);
     void DispatchedAsBlocking();
     void RemoveDispatchedAsBlocking();
 
+    void DisableSpdy() override;
+
     nsHttpTransaction *QueryHttpTransaction() override { return this; }
 
     Http2PushedStream *GetPushedStream() { return mPushedStream; }
     Http2PushedStream *TakePushedStream()
     {
         Http2PushedStream *r = mPushedStream;
         mPushedStream = nullptr;
         return r;
@@ -218,17 +220,16 @@ private:
     static MOZ_MUST_USE nsresult WritePipeSegment(nsIOutputStream *, void *,
                                                   char *, uint32_t, uint32_t,
                                                   uint32_t *);
 
     bool TimingEnabled() const { return mCaps & NS_HTTP_TIMING_ENABLED; }
 
     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
     // authentication schemes in reponse headers for WWW and Proxy authentication.
     // If such is found in any of them, NS_HTTP_STICKY_CONNECTION is set in mCaps.
     // We need the sticky flag be set early to keep the connection from very start
     // of the authentication process.
     void CheckForStickyAuthScheme();