diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 07555121c4e9..4a4a09832587 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -2175,6 +2175,9 @@ pref("network.http.tailing.delay-max", 6000); // Total limit we delay tailed requests since a page load beginning. pref("network.http.tailing.total-max", 45000); +// Enable or disable the whole fix from bug 1563538 +pref("network.http.spdy.bug1563538", true); + pref("permissions.default.image", 1); // 1-Accept, 2-Deny, 3-dontAcceptForeign pref("network.proxy.type", 5); diff --git a/netwerk/protocol/http/TunnelUtils.cpp b/netwerk/protocol/http/TunnelUtils.cpp index e005fec56e63..7fda432a0c8b 100644 --- a/netwerk/protocol/http/TunnelUtils.cpp +++ b/netwerk/protocol/http/TunnelUtils.cpp @@ -48,6 +48,7 @@ TLSFilterTransaction::TLSFilterTransaction(nsAHttpTransaction* aWrapped, mFilterReadCode(NS_ERROR_NOT_INITIALIZED), mForce(false), mReadSegmentReturnValue(NS_OK), + mCloseReason(NS_ERROR_UNEXPECTED), mNudgeCounter(0) { MOZ_ASSERT(OnSocketThread(), "not on socket thread"); LOG(("TLSFilterTransaction ctor %p\n", this)); @@ -130,6 +131,16 @@ void TLSFilterTransaction::Close(nsresult aReason) { } mTransaction->Close(aReason); mTransaction = nullptr; + + if (gHttpHandler->Bug1563538()) { + if (NS_FAILED(aReason)) { + mCloseReason = aReason; + } else { + mCloseReason = NS_BASE_STREAM_CLOSED; + } + } else { + MOZ_ASSERT(NS_ERROR_UNEXPECTED == mCloseReason); + } } nsresult TLSFilterTransaction::OnReadSegment(const char* aData, uint32_t aCount, @@ -326,7 +337,7 @@ nsresult TLSFilterTransaction::ReadSegments(nsAHttpSegmentReader* aReader, LOG(("TLSFilterTransaction::ReadSegments %p max=%d\n", this, aCount)); if (!mTransaction) { - return NS_ERROR_UNEXPECTED; + return mCloseReason; } mReadSegmentReturnValue = NS_OK; @@ -351,7 +362,7 @@ nsresult TLSFilterTransaction::WriteSegments(nsAHttpSegmentWriter* aWriter, LOG(("TLSFilterTransaction::WriteSegments %p max=%d\n", this, aCount)); if (!mTransaction) { - return NS_ERROR_UNEXPECTED; + return mCloseReason; } mSegmentWriter = aWriter; @@ -649,6 +660,11 @@ nsresult TLSFilterTransaction::SetProxiedTransaction( this, aTrans, aSpdyConnectTransaction)); mTransaction = aTrans; + + // Reverting mCloseReason to the default value for consistency to indicate we + // are no longer in closed state. + mCloseReason = NS_ERROR_UNEXPECTED; + nsCOMPtr callbacks; mTransaction->GetSecurityCallbacks(getter_AddRefs(callbacks)); nsCOMPtr secCtrl(do_QueryInterface(mSecInfo)); @@ -829,23 +845,6 @@ SpdyConnectTransaction* TLSFilterTransaction::QuerySpdyConnectTransaction() { return mTransaction->QuerySpdyConnectTransaction(); } -class SocketTransportShim : public nsISocketTransport { - public: - NS_DECL_THREADSAFE_ISUPPORTS - NS_DECL_NSITRANSPORT - NS_DECL_NSISOCKETTRANSPORT - - explicit SocketTransportShim(nsISocketTransport* aWrapped, bool aIsWebsocket) - : mWrapped(aWrapped), mIsWebsocket(aIsWebsocket){}; - - private: - virtual ~SocketTransportShim() = default; - - nsCOMPtr mWrapped; - bool mIsWebsocket; - nsCOMPtr mSecurityCallbacks; -}; - class WeakTransProxy final : public nsISupports { public: NS_DECL_THREADSAFE_ISUPPORTS @@ -891,6 +890,33 @@ class WeakTransFreeProxy final : public Runnable { RefPtr mProxy; }; +class SocketTransportShim : public nsISocketTransport { + public: + NS_DECL_THREADSAFE_ISUPPORTS + NS_DECL_NSITRANSPORT + NS_DECL_NSISOCKETTRANSPORT + + explicit SocketTransportShim(SpdyConnectTransaction* aTrans, + nsISocketTransport* aWrapped, bool aIsWebsocket) + : mWrapped(aWrapped), mIsWebsocket(aIsWebsocket) { + mWeakTrans = new WeakTransProxy(aTrans); + } + + private: + virtual ~SocketTransportShim() { + if (!OnSocketThread()) { + RefPtr p = new WeakTransFreeProxy(mWeakTrans); + mWeakTrans = nullptr; + p->Dispatch(); + } + } + + nsCOMPtr mWrapped; + bool mIsWebsocket; + nsCOMPtr mSecurityCallbacks; + RefPtr mWeakTrans; // SpdyConnectTransaction * +}; + class OutputStreamShim : public nsIAsyncOutputStream { public: NS_DECL_THREADSAFE_ISUPPORTS @@ -1030,7 +1056,7 @@ void SpdyConnectTransaction::MapStreamToHttpConnection( int32_t httpResponseCode) { mConnInfo = aConnInfo; - mTunnelTransport = new SocketTransportShim(aTransport, mIsWebsocket); + mTunnelTransport = new SocketTransportShim(this, aTransport, mIsWebsocket); mTunnelStreamIn = new InputStreamShim(this, mIsWebsocket); mTunnelStreamOut = new OutputStreamShim(this, mIsWebsocket); mTunneledConn = new nsHttpConnection(); @@ -1801,7 +1827,36 @@ NS_IMETHODIMP SocketTransportShim::Close(nsresult aReason) { if (mIsWebsocket) { LOG3(("WARNING: SocketTransportShim::Close %p", this)); + } else { + LOG(("SocketTransportShim::Close %p", this)); } + + if (gHttpHandler->Bug1563538()) { + // Must always post, because mSession->CloseTransaction releases the + // Http2Stream which is still on stack. + RefPtr self(this); + + nsCOMPtr sts = + do_GetService("@mozilla.org/network/socket-transport-service;1"); + Unused << sts->Dispatch(NS_NewRunnableFunction( + "SocketTransportShim::Close", [self = std::move(self), aReason]() { + RefPtr baseTrans = + self->mWeakTrans->QueryTransaction(); + if (!baseTrans) { + return; + } + SpdyConnectTransaction* trans = + baseTrans->QuerySpdyConnectTransaction(); + MOZ_ASSERT(trans); + if (!trans) { + return; + } + + trans->mSession->CloseTransaction(trans, aReason); + })); + return NS_OK; + } + return NS_ERROR_NOT_IMPLEMENTED; } diff --git a/netwerk/protocol/http/TunnelUtils.h b/netwerk/protocol/http/TunnelUtils.h index a0fc7722e367..b571372d7cfd 100644 --- a/netwerk/protocol/http/TunnelUtils.h +++ b/netwerk/protocol/http/TunnelUtils.h @@ -182,6 +182,11 @@ class TLSFilterTransaction final : public nsAHttpTransaction, nsresult mFilterReadCode; bool mForce; nsresult mReadSegmentReturnValue; + // Before Close() is called this is NS_ERROR_UNEXPECTED, in Close() we either + // take the reason, if it is a failure, or we change to + // NS_ERROR_BASE_STREAM_CLOSE. This is returned when Write/ReadSegments is + // called after Close, when we don't have mTransaction any more. + nsresult mCloseReason; uint32_t mNudgeCounter; }; @@ -227,6 +232,7 @@ class SpdyConnectTransaction final : public NullHttpTransaction { void SetConnRefTaken(); private: + friend class SocketTransportShim; friend class InputStreamShim; friend class OutputStreamShim; diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp index 3504ae3598b2..9b2870ed912b 100644 --- a/netwerk/protocol/http/nsHttpHandler.cpp +++ b/netwerk/protocol/http/nsHttpHandler.cpp @@ -274,6 +274,7 @@ nsHttpHandler::nsHttpHandler() mTCPKeepaliveLongLivedIdleTimeS(600), mEnforceH1Framing(FRAMECHECK_BARELY), mDefaultHpackBuffer(4096), + mBug1563538(true), mMaxHttpResponseHeaderSize(393216), mFocusedWindowTransactionRatio(0.9f), mSpeculativeConnectEnabled(false), @@ -1878,6 +1879,13 @@ void nsHttpHandler::PrefsChanged(const char* pref) { } } + if (PREF_CHANGED(HTTP_PREF("spdy.bug1563538"))) { + rv = Preferences::GetBool(HTTP_PREF("spdy.bug1563538"), &cVar); + if (NS_SUCCEEDED(rv)) { + mBug1563538 = cVar; + } + } + // Enable HTTP response timeout if TCP Keepalives are disabled. mResponseTimeoutEnabled = !mTCPKeepaliveShortLivedEnabled && !mTCPKeepaliveLongLivedEnabled; diff --git a/netwerk/protocol/http/nsHttpHandler.h b/netwerk/protocol/http/nsHttpHandler.h index c14e209a1ffc..b413aaae94e3 100644 --- a/netwerk/protocol/http/nsHttpHandler.h +++ b/netwerk/protocol/http/nsHttpHandler.h @@ -429,6 +429,8 @@ class nsHttpHandler final : public nsIHttpProtocolHandler, uint32_t DefaultHpackBuffer() const { return mDefaultHpackBuffer; } + bool Bug1563538() const { return mBug1563538; } + uint32_t MaxHttpResponseHeaderSize() const { return mMaxHttpResponseHeaderSize; } @@ -671,6 +673,9 @@ class nsHttpHandler final : public nsIHttpProtocolHandler, // The default size (in bytes) of the HPACK decompressor table. uint32_t mDefaultHpackBuffer; + // Pref for the whole fix that bug provides + Atomic mBug1563538; + // The max size (in bytes) for received Http response header. uint32_t mMaxHttpResponseHeaderSize;