From de07b3c524b3a542b25e6fac86e64c6aa167fb60 Mon Sep 17 00:00:00 2001 From: Cosmin Sabou Date: Mon, 11 Sep 2023 18:10:57 +0300 Subject: [PATCH] Backed out changeset 11fddb3ea9c0 (bug 1820807) for causing xpcshell failures on test_proxy-failover_canceled.js. CLOSED TREE --- modules/libpref/init/StaticPrefList.yaml | 6 - netwerk/base/nsIChannelEventSink.idl | 8 - netwerk/protocol/http/HttpBaseChannel.cpp | 6 - netwerk/protocol/http/HttpChannelParent.cpp | 24 +-- netwerk/protocol/http/HttpChannelParent.h | 3 - netwerk/protocol/http/nsHttpChannel.cpp | 239 +++++++--------------------- netwerk/protocol/http/nsHttpChannel.h | 8 +- netwerk/test/unit/test_authentication.js | 201 ++++++----------------- netwerk/test/unit/test_ntlm_proxy_auth.js | 3 +- 9 files changed, 119 insertions(+), 379 deletions(-) diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index 9671b2854e30..e24c40a9354d 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -11197,12 +11197,6 @@ value: true mirror: always -# whether to redirect the channel for auth redirects. See Bug 1820807 -- name: network.auth.use_redirect_for_retries - type: RelaxedAtomicBool - value: @IS_EARLY_BETA_OR_EARLIER@ - mirror: always - # Whether to allow truncated brotli with empty output. This also fixes # throwing an erroring when receiving highly compressed brotli files when # no content type is specified (Bug 1715401). This pref can be removed after diff --git a/netwerk/base/nsIChannelEventSink.idl b/netwerk/base/nsIChannelEventSink.idl index 56484e933f98..1f77cfb4668b 100644 --- a/netwerk/base/nsIChannelEventSink.idl +++ b/netwerk/base/nsIChannelEventSink.idl @@ -53,14 +53,6 @@ interface nsIChannelEventSink : nsISupports const unsigned long REDIRECT_STS_UPGRADE = 1 << 3; /** - * This is a internal redirect used to handle http authentication retries. - * Upon receiving a 401 or 407 the channel gets redirected to a new channel - * (same URL) that performs the request with the appropriate credentials. - * Auth retry to the server must be made after redirecting to a new channel - */ - const unsigned long REDIRECT_AUTH_RETRY = 1 << 4; - - /** * Called when a redirect occurs. This may happen due to an HTTP 3xx status * code. The purpose of this method is to notify the sink that a redirect * is about to happen, but also to give the sink the right to veto the diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp index 840d071f25dd..b48814810184 100644 --- a/netwerk/protocol/http/HttpBaseChannel.cpp +++ b/netwerk/protocol/http/HttpBaseChannel.cpp @@ -5864,12 +5864,6 @@ HttpBaseChannel::SetNavigationStartTimeStamp(TimeStamp aTimeStamp) { nsresult HttpBaseChannel::CheckRedirectLimit(uint32_t aRedirectFlags) const { if (aRedirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL) { - // for internal redirect due to auth retry we do not have any limit - // as we might restrict the number of times a user might retry - // authentication - if (aRedirectFlags & nsIChannelEventSink::REDIRECT_AUTH_RETRY) { - return NS_OK; - } // Some platform features, like Service Workers, depend on internal // redirects. We should allow some number of internal redirects above // and beyond the normal redirect limit so these features continue diff --git a/netwerk/protocol/http/HttpChannelParent.cpp b/netwerk/protocol/http/HttpChannelParent.cpp index bea1808939de..73f06b18e5cc 100644 --- a/netwerk/protocol/http/HttpChannelParent.cpp +++ b/netwerk/protocol/http/HttpChannelParent.cpp @@ -1747,10 +1747,6 @@ HttpChannelParent::GetRemoteType(nsACString& aRemoteType) { return NS_OK; } -bool HttpChannelParent::IsRedirectDueToAuthRetry(uint32_t redirectFlags) { - return (redirectFlags & nsIChannelEventSink::REDIRECT_AUTH_RETRY); -} - //----------------------------------------------------------------------------- // HttpChannelParent::nsIParentRedirectingChannel //----------------------------------------------------------------------------- @@ -1779,30 +1775,24 @@ HttpChannelParent::StartRedirect(nsIChannel* newChannel, uint32_t redirectFlags, return NS_BINDING_ABORTED; } - // If this is an internal redirect for service worker interception or - // internal redirect due to auth retries, then hide it from the child - // process. The original e10s interception code was not designed with this - // in mind and its not necessary to replace the HttpChannelChild/Parent - // objects in this case. + // If this is an internal redirect for service worker interception, then + // hide it from the child process. The original e10s interception code + // was not designed with this in mind and its not necessary to replace + // the HttpChannelChild/Parent objects in this case. if (redirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL) { nsCOMPtr oldIntercepted = do_QueryInterface(static_cast(mChannel.get())); nsCOMPtr newIntercepted = do_QueryInterface(newChannel); - // 1. We only want to hide the special internal redirects from - // nsHttpChannel to InterceptedHttpChannel. - // 2. We want to allow through internal redirects + // We only want to hide the special internal redirect from nsHttpChannel + // to InterceptedHttpChannel. We want to allow through internal redirects // initiated from the InterceptedHttpChannel even if they are to another // InterceptedHttpChannel, except the interception reset, since // corresponding HttpChannelChild/Parent objects can be reused for reset // case. - // 3. If this is an internal redirect due to auth retry then we will - // hide it from the child process - if ((!oldIntercepted && newIntercepted) || - (oldIntercepted && !newIntercepted && oldIntercepted->IsReset()) || - (IsRedirectDueToAuthRetry(redirectFlags))) { + (oldIntercepted && !newIntercepted && oldIntercepted->IsReset())) { // We need to move across the reserved and initial client information // to the new channel. Normally this would be handled by the child // ClientChannelHelper, but that is not notified of this redirect since diff --git a/netwerk/protocol/http/HttpChannelParent.h b/netwerk/protocol/http/HttpChannelParent.h index d30b0e0a5806..f3eb5bf01906 100644 --- a/netwerk/protocol/http/HttpChannelParent.h +++ b/netwerk/protocol/http/HttpChannelParent.h @@ -248,9 +248,6 @@ class HttpChannelParent final : public nsIInterfaceRequestor, // That is, we may suspend the channel if the ODA-s to child process are not // consumed quickly enough. Otherwise, memory explosion could happen. bool NeedFlowControl(); - - bool IsRedirectDueToAuthRetry(uint32_t redirectFlags); - int32_t mSendWindowSize; friend class HttpBackgroundChannelParent; diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 13bf7887265c..5c5ceabe10bf 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -494,7 +494,7 @@ void nsHttpChannel::HandleContinueCancellingByURLClassifier( } nsresult nsHttpChannel::OnBeforeConnect() { - nsresult rv = NS_OK; + nsresult rv; // Check if request was cancelled during suspend AFTER on-modify-request if (mCanceled) { @@ -507,6 +507,18 @@ nsresult nsHttpChannel::OnBeforeConnect() { return AsyncCall(&nsHttpChannel::HandleAsyncAPIRedirect); } + // Check to see if we should redirect this channel to the unstripped URI. To + // revert the query stripping if the loading channel is in the content + // blocking allow list. + if (ContentBlockingAllowList::Check(this)) { + nsCOMPtr unstrippedURI; + mLoadInfo->GetUnstrippedURI(getter_AddRefs(unstrippedURI)); + + if (unstrippedURI) { + return AsyncCall(&nsHttpChannel::HandleAsyncRedirectToUnstrippedURI); + } + } + // Note that we are only setting the "Upgrade-Insecure-Requests" request // header for *all* navigational requests instead of all requests as // defined in the spec, see: @@ -519,27 +531,8 @@ nsresult nsHttpChannel::OnBeforeConnect() { NS_ENSURE_SUCCESS(rv, rv); } - if (LoadAuthRedirectedChannel()) { - // This channel is a result of a redirect due to auth retry - // We have already checked for HSTS upgarde in the redirecting channel. - // We can safely skip those checks - return ContinueOnBeforeConnect(false, rv); - } - SecFetch::AddSecFetchHeader(this); - // Check to see if we should redirect this channel to the unstripped URI. To - // revert the query stripping if the loading channel is in the content - // blocking allow list. - if (ContentBlockingAllowList::Check(this)) { - nsCOMPtr unstrippedURI; - mLoadInfo->GetUnstrippedURI(getter_AddRefs(unstrippedURI)); - - if (unstrippedURI) { - return AsyncCall(&nsHttpChannel::HandleAsyncRedirectToUnstrippedURI); - } - } - nsCOMPtr resultPrincipal; if (!mURI->SchemeIs("https")) { nsContentUtils::GetSecurityManager()->GetChannelResultPrincipal( @@ -762,15 +755,6 @@ nsresult nsHttpChannel::ContinueOnBeforeConnect(bool aShouldUpgrade, mCaps |= NS_HTTP_DISALLOW_HTTPS_RR; } - if (mTransactionSticky) { - MOZ_ASSERT(LoadAuthRedirectedChannel()); - // this means this is a redirected channel channel due to auth retry and a - // connection based auth scheme was used - // we have a reference to the old-transaction with sticky connection which - // we need to use - mCaps |= NS_HTTP_STICKY_CONNECTION; - } - mCaps |= NS_HTTP_TRR_FLAGS_FROM_MODE(nsIRequest::GetTRRMode()); // Finalize ConnectionInfo flags before SpeculativeConnect @@ -944,7 +928,7 @@ nsresult nsHttpChannel::ContinueConnect() { } // hit the net... - return DoConnect(mTransactionSticky); + return DoConnect(); } nsresult nsHttpChannel::DoConnect(HttpTransactionShell* aTransWithStickyConn) { @@ -2436,11 +2420,10 @@ nsresult nsHttpChannel::ContinueProcessResponse3(nsresult rv) { // The transaction has been internally restarted. We want to // authenticate to the proxy again, so reuse either cached credentials // or use default credentials for NTLM/Negotiate. This prevents - // considering the previously used credentials as invalid. + // considering the previously used creadentials as invalid. mAuthProvider->ClearProxyIdent(); } - if (!LoadAuthRedirectedChannel() && - MOZ_UNLIKELY(LoadCustomAuthHeader()) && httpStatus == 401) { + if (MOZ_UNLIKELY(LoadCustomAuthHeader()) && httpStatus == 401) { // When a custom auth header fails, we don't want to try // any cached credentials, nor we want to ask the user. // It's up to the consumer to re-try w/o setting a custom @@ -2502,14 +2485,7 @@ nsresult nsHttpChannel::ContinueProcessResponse3(nsresult rv) { rv = ProcessNormal(); } else { mIsAuthChannel = true; - mAuthRetryPending = true; - if (StaticPrefs::network_auth_use_redirect_for_retries()) { - if (NS_SUCCEEDED(RedirectToNewChannelForAuthRetry())) { - return NS_OK; - } - mAuthRetryPending = false; - rv = ProcessNormal(); - } + mAuthRetryPending = true; // see DoAuthRetry } break; @@ -2918,114 +2894,7 @@ void nsHttpChannel::HandleAsyncRedirectToUnstrippedURI() { } } } -nsresult nsHttpChannel::RedirectToNewChannelForAuthRetry() { - LOG(("nsHttpChannel::RedirectToNewChannelForAuthRetry %p", this)); - nsresult rv = NS_OK; - - nsCOMPtr redirectLoadInfo = CloneLoadInfoForRedirect( - mURI, nsIChannelEventSink::REDIRECT_INTERNAL | - nsIChannelEventSink::REDIRECT_AUTH_RETRY); - - nsCOMPtr ioService; - rv = gHttpHandler->GetIOService(getter_AddRefs(ioService)); - NS_ENSURE_SUCCESS(rv, rv); - - nsCOMPtr newChannel; - rv = gHttpHandler->NewProxiedChannel(mURI, mProxyInfo, mProxyResolveFlags, - mProxyURI, mLoadInfo, - getter_AddRefs(newChannel)); - - NS_ENSURE_SUCCESS(rv, rv); - - rv = SetupReplacementChannel(mURI, newChannel, true, - nsIChannelEventSink::REDIRECT_INTERNAL | - nsIChannelEventSink::REDIRECT_AUTH_RETRY); - NS_ENSURE_SUCCESS(rv, rv); - - // rewind the upload stream - if (mUploadStream) { - nsCOMPtr seekable = do_QueryInterface(mUploadStream); - nsresult rv = NS_ERROR_NO_INTERFACE; - if (seekable) { - rv = seekable->Seek(nsISeekableStream::NS_SEEK_SET, 0); - } - - // This should not normally happen, but it's possible that big memory - // blobs originating in the other process can't be rewinded. - // In that case we just fail the request, otherwise the content length - // will not match and this load will never complete. - NS_ENSURE_SUCCESS(rv, rv); - } - - RefPtr httpChannelImpl = do_QueryObject(newChannel); - - MOZ_ASSERT(mAuthProvider); - httpChannelImpl->mAuthProvider = std::move(mAuthProvider); - - httpChannelImpl->mProxyInfo = mProxyInfo; - - if ((mCaps & NS_HTTP_STICKY_CONNECTION) || - mTransaction->HasStickyConnection()) { - mConnectionInfo = mTransaction->GetConnInfo(); - - httpChannelImpl->mTransactionSticky = mTransaction; - - if (mTransaction->Http2Disabled()) { - httpChannelImpl->mCaps |= NS_HTTP_DISALLOW_SPDY; - } - if (mTransaction->Http3Disabled()) { - httpChannelImpl->mCaps |= NS_HTTP_DISALLOW_HTTP3; - } - } - httpChannelImpl->mCaps |= NS_HTTP_STICKY_CONNECTION; - if (LoadAuthConnectionRestartable()) { - httpChannelImpl->mCaps |= NS_HTTP_CONNECTION_RESTARTABLE; - } else { - httpChannelImpl->mCaps &= ~NS_HTTP_CONNECTION_RESTARTABLE; - } - - MOZ_ASSERT(mConnectionInfo); - httpChannelImpl->mConnectionInfo = mConnectionInfo->Clone(); - - // we need to store the state to skip unnecessary checks in the new channel - httpChannelImpl->StoreAuthRedirectedChannel(true); - - // We must copy proxy and auth header to the new channel. - // Although the new channel can populate auth headers from auth cache, we - // would still like to use the auth headers generated in this channel. The - // main reason for doing this is that certain connection-based/stateful auth - // schemes like NTLM will fail when we try generate the credentials more than - // the number of times the server has presented us the challenge due to the - // usage of nonce in generating the credentials Copying the auth header will - // bypass generation of the credentials - nsAutoCString authVal; - if (NS_SUCCEEDED(GetRequestHeader("Proxy-Authorization"_ns, authVal))) { - httpChannelImpl->SetRequestHeader("Proxy-Authorization"_ns, authVal, false); - } - if (NS_SUCCEEDED(GetRequestHeader("Authorization"_ns, authVal))) { - httpChannelImpl->SetRequestHeader("Authorization"_ns, authVal, false); - } - - httpChannelImpl->SetBlockAuthPrompt(LoadBlockAuthPrompt()); - mRedirectChannel = newChannel; - - PushRedirectAsyncFunc(&nsHttpChannel::OpenRedirectChannel); - rv = gHttpHandler->AsyncOnChannelRedirect( - this, newChannel, - nsIChannelEventSink::REDIRECT_INTERNAL | - nsIChannelEventSink::REDIRECT_AUTH_RETRY); - - if (NS_SUCCEEDED(rv)) rv = WaitForRedirectCallback(); - - if (NS_FAILED(rv)) { - AutoRedirectVetoNotifier notifier(this, rv); - mRedirectChannel = nullptr; - PopRedirectAsyncFunc(&nsHttpChannel::OpenRedirectChannel); - } - - return rv; -} nsresult nsHttpChannel::StartRedirectChannelToURI(nsIURI* upgradedURI, uint32_t flags) { nsresult rv = NS_OK; @@ -3148,19 +3017,43 @@ nsresult nsHttpChannel::AsyncDoReplaceWithProxy(nsIProxyInfo* pi) { // Inform consumers about this fake redirect mRedirectChannel = newChannel; - PushRedirectAsyncFunc(&nsHttpChannel::OpenRedirectChannel); + PushRedirectAsyncFunc(&nsHttpChannel::ContinueDoReplaceWithProxy); rv = gHttpHandler->AsyncOnChannelRedirect(this, newChannel, flags); if (NS_SUCCEEDED(rv)) rv = WaitForRedirectCallback(); if (NS_FAILED(rv)) { AutoRedirectVetoNotifier notifier(this, rv); - PopRedirectAsyncFunc(&nsHttpChannel::OpenRedirectChannel); + PopRedirectAsyncFunc(&nsHttpChannel::ContinueDoReplaceWithProxy); } return rv; } +nsresult nsHttpChannel::ContinueDoReplaceWithProxy(nsresult rv) { + AutoRedirectVetoNotifier notifier(this, rv); + + if (NS_FAILED(rv)) return rv; + + MOZ_ASSERT(mRedirectChannel, "No redirect channel?"); + + // Make sure to do this after we received redirect veto answer, + // i.e. after all sinks had been notified + mRedirectChannel->SetOriginalURI(mOriginalURI); + + // open new channel + rv = mRedirectChannel->AsyncOpen(mListener); + NS_ENSURE_SUCCESS(rv, rv); + + mStatus = NS_BINDING_REDIRECTED; + + notifier.RedirectSucceeded(); + + ReleaseListeners(); + + return rv; +} + nsresult nsHttpChannel::ResolveProxy() { LOG(("nsHttpChannel::ResolveProxy [this=%p]\n", this)); @@ -5524,11 +5417,6 @@ NS_IMETHODIMP nsHttpChannel::OnAuthAvailable() { mTransactionPump->Resume(); } - if (StaticPrefs::network_auth_use_redirect_for_retries()) { - return CallOrWaitForResume( - [](auto* self) { return self->RedirectToNewChannelForAuthRetry(); }); - } - return NS_OK; } @@ -6181,8 +6069,7 @@ void nsHttpChannel::AsyncOpenFinal(TimeStamp aTimeStamp) { // yes, this channel will be canceled by channel classifier. Chances are the // lookup is not needed so CheckIsTrackerWithLocalTable() will return an // error and then we can MaybeResolveProxyAndBeginConnect() right away. - // We skip the check in case this is an internal redirected channel - if (!LoadAuthRedirectedChannel() && NS_ShouldClassifyChannel(this)) { + if (NS_ShouldClassifyChannel(this)) { RefPtr self = this; willCallback = NS_SUCCEEDED( AsyncUrlChannelClassifier::CheckChannel(this, [self]() -> void { @@ -6466,12 +6353,7 @@ nsresult nsHttpChannel::BeginConnect() { mConnectionInfo->SetNoSpdy(true); } - // We can be passed with the auth provider if this channel was - // a result of redirect due to auth retry - if (!mAuthProvider) { - mAuthProvider = new nsHttpChannelAuthProvider(); - } - + mAuthProvider = new nsHttpChannelAuthProvider(); rv = mAuthProvider->Init(this); if (NS_FAILED(rv)) { return rv; @@ -6534,10 +6416,8 @@ nsresult nsHttpChannel::BeginConnect() { if (mCanceled) { return mStatus; } - // skip classifier checks if this channel was the result of internal auth - // redirect - bool shouldBeClassified = - !LoadAuthRedirectedChannel() && NS_ShouldClassifyChannel(this); + + bool shouldBeClassified = NS_ShouldClassifyChannel(this); if (shouldBeClassified) { if (LoadChannelClassifierCancellationPending()) { @@ -6594,8 +6474,7 @@ nsresult nsHttpChannel::MaybeStartDNSPrefetch() { // be correct, and even when it isn't, the timing still represents _a_ // valid DNS lookup timing for the site, even if it is not _the_ // timing we used. - if ((mLoadFlags & (LOAD_NO_NETWORK_IO | LOAD_ONLY_FROM_CACHE)) || - LoadAuthRedirectedChannel()) { + if (mLoadFlags & (LOAD_NO_NETWORK_IO | LOAD_ONLY_FROM_CACHE)) { return NS_OK; } @@ -7623,8 +7502,7 @@ nsHttpChannel::OnStopRequest(nsIRequest* request, nsresult status) { if (mTransaction) { // determine if we should call DoAuthRetry - bool authRetry = (!StaticPrefs::network_auth_use_redirect_for_retries() && - mAuthRetryPending && NS_SUCCEEDED(status)); + bool authRetry = mAuthRetryPending && NS_SUCCEEDED(status); StoreStronglyFramed(mTransaction->ResponseIsComplete()); LOG(("nsHttpChannel %p has a strongly framed transaction: %d", this, LoadStronglyFramed())); @@ -7663,13 +7541,16 @@ nsHttpChannel::OnStopRequest(nsIRequest* request, nsresult status) { // the reposnse headers yet on the socket thread (found connection based // auth schema). - if ((NS_FAILED(status)) && transactionWithStickyConn) { - // Close (don't reuse) the sticky connection if this channel has been - // cancelled. There are proxy servers known to get confused when we send - // a new request over such a half-stated connection. - if (!LoadAuthConnectionRestartable()) { - LOG((" not reusing a half-authenticated sticky connection")); - transactionWithStickyConn->DontReuseConnection(); + if ((mAuthRetryPending || NS_FAILED(status)) && transactionWithStickyConn) { + if (NS_FAILED(status)) { + // Close (don't reuse) the sticky connection if it's in the middle + // of an NTLM negotiation and this channel has been cancelled. + // There are proxy servers known to get confused when we send + // a new request over such a half-stated connection. + if (!LoadAuthConnectionRestartable()) { + LOG((" not reusing a half-authenticated sticky connection")); + transactionWithStickyConn->DontReuseConnection(); + } } } @@ -8072,8 +7953,6 @@ nsresult nsHttpChannel::ContinueOnStopRequest(nsresult aStatus, bool aIsFromNet, // The prefetch needs to be released on the main thread mDNSPrefetch = nullptr; - mTransactionSticky = nullptr; - // notify "http-on-stop-connect" observers gHttpHandler->OnStopRequest(this); diff --git a/netwerk/protocol/http/nsHttpChannel.h b/netwerk/protocol/http/nsHttpChannel.h index bd2a59b7200c..cbfe0f85c89c 100644 --- a/netwerk/protocol/http/nsHttpChannel.h +++ b/netwerk/protocol/http/nsHttpChannel.h @@ -386,6 +386,7 @@ class nsHttpChannel final : public HttpBaseChannel, // proxy specific methods [[nodiscard]] nsresult ProxyFailover(); [[nodiscard]] nsresult AsyncDoReplaceWithProxy(nsIProxyInfo*); + [[nodiscard]] nsresult ContinueDoReplaceWithProxy(nsresult); [[nodiscard]] nsresult ResolveProxy(); // cache specific methods @@ -527,9 +528,6 @@ class nsHttpChannel final : public HttpBaseChannel, // resolve in firing a ServiceWorker FetchEvent. [[nodiscard]] nsresult RedirectToInterceptedChannel(); - // Start an internal redirect to a new channel for auth retry - [[nodiscard]] nsresult RedirectToNewChannelForAuthRetry(); - // Determines and sets content type in the cache entry. It's called when // writing a new entry. The content type is used in cache internally only. void SetCachedContentType(); @@ -572,7 +570,6 @@ class nsHttpChannel final : public HttpBaseChannel, nsCOMPtr mTransactionPump; RefPtr mTransaction; - RefPtr mTransactionSticky; uint64_t mLogicalOffset{0}; @@ -703,8 +700,7 @@ class nsHttpChannel final : public HttpBaseChannel, // Only set to true when we receive an HTTPSSVC record before the // transaction is created. (uint32_t, HTTPSSVCTelemetryReported, 1), - (uint32_t, EchConfigUsed, 1), - (uint32_t, AuthRedirectedChannel, 1) + (uint32_t, EchConfigUsed, 1) )) // clang-format on diff --git a/netwerk/test/unit/test_authentication.js b/netwerk/test/unit/test_authentication.js index ca6d722771b7..ae477dd65c0e 100644 --- a/netwerk/test/unit/test_authentication.js +++ b/netwerk/test/unit/test_authentication.js @@ -33,8 +33,6 @@ function AuthPrompt1(flags) { this.flags = flags; } -var initialChannelId = -1; - AuthPrompt1.prototype = { user: "guest", pass: "guest", @@ -271,20 +269,6 @@ var listener = { do_throw("Expecting an HTTP channel"); } - if ( - Services.prefs.getBoolPref("network.auth.use_redirect_for_retries") && - // we should skip redirect check if we do not expect to succeed - this.expectedCode == 200 - ) { - // ensure channel ids are initialized - Assert.notEqual(initialChannelId, -1); - - // for each request we must use a unique channel ID. - // See Bug 1820807 - var chan = request.QueryInterface(Ci.nsIIdentChannel); - Assert.notEqual(initialChannelId, chan.channelId); - } - Assert.equal(request.responseStatus, this.expectedCode); // The request should be succeeded if we expect 200 Assert.equal(request.requestSucceeded, this.expectedCode == 200); @@ -301,7 +285,7 @@ var listener = { onStopRequest: function test_onStopR(request, status) { Assert.equal(status, Cr.NS_ERROR_ABORT); - initialChannelId = -1; + this.nextTest(); }, }; @@ -324,25 +308,12 @@ function makeChan( }); } -var ChannelCreationObserver = { - QueryInterface: ChromeUtils.generateQI(["nsIObserver"]), - observe(aSubject, aTopic, aData) { - if (aTopic == "http-on-opening-request") { - initialChannelId = aSubject.QueryInterface(Ci.nsIIdentChannel).channelId; - } - }, -}; - var httpserv = null; function setup() { httpserv = new HttpServer(); httpserv.registerPathHandler("/auth", authHandler); - httpserv.registerPathHandler( - "/auth/stored/wrong/credentials/", - authHandlerWrongStoredCredentials - ); httpserv.registerPathHandler("/auth/ntlm/simple", authNtlmSimple); httpserv.registerPathHandler("/auth/realm", authRealm); httpserv.registerPathHandler("/auth/non_ascii", authNonascii); @@ -367,7 +338,6 @@ function setup() { registerCleanupFunction(async () => { await httpserv.stop(); }); - Services.obs.addObserver(ChannelCreationObserver, "http-on-opening-request"); } setup(); @@ -381,100 +351,86 @@ async function openAndListen(chan) { .clearAll(); } -async function test_noauth() { +add_task(async function test_noauth() { var chan = makeChan(URL + "/auth", URL); listener.expectedCode = 401; // Unauthorized await openAndListen(chan); -} +}); -async function test_returnfalse1() { +add_task(async function test_returnfalse1() { var chan = makeChan(URL + "/auth", URL); chan.notificationCallbacks = new Requestor(FLAG_RETURN_FALSE, 1); listener.expectedCode = 401; // Unauthorized await openAndListen(chan); -} +}); -async function test_wrongpw1() { +add_task(async function test_wrongpw1() { var chan = makeChan(URL + "/auth", URL); chan.notificationCallbacks = new Requestor(FLAG_WRONG_PASSWORD, 1); listener.expectedCode = 200; // OK await openAndListen(chan); -} +}); -async function test_prompt1() { +add_task(async function test_prompt1() { var chan = makeChan(URL + "/auth", URL); chan.notificationCallbacks = new Requestor(0, 1); listener.expectedCode = 200; // OK await openAndListen(chan); -} +}); -async function test_prompt1CrossOrigin() { +add_task(async function test_prompt1CrossOrigin() { var chan = makeChan(URL + "/auth", "http://example.org"); chan.notificationCallbacks = new Requestor(16, 1); listener.expectedCode = 200; // OK await openAndListen(chan); -} +}); -async function test_prompt2CrossOrigin() { +add_task(async function test_prompt2CrossOrigin() { var chan = makeChan(URL + "/auth", "http://example.org"); chan.notificationCallbacks = new Requestor(16, 2); listener.expectedCode = 200; // OK await openAndListen(chan); -} +}); -async function test_returnfalse2() { +add_task(async function test_returnfalse2() { var chan = makeChan(URL + "/auth", URL); chan.notificationCallbacks = new Requestor(FLAG_RETURN_FALSE, 2); listener.expectedCode = 401; // Unauthorized await openAndListen(chan); -} +}); -async function test_wrongpw2() { +add_task(async function test_wrongpw2() { var chan = makeChan(URL + "/auth", URL); chan.notificationCallbacks = new Requestor(FLAG_WRONG_PASSWORD, 2); listener.expectedCode = 200; // OK await openAndListen(chan); -} - -var requestNum = 0; -var expectedRequestNum = 0; -async function test_wrong_stored_passwd() { - // tests that we don't retry auth requests for incorrect custom credentials passed during channel creation - requestNum = 0; - expectedRequestNum = 1; - var chan = makeChan(URL + "/auth/stored/wrong/credentials/", URL); - chan.nsIHttpChannel.setRequestHeader("Authorization", "wrong_cred", false); - chan.notificationCallbacks = new Requestor(0, 1); - listener.expectedCode = 401; // Unauthorized - - await openAndListen(chan); -} +}); -async function test_prompt2() { +add_task(async function test_prompt2() { var chan = makeChan(URL + "/auth", URL); chan.notificationCallbacks = new Requestor(0, 2); listener.expectedCode = 200; // OK await openAndListen(chan); -} +}); -async function test_ntlm() { +add_task(async function test_ntlm() { var chan = makeChan(URL + "/auth/ntlm/simple", URL); chan.notificationCallbacks = new Requestor(FLAG_RETURN_FALSE, 2); listener.expectedCode = 401; // Unauthorized await openAndListen(chan); -} +}); -async function test_basicrealm() { +add_task(async function test_basicrealm() { var chan = makeChan(URL + "/auth/realm", URL); let requestor = new RealmTestRequestor(); @@ -482,97 +438,97 @@ async function test_basicrealm() { listener.expectedCode = 401; // Unauthorized await openAndListen(chan); Assert.equal(requestor.promptRealm, '"foo_bar'); -} +}); -async function test_nonascii() { +add_task(async function test_nonascii() { var chan = makeChan(URL + "/auth/non_ascii", URL); chan.notificationCallbacks = new Requestor(FLAG_NON_ASCII_USER_PASSWORD, 2); listener.expectedCode = 200; // OK await openAndListen(chan); -} +}); -async function test_digest_noauth() { +add_task(async function test_digest_noauth() { var chan = makeChan(URL + "/auth/digest_md5", URL); // chan.notificationCallbacks = new Requestor(FLAG_RETURN_FALSE, 2); listener.expectedCode = 401; // Unauthorized await openAndListen(chan); -} +}); -async function test_digest_md5() { +add_task(async function test_digest_md5() { var chan = makeChan(URL + "/auth/digest_md5", URL); chan.notificationCallbacks = new Requestor(0, 2); listener.expectedCode = 200; // OK await openAndListen(chan); -} +}); -async function test_digest_md5sess() { +add_task(async function test_digest_md5sess() { var chan = makeChan(URL + "/auth/digest_md5sess", URL); chan.notificationCallbacks = new Requestor(0, 2); listener.expectedCode = 200; // OK await openAndListen(chan); -} +}); -async function test_digest_sha256() { +add_task(async function test_digest_sha256() { var chan = makeChan(URL + "/auth/digest_sha256", URL); chan.notificationCallbacks = new Requestor(0, 2); listener.expectedCode = 200; // OK await openAndListen(chan); -} +}); -async function test_digest_sha256sess() { +add_task(async function test_digest_sha256sess() { var chan = makeChan(URL + "/auth/digest_sha256sess", URL); chan.notificationCallbacks = new Requestor(0, 2); listener.expectedCode = 200; // OK await openAndListen(chan); -} +}); -async function test_digest_sha256_md5() { +add_task(async function test_digest_sha256_md5() { var chan = makeChan(URL + "/auth/digest_sha256_md5", URL); chan.notificationCallbacks = new Requestor(0, 2); listener.expectedCode = 200; // OK await openAndListen(chan); -} +}); -async function test_digest_md5_sha256() { +add_task(async function test_digest_md5_sha256() { var chan = makeChan(URL + "/auth/digest_md5_sha256", URL); chan.notificationCallbacks = new Requestor(0, 2); listener.expectedCode = 200; // OK await openAndListen(chan); -} +}); -async function test_digest_md5_sha256_oneline() { +add_task(async function test_digest_md5_sha256_oneline() { var chan = makeChan(URL + "/auth/digest_md5_sha256_oneline", URL); chan.notificationCallbacks = new Requestor(0, 2); listener.expectedCode = 200; // OK await openAndListen(chan); -} +}); -async function test_digest_bogus_user() { +add_task(async function test_digest_bogus_user() { var chan = makeChan(URL + "/auth/digest_md5", URL); chan.notificationCallbacks = new Requestor(FLAG_BOGUS_USER, 2); listener.expectedCode = 401; // unauthorized await openAndListen(chan); -} +}); // Test header "WWW-Authenticate: Digest" - bug 1338876. -async function test_short_digest() { +add_task(async function test_short_digest() { var chan = makeChan(URL + "/auth/short_digest", URL); chan.notificationCallbacks = new Requestor(FLAG_NO_REALM, 2); listener.expectedCode = 401; // OK await openAndListen(chan); -} +}); // Test that COOP/COEP are processed even though asyncPromptAuth is cancelled. -async function test_corp_coep() { +add_task(async function test_corp_coep() { var chan = makeChan( URL + "/corp-coep", URL, @@ -592,10 +548,10 @@ async function test_corp_coep() { chan.getResponseHeader("cross-origin-opener-policy"), "same-origin" ); -} +}); // XXX(valentin): this makes tests fail if it's not run last. Why? -async function test_nonascii_xhr() { +add_task(async function test_nonascii_xhr() { await new Promise(resolve => { let xhr = new XMLHttpRequest(); xhr.open("GET", URL + "/auth/non_ascii", true, "é", "é"); @@ -608,49 +564,7 @@ async function test_nonascii_xhr() { }; xhr.send(null); }); -} - -let auth_tests = [ - test_noauth, - test_returnfalse1, - test_wrongpw1, - test_wrong_stored_passwd, - test_prompt1, - test_prompt1CrossOrigin, - test_prompt2CrossOrigin, - test_returnfalse2, - test_wrongpw2, - test_prompt2, - test_ntlm, - test_basicrealm, - test_nonascii, - test_digest_noauth, - test_digest_md5, - test_digest_md5sess, - test_digest_sha256, - test_digest_sha256sess, - test_digest_sha256_md5, - test_digest_md5_sha256, - test_digest_md5_sha256_oneline, - test_digest_bogus_user, - test_short_digest, - test_corp_coep, - test_nonascii_xhr, -]; - -for (let auth_test of auth_tests) { - add_task( - { pref_set: [["network.auth.use_redirect_for_retries", false]] }, - auth_test - ); -} - -for (let auth_test of auth_tests) { - add_task( - { pref_set: [["network.auth.use_redirect_for_retries", true]] }, - auth_test - ); -} +}); // PATH HANDLERS @@ -679,23 +593,6 @@ function authHandler(metadata, response) { response.bodyOutputStream.write(body, body.length); } -function authHandlerWrongStoredCredentials(metadata, response) { - var body; - if (++requestNum > expectedRequestNum) { - response.setStatusLine(metadata.httpVersion, 500, ""); - } else { - response.setStatusLine( - metadata.httpVersion, - 401, - "Unauthorized" + requestNum - ); - response.setHeader("WWW-Authenticate", 'Basic realm="secret"', false); - } - - body = "failed"; - response.bodyOutputStream.write(body, body.length); -} - // /auth/ntlm/simple function authNtlmSimple(metadata, response) { response.setStatusLine(metadata.httpVersion, 401, "Unauthorized"); diff --git a/netwerk/test/unit/test_ntlm_proxy_auth.js b/netwerk/test/unit/test_ntlm_proxy_auth.js index 24c7d8cedfdd..ec77e6579064 100644 --- a/netwerk/test/unit/test_ntlm_proxy_auth.js +++ b/netwerk/test/unit/test_ntlm_proxy_auth.js @@ -179,6 +179,7 @@ function failedAuth(metadata, response) { case 2: // Proxy - Expecting a type 3 Authenticate message from the client // Respond with a 407 to indicate invalid credentials + // authorization = metadata.getHeader("Proxy-Authorization"); authPrefix = authorization.substring(0, NTLM_PREFIX_LEN); Assert.equal(NTLM_TYPE3_PREFIX, authPrefix, "Expecting a Type 3 message"); @@ -221,9 +222,9 @@ function connectionReset(metadata, response) { authPrefix = authorization.substring(0, NTLM_PREFIX_LEN); Assert.equal(NTLM_TYPE3_PREFIX, authPrefix, "Expecting a Type 3 message"); ntlmTypeTwoCount++; - response.finish(); response.seizePower(); response.bodyOutPutStream.close(); + response.finish(); break; default: // Should not get any further requests on this channel -- 2.11.4.GIT