author | Honza Bambas <honzab.moz@firemni.cz> |
Wed, 26 Apr 2017 17:02:05 +0200 | |
changeset 355104 | 3c182ffc4e7655e529541d06c4b6b598c3373d6a |
parent 355103 | 6fbda056532ebc49e993d169799e2b19205b36ba |
child 355105 | c9210201f671ae0ca61e517aff6c0dcdc7419f7b |
push id | 31721 |
push user | cbook@mozilla.com |
push date | Thu, 27 Apr 2017 14:32:57 +0000 |
treeherder | mozilla-central@c0d35b1c5ab5 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | mcmanus |
bugs | 1345910 |
milestone | 55.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
|
--- a/netwerk/protocol/http/nsHttp.h +++ b/netwerk/protocol/http/nsHttp.h @@ -91,16 +91,20 @@ typedef uint8_t nsHttpVersion; // This corresponds to nsIHttpChannelInternal.beConservative // it disables any cutting edge features that we are worried might result in // interop problems with critical infrastructure #define NS_HTTP_BE_CONSERVATIVE (1<<11) // Transactions with this flag should be processed first. #define NS_HTTP_URGENT_START (1<<12) +// A sticky connection of the transaction is explicitly allowed to be restarted +// on ERROR_NET_RESET. +#define NS_HTTP_CONNECTION_RESTARTABLE (1<<13) + //----------------------------------------------------------------------------- // some default values //----------------------------------------------------------------------------- #define NS_HTTP_DEFAULT_PORT 80 #define NS_HTTPS_DEFAULT_PORT 443 #define NS_HTTP_HEADER_SEPS ", \t"
--- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -284,16 +284,17 @@ nsHttpChannel::nsHttpChannel() , mHasQueryString(0) , mConcurrentCacheAccess(0) , mIsPartialRequest(0) , mHasAutoRedirectVetoNotifier(0) , mPinCacheContent(0) , mIsCorsPreflightDone(0) , mStronglyFramed(false) , mUsedNetwork(0) + , mAuthConnectionRestartable(0) , mPushedStream(nullptr) , mLocalBlocklist(false) , mWarningReporter(nullptr) , mIsReadingFromCache(false) , mOnCacheAvailableCalled(false) , mRaceCacheWithNetwork(false) , mDidReval(false) { @@ -5662,16 +5663,24 @@ NS_IMETHODIMP nsHttpChannel::ForceNoSpdy if (!(mTransaction->Caps() & NS_HTTP_DISALLOW_SPDY)) { mTransaction->DisableSpdy(); } return NS_OK; } +NS_IMETHODIMP nsHttpChannel::ConnectionRestartable(bool aRestartable) +{ + LOG(("nsHttpChannel::ConnectionRestartable this=%p, restartable=%d", + this, aRestartable)); + mAuthConnectionRestartable = aRestartable; + return NS_OK; +} + //----------------------------------------------------------------------------- // nsHttpChannel::nsISupports //----------------------------------------------------------------------------- NS_IMPL_ADDREF_INHERITED(nsHttpChannel, HttpBaseChannel) NS_IMPL_RELEASE_INHERITED(nsHttpChannel, HttpBaseChannel) NS_INTERFACE_MAP_BEGIN(nsHttpChannel) @@ -7830,18 +7839,27 @@ nsHttpChannel::DoAuthRetry(nsAHttpConnec // rewind the upload stream if (mUploadStream) { nsCOMPtr<nsISeekableStream> seekable = do_QueryInterface(mUploadStream); if (seekable) seekable->Seek(nsISeekableStream::NS_SEEK_SET, 0); } - // set sticky connection flag - mCaps |= NS_HTTP_STICKY_CONNECTION; + // always set sticky connection flag + mCaps |= NS_HTTP_STICKY_CONNECTION; + // and when needed, allow restart regardless the sticky flag + if (mAuthConnectionRestartable) { + LOG((" connection made restartable")); + mCaps |= NS_HTTP_CONNECTION_RESTARTABLE; + mAuthConnectionRestartable = false; + } else { + LOG((" connection made non-restartable")); + mCaps &= ~NS_HTTP_CONNECTION_RESTARTABLE; + } // and create a new one... rv = SetupTransaction(); if (NS_FAILED(rv)) return rv; // transfer ownership of connection to transaction if (conn) mTransaction->SetConnection(conn);
--- a/netwerk/protocol/http/nsHttpChannel.h +++ b/netwerk/protocol/http/nsHttpChannel.h @@ -113,16 +113,17 @@ public: NS_IMETHOD GetProxyChallenges(nsACString & aChallenges) override; NS_IMETHOD GetWWWChallenges(nsACString & aChallenges) override; NS_IMETHOD SetProxyCredentials(const nsACString & aCredentials) override; NS_IMETHOD SetWWWCredentials(const nsACString & aCredentials) override; NS_IMETHOD OnAuthAvailable() override; NS_IMETHOD OnAuthCancelled(bool userCancel) override; NS_IMETHOD CloseStickyConnection() override; NS_IMETHOD ForceNoSpdy() override; + NS_IMETHOD ConnectionRestartable(bool) override; // Functions we implement from nsIHttpAuthenticableChannel but are // declared in HttpBaseChannel must be implemented in this class. We // just call the HttpBaseChannel:: impls. NS_IMETHOD GetLoadFlags(nsLoadFlags *aLoadFlags) override; NS_IMETHOD GetURI(nsIURI **aURI) override; NS_IMETHOD GetNotificationCallbacks(nsIInterfaceRequestor **aCallbacks) override; NS_IMETHOD GetLoadGroup(nsILoadGroup **aLoadGroup) override; NS_IMETHOD GetRequestMethod(nsACString& aMethod) override; @@ -623,16 +624,19 @@ private: // if the http transaction was performed (i.e. not cached) and // the result in OnStopRequest was known to be correctly delimited // by chunking, content-length, or h2 end-stream framing uint32_t mStronglyFramed : 1; // true if an HTTP transaction is created for the socket thread uint32_t mUsedNetwork : 1; + // the next authentication request can be sent on a whole new connection + uint32_t mAuthConnectionRestartable : 1; + nsTArray<nsContinueRedirectionFunc> mRedirectFuncStack; // Needed for accurate DNS timing RefPtr<nsDNSPrefetch> mDNSPrefetch; Http2PushedStream *mPushedStream; // True if the channel's principal was found on a phishing, malware, or // tracking (if tracking protection is enabled) blocklist
--- a/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp +++ b/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp @@ -775,16 +775,21 @@ nsHttpChannelAuthProvider::GetCredential realm.get(), suffix, &entry); // hold reference to the auth session state (in case we clear our // reference to the entry). nsCOMPtr<nsISupports> sessionStateGrip; if (entry) sessionStateGrip = entry->mMetaData; + // remember if we already had the continuation state. it means we are in + // the middle of the authentication exchange and the connection must be + // kept sticky then (and only then). + bool authAtProgress = !!*continuationState; + // for digest auth, maybe our cached nonce value simply timed out... bool identityInvalid; nsISupports *sessionState = sessionStateGrip; rv = auth->ChallengeReceived(mAuthChannel, challenge, proxyAuth, &sessionState, &*continuationState, @@ -798,37 +803,42 @@ nsHttpChannelAuthProvider::GetCredential // If the flag is set and identity is invalid, it means we received the first // challange for a new negotiation round after negotiating a connection based // auth failed (invalid password). // The mConnectionBased flag is set later for the newly received challenge, // so here it reflects the previous 401/7 response schema. rv = mAuthChannel->CloseStickyConnection(); MOZ_ASSERT(NS_SUCCEEDED(rv)); if (!proxyAuth) { - // We must clear proxy ident in the following scenario + explanation: - // - we are authenticating to an NTLM proxy and an NTLM server - // - we successfully authenticated to the proxy, mProxyIdent keeps - // the user name/domain and password, the identity has also been cached - // - we just threw away the connection because we are now asking for - // creds for the server (WWW auth) - // - hence, we will have to auth to the proxy again as well - // - if we didn't clear the proxy identity, it would be considered - // as non-valid and we would ask the user again ; clearing it forces - // use of the cached identity and not asking the user again - mProxyIdent.Clear(); + // We must clear proxy ident in the following scenario + explanation: + // - we are authenticating to an NTLM proxy and an NTLM server + // - we successfully authenticated to the proxy, mProxyIdent keeps + // the user name/domain and password, the identity has also been cached + // - we just threw away the connection because we are now asking for + // creds for the server (WWW auth) + // - hence, we will have to auth to the proxy again as well + // - if we didn't clear the proxy identity, it would be considered + // as non-valid and we would ask the user again ; clearing it forces + // use of the cached identity and not asking the user again + mProxyIdent.Clear(); } - mConnectionBased = false; } mConnectionBased = !!(authFlags & nsIHttpAuthenticator::CONNECTION_BASED); if (mConnectionBased) { rv = mAuthChannel->ForceNoSpdy(); MOZ_ASSERT(NS_SUCCEEDED(rv)); } + // It's legal if the peer closes the connection after the first 401/7. + // Making the connection sticky will prevent its restart giving the user + // a 'network reset' error every time. Hence, we mark the connection + // as restartable. + mAuthChannel->ConnectionRestartable(mConnectionBased && !authAtProgress); + if (identityInvalid) { if (entry) { if (ident->Equals(entry->Identity())) { if (!identFromURI) { LOG((" clearing bad auth cache entry\n")); // ok, we've already tried this user identity, so clear the // corresponding entry from the auth cache. authCache->ClearAuthEntry(scheme.get(), host,
--- a/netwerk/protocol/http/nsHttpTransaction.cpp +++ b/netwerk/protocol/http/nsHttpTransaction.cpp @@ -993,17 +993,17 @@ nsHttpTransaction::Close(nsresult reason // this bondage. Major issue may arise when there is an NTLM message auth // header on the transaction and we send it to a different NTLM authenticated // connection. It will break that connection and also confuse the channel's // auth provider, beliving the cached credentials are wrong and asking for // the password mistakenly again from the user. if ((reason == NS_ERROR_NET_RESET || reason == NS_OK || reason == psm::GetXPCOMFromNSSError(SSL_ERROR_DOWNGRADE_WITH_EARLY_DATA)) && - !(mCaps & NS_HTTP_STICKY_CONNECTION)) { + (!(mCaps & NS_HTTP_STICKY_CONNECTION) || (mCaps & NS_HTTP_CONNECTION_RESTARTABLE))) { if (mForceRestart && NS_SUCCEEDED(Restart())) { if (mResponseHead) { mResponseHead->Reset(); } mContentRead = 0; mContentLength = -1; delete mChunkedDecoder;
--- a/netwerk/protocol/http/nsIHttpAuthenticableChannel.idl +++ b/netwerk/protocol/http/nsIHttpAuthenticableChannel.idl @@ -113,9 +113,16 @@ interface nsIHttpAuthenticableChannel : */ [must_use] void closeStickyConnection(); /** * Tells the channel to not use SPDY-like protocols, since this will be * using connection-oriented auth. */ [must_use] void forceNoSpdy(); + + /** + * Tells the channel to mark the connection as allowed to restart on + * authentication retry. This is allowed when the request is a start + * of a new authentication round. + */ + void connectionRestartable(in boolean restartable); };