commit d654fc7328fe6de7722153f09ac1ea4be92dbf98
parent 574d358f3cbc2352c09260cdb53d973000fcc0d4
Author: Randell Jesup <rjesup@mozilla.com>
Date: Fri, 19 Dec 2025 17:32:43 +0000
Bug 2005971: Retry requests failed because of a corrupt Compression Dictionary r=necko-reviewers,valentin
Differential Revision: https://phabricator.services.mozilla.com/D276398
Diffstat:
4 files changed, 327 insertions(+), 48 deletions(-)
diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -132,9 +132,11 @@ static bool IsHeaderBlacklistedForRedirectCopy(nsHttpAtom const& aHeader) {
&nsHttp::Alternate_Service_Used,
&nsHttp::Authentication,
&nsHttp::Authorization,
+ &nsHttp::Available_Dictionary,
&nsHttp::Connection,
&nsHttp::Content_Length,
&nsHttp::Cookie,
+ &nsHttp::Dictionary_Id,
&nsHttp::Host,
&nsHttp::If,
&nsHttp::If_Match,
diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -649,6 +649,22 @@ nsresult nsHttpChannel::PrepareToConnect() {
mURI, mLoadInfo->GetExternalContentPolicyType(), &mRequestHead, IsHTTPS(),
this, nsHttpChannel::StaticSuspend,
[self = RefPtr(this)](bool aNeedsResume, DictionaryCacheEntry* aDict) {
+ // Don't use dictionary if we're retrying after hash mismatch
+ if (self->mDictionaryDisabledForRetry) {
+ LOG_DICTIONARIES(("Dictionary disabled for retry, skipping [this=%p]",
+ self.get()));
+ if (self->mUsingDictionary) {
+ self->mDictDecompress->UseCompleted();
+ self->mUsingDictionary = false;
+ }
+ self->mDictDecompress = nullptr;
+ if (aNeedsResume) {
+ self->Resume();
+ }
+ // Return false to prevent Available-Dictionary header from being
+ // added
+ return false;
+ }
self->mDictDecompress = aDict;
if (aNeedsResume) {
LOG_DICTIONARIES(("Resuming after getting Dictionary headers"));
@@ -689,6 +705,37 @@ nsresult nsHttpChannel::PrepareToConnect() {
self->mUsingDictionary = false;
}
self->mDictDecompress = nullptr;
+
+ // On hash mismatch, set up redirect to retry without
+ // dictionary. The request may have already been sent
+ // with Available-Dictionary header, so we need to cancel
+ // (or more to the point ignore) and retry without it.
+ if (aResult == NS_ERROR_CORRUPTED_CONTENT) {
+ LOG(
+ ("nsHttpChannel::Prefetch hash mismatch, setting up "
+ "retry [this=%p]",
+ self.get()));
+ // Set up redirect channel - it will be opened in
+ // OnStopRequest
+ nsresult rv =
+ self->RedirectToNewChannelForDictionaryRetry();
+ if (NS_SUCCEEDED(rv)) {
+ // Redirect set up successfully. Resume if suspended so
+ // OnStopRequest can be called and open the redirect
+ // channel.
+ if (self->mSuspendedForDictionary) {
+ self->mSuspendedForDictionary = false;
+ self->Resume();
+ }
+ return;
+ }
+ // If redirect setup failed, fall through to cancel
+ LOG(
+ ("nsHttpChannel::Prefetch redirect setup failed "
+ "[this=%p rv=0x%" PRIx32 "]",
+ self.get(), static_cast<uint32_t>(rv)));
+ }
+
if (self->mSuspendedForDictionary) {
self->mSuspendedForDictionary = false;
self->Cancel(aResult);
@@ -2315,6 +2362,14 @@ void nsHttpChannel::SetCachedContentType() {
nsresult nsHttpChannel::CallOnStartRequest() {
LOG(("nsHttpChannel::CallOnStartRequest [this=%p]", this));
+ // If dictionary retry is pending, don't call OnStartRequest on the original
+ // channel. The redirect channel will call OnStartRequest instead.
+ if (mDictionaryRetryPending) {
+ LOG(("CallOnStartRequest skipped due to pending dictionary retry [this=%p]",
+ this));
+ return NS_OK;
+ }
+
MOZ_RELEASE_ASSERT(!LoadRequireCORSPreflight() || LoadIsCorsPreflightDone(),
"CORS preflight must have been finished by the time we "
"call OnStartRequest");
@@ -3892,29 +3947,26 @@ void nsHttpChannel::HandleAsyncRedirectToUnstrippedURI() {
}
}
}
-nsresult nsHttpChannel::RedirectToNewChannelForAuthRetry() {
- LOG(("nsHttpChannel::RedirectToNewChannelForAuthRetry %p", this));
- nsresult rv = NS_OK;
+nsresult nsHttpChannel::RedirectToNewChannelInternal(
+ uint32_t redirectFlags,
+ std::function<nsresult(nsHttpChannel*)> setupCallback) {
+ LOG(("nsHttpChannel::RedirectToNewChannelInternal [this=%p, flags=0x%x]",
+ this, redirectFlags));
- nsCOMPtr<nsILoadInfo> redirectLoadInfo = CloneLoadInfoForRedirect(
- mURI, nsIChannelEventSink::REDIRECT_INTERNAL |
- nsIChannelEventSink::REDIRECT_AUTH_RETRY);
+ nsCOMPtr<nsILoadInfo> redirectLoadInfo =
+ CloneLoadInfoForRedirect(mURI, redirectFlags);
nsCOMPtr<nsIIOService> ioService;
-
- rv = gHttpHandler->GetIOService(getter_AddRefs(ioService));
+ nsresult rv = gHttpHandler->GetIOService(getter_AddRefs(ioService));
NS_ENSURE_SUCCESS(rv, rv);
nsCOMPtr<nsIChannel> 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);
+ rv = SetupReplacementChannel(mURI, newChannel, true, redirectFlags);
NS_ENSURE_SUCCESS(rv, rv);
// rewind the upload stream
@@ -3934,70 +3986,112 @@ nsresult nsHttpChannel::RedirectToNewChannelForAuthRetry() {
RefPtr<nsHttpChannel> httpChannelImpl = do_QueryObject(newChannel);
+ rv = setupCallback(httpChannelImpl);
+ NS_ENSURE_SUCCESS(rv, rv);
+
+ mRedirectChannel = newChannel;
+
+ rv = gHttpHandler->AsyncOnChannelRedirect(this, newChannel, redirectFlags);
+
+ if (NS_SUCCEEDED(rv)) {
+ rv = WaitForRedirectCallback();
+ }
+
+ if (NS_FAILED(rv)) {
+ AutoRedirectVetoNotifier notifier(this, rv);
+ mRedirectChannel = nullptr;
+ }
+
+ return rv;
+}
+
+nsresult nsHttpChannel::SetupChannelForAuthRetry() {
MOZ_ASSERT(mAuthProvider);
- httpChannelImpl->mAuthProvider = std::move(mAuthProvider);
+ mAuthProvider = std::move(mAuthProvider);
- httpChannelImpl->mProxyInfo = mProxyInfo;
+ mProxyInfo = mProxyInfo;
if ((mCaps & NS_HTTP_STICKY_CONNECTION) ||
mTransaction->HasStickyConnection()) {
mConnectionInfo = mTransaction->GetConnInfo();
- httpChannelImpl->mTransactionSticky = mTransaction;
+ mTransactionSticky = mTransaction;
if (mTransaction->Http2Disabled()) {
- httpChannelImpl->mCaps |= NS_HTTP_DISALLOW_SPDY;
+ mCaps |= NS_HTTP_DISALLOW_SPDY;
}
if (mTransaction->Http3Disabled()) {
- httpChannelImpl->mCaps |= NS_HTTP_DISALLOW_HTTP3;
+ mCaps |= NS_HTTP_DISALLOW_HTTP3;
}
}
- // always set sticky connection flag
- httpChannelImpl->mCaps |= NS_HTTP_STICKY_CONNECTION;
+ mCaps |= NS_HTTP_STICKY_CONNECTION;
if (LoadAuthConnectionRestartable()) {
- httpChannelImpl->mCaps |= NS_HTTP_CONNECTION_RESTARTABLE;
+ mCaps |= NS_HTTP_CONNECTION_RESTARTABLE;
} else {
- httpChannelImpl->mCaps &= ~NS_HTTP_CONNECTION_RESTARTABLE;
+ mCaps &= ~NS_HTTP_CONNECTION_RESTARTABLE;
}
MOZ_ASSERT(mConnectionInfo);
- httpChannelImpl->mConnectionInfo = mConnectionInfo->Clone();
+ mConnectionInfo = mConnectionInfo->Clone();
- // we need to store the state to skip unnecessary checks in the new channel
- httpChannelImpl->StoreAuthRedirectedChannel(true);
+ // we need to store the state to skip unnecessary checks in the new
+ // channel
+ 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
+ // 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);
+ SetRequestHeader("Proxy-Authorization"_ns, authVal, false);
}
if (NS_SUCCEEDED(GetRequestHeader("Authorization"_ns, authVal))) {
- httpChannelImpl->SetRequestHeader("Authorization"_ns, authVal, false);
+ SetRequestHeader("Authorization"_ns, authVal, false);
}
- httpChannelImpl->SetBlockAuthPrompt(LoadBlockAuthPrompt());
- mRedirectChannel = newChannel;
+ SetBlockAuthPrompt(LoadBlockAuthPrompt());
- rv = gHttpHandler->AsyncOnChannelRedirect(
- this, newChannel,
+ return NS_OK;
+}
+
+nsresult nsHttpChannel::RedirectToNewChannelForAuthRetry() {
+ LOG(("nsHttpChannel::RedirectToNewChannelForAuthRetry %p", this));
+
+ return RedirectToNewChannelInternal(
nsIChannelEventSink::REDIRECT_INTERNAL |
- nsIChannelEventSink::REDIRECT_AUTH_RETRY);
+ nsIChannelEventSink::REDIRECT_AUTH_RETRY,
+ [this](nsHttpChannel* httpChannelImpl) -> nsresult {
+ return httpChannelImpl->SetupChannelForAuthRetry();
+ });
+}
- if (NS_SUCCEEDED(rv)) rv = WaitForRedirectCallback();
+nsresult nsHttpChannel::RedirectToNewChannelForDictionaryRetry() {
+ LOG(("nsHttpChannel::RedirectToNewChannelForDictionaryRetry [this=%p]",
+ this));
+
+ // Don't retry if it's POST, PUT, etc (pretty unlikely)
+ if (!mRequestHead.IsSafeMethod()) {
+ return NS_ERROR_FAILURE;
+ }
- // redirected channel will be opened after we receive the OnStopRequest
+ mDictionaryRetryPending = true;
+
+ nsresult rv = RedirectToNewChannelInternal(
+ nsIChannelEventSink::REDIRECT_INTERNAL,
+ [](nsHttpChannel* httpChannelImpl) -> nsresult {
+ httpChannelImpl->mDictionaryDisabledForRetry = true;
+ return NS_OK;
+ });
if (NS_FAILED(rv)) {
- AutoRedirectVetoNotifier notifier(this, rv);
- mRedirectChannel = nullptr;
+ mDictionaryRetryPending = false;
}
return rv;
@@ -10357,6 +10451,28 @@ nsresult nsHttpChannel::ContinueOnStopRequest(nsresult aStatus, bool aIsFromNet,
mAuthRetryPending = false;
}
+ // Handle dictionary retry - similar to auth retry above
+ if (mDictionaryRetryPending) {
+ nsresult rv = OpenRedirectChannel(aStatus);
+ LOG(("Opening redirect channel for dictionary retry %x",
+ static_cast<uint32_t>(rv)));
+ if (NS_FAILED(rv)) {
+ if (mListener) {
+ MOZ_ASSERT(!LoadOnStartRequestCalled(),
+ "We should not call OnStartRequest twice.");
+ if (!LoadOnStartRequestCalled()) {
+ nsCOMPtr<nsIStreamListener> listener(mListener);
+ StoreOnStartRequestCalled(true);
+ listener->OnStartRequest(this);
+ }
+ } else {
+ StoreOnStartRequestCalled(true);
+ NS_WARNING("OnStartRequest skipped because of null listener");
+ }
+ }
+ mDictionaryRetryPending = false;
+ }
+
if (LoadUsedNetwork() && !mReportedNEL) {
MaybeGenerateNELReport();
}
@@ -10469,7 +10585,8 @@ nsHttpChannel::OnDataAvailable(nsIRequest* request, nsIInputStream* input,
// don't send out OnDataAvailable notifications if we've been canceled.
if (mCanceled) return mStatus;
- if (mAuthRetryPending || WRONG_RACING_RESPONSE_SOURCE(request) ||
+ if (mAuthRetryPending || mDictionaryRetryPending ||
+ WRONG_RACING_RESPONSE_SOURCE(request) ||
(request == mTransactionPump && LoadTransactionReplaced())) {
uint32_t n;
return input->ReadSegments(NS_DiscardSegment, nullptr, count, &n);
@@ -11244,11 +11361,14 @@ nsHttpChannel::OnRedirectVerifyCallback(nsresult result) {
if (!LoadWaitingForRedirectCallback()) {
// We are not waiting for the callback. At this moment we must release
// reference to the redirect target channel, otherwise we may leak.
- // However, if we are redirecting due to auth retries, we open the
- // redirected channel after OnStopRequest. In that case we should not
- // release the reference
- if (!StaticPrefs::network_auth_use_redirect_for_retries() ||
- !mAuthRetryPending) {
+ // However, if we are redirecting due to auth retries or dictionary retries,
+ // we open the redirected channel after OnStopRequest. In that case we
+ // should not release the reference.
+ bool keepRedirectChannel =
+ (StaticPrefs::network_auth_use_redirect_for_retries() &&
+ mAuthRetryPending) ||
+ mDictionaryRetryPending;
+ if (!keepRedirectChannel) {
mRedirectChannel = nullptr;
}
}
diff --git a/netwerk/protocol/http/nsHttpChannel.h b/netwerk/protocol/http/nsHttpChannel.h
@@ -546,14 +546,25 @@ class nsHttpChannel final : public HttpBaseChannel,
// resolve in firing a ServiceWorker FetchEvent.
[[nodiscard]] nsresult RedirectToInterceptedChannel();
+ // the internals to set up for AuthRetry
+ [[nodiscard]] nsresult SetupChannelForAuthRetry();
+
// Start an internal redirect to a new channel for auth retry
[[nodiscard]] nsresult RedirectToNewChannelForAuthRetry();
+ // Start an internal redirect to retry without dictionary after hash mismatch
+ [[nodiscard]] nsresult RedirectToNewChannelForDictionaryRetry();
+
// 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();
private:
+ // Helper function for RedirectToNewChannelForAuthRetry and
+ // RedirectToNewChannelForDictionaryRetry
+ [[nodiscard]] nsresult RedirectToNewChannelInternal(
+ uint32_t redirectFlags,
+ std::function<nsresult(nsHttpChannel*)> setupCallback);
// this section is for main-thread-only object
// all the references need to be proxy released on main thread.
// auth specific data
@@ -922,6 +933,10 @@ class nsHttpChannel final : public HttpBaseChannel,
bool mUsingDictionary{false}; // we added Available-Dictionary
bool mShouldSuspendForDictionary{false};
bool mSuspendedForDictionary{false};
+ // Set on retry channel to prevent offering dictionary again
+ bool mDictionaryDisabledForRetry{false};
+ // Set when redirect channel is ready, waiting for OnStopRequest to open it
+ bool mDictionaryRetryPending{false};
protected:
virtual void DoNotifyListenerCleanup() override;
diff --git a/netwerk/test/unit/test_dictionary_replacement.js b/netwerk/test/unit/test_dictionary_replacement.js
@@ -656,6 +656,148 @@ add_task(async function test_dictionary_hash_mismatch() {
dump("**** Test passed: Hash mismatch properly handled\n");
});
+/**
+ * Test that hash mismatch triggers an automatic retry WITHOUT the
+ * Available-Dictionary header, and the retry succeeds.
+ *
+ * Steps:
+ * 1. Load a resource with Use-As-Dictionary header (creates dictionary entry)
+ * 2. Verify Available-Dictionary is sent for matching resources
+ * 3. Corrupt the hash and clear dictionary data
+ * 4. Request matching resource - should trigger hash mismatch
+ * 5. Verify retry occurs WITHOUT Available-Dictionary header
+ * 6. Verify the request succeeds
+ */
+add_task(async function test_dictionary_hash_mismatch_retry() {
+ dump("**** Clear cache and start fresh\n");
+ let lci = Services.loadContextInfo.custom(false, {
+ partitionKey: `(https,localhost)`,
+ });
+ evict_cache_entries("all", lci);
+
+ let testingInterface = Services.cache2.QueryInterface(Ci.nsICacheTesting);
+ testingInterface.clearDictionaryCacheMemory();
+
+ await syncCache();
+
+ // Register dictionary endpoint
+ await server.registerPathHandler(
+ "/dict/retry-test",
+ function (request, response) {
+ response.writeHead(200, {
+ "Content-Type": "application/octet-stream",
+ "Use-As-Dictionary":
+ 'match="/retry-match/*", id="retry-test-dict", type=raw',
+ "Cache-Control": "max-age=3600",
+ });
+ response.end("DICTIONARY_FOR_RETRY_TEST", "binary");
+ }
+ );
+
+ // Register matching resource endpoint that tracks ALL requests
+ await server.registerPathHandler(
+ "/retry-match/test",
+ function (request, response) {
+ // Initialize array if needed
+ if (!global.retryTestRequests) {
+ global.retryTestRequests = [];
+ }
+ // Record each request's Available-Dictionary header
+ global.retryTestRequests.push(
+ request.headers["available-dictionary"] || null
+ );
+ response.writeHead(200, {
+ "Content-Type": "text/plain",
+ "Cache-Control": "no-cache",
+ });
+ response.end("RETRY_TEST_CONTENT", "binary");
+ }
+ );
+
+ // Reset request tracking
+ await server.execute("global.retryTestRequests = []");
+
+ dump("**** Step 1: Load dictionary resource with Use-As-Dictionary\n");
+
+ let dictUrl = `https://localhost:${server.port()}/dict/retry-test`;
+ let chan = makeChan(dictUrl);
+ let [, data] = await channelOpenPromise(chan);
+
+ Assert.equal(
+ data,
+ "DICTIONARY_FOR_RETRY_TEST",
+ "Dictionary content should match"
+ );
+ await verifyDictionaryStored(dictUrl, true);
+ await syncCache();
+
+ dump("**** Step 2: Verify Available-Dictionary is sent initially\n");
+
+ let matchingUrl = `https://localhost:${server.port()}/retry-match/test`;
+ chan = makeChan(matchingUrl);
+ await channelOpenPromise(chan);
+
+ let requests = await server.execute("global.retryTestRequests");
+ Assert.equal(requests.length, 1, "Should have one request");
+ Assert.notStrictEqual(
+ requests[0],
+ null,
+ "First request should have Available-Dictionary"
+ );
+
+ dump("**** Step 3: Corrupt hash and clear dictionary data\n");
+
+ // Reset request tracking for the retry test
+ await server.execute("global.retryTestRequests = []");
+
+ testingInterface.corruptDictionaryHash(dictUrl);
+ testingInterface.clearDictionaryDataForTesting(dictUrl);
+
+ dump(
+ "**** Step 4: Request matching resource - triggers hash mismatch and retry\n"
+ );
+
+ chan = makeChan(matchingUrl);
+ [, data] = await channelOpenPromise(chan);
+
+ dump("**** Step 5: Verify retry occurred without Available-Dictionary\n");
+
+ requests = await server.execute("global.retryTestRequests");
+ dump(`**** Received ${requests.length} request(s)\n`);
+ for (let i = 0; i < requests.length; i++) {
+ dump(`**** Request ${i}: Available-Dictionary = ${requests[i]}\n`);
+ }
+
+ // We expect 2 requests:
+ // 1. First request WITH Available-Dictionary (triggers hash mismatch)
+ // 2. Retry request WITHOUT Available-Dictionary
+ Assert.equal(
+ requests.length,
+ 2,
+ "Should have two requests (original + retry)"
+ );
+ Assert.notStrictEqual(
+ requests[0],
+ null,
+ "First request should have Available-Dictionary"
+ );
+ Assert.equal(
+ requests[1],
+ null,
+ "Retry request should NOT have Available-Dictionary"
+ );
+
+ dump("**** Step 6: Verify the request succeeded\n");
+
+ Assert.equal(
+ data,
+ "RETRY_TEST_CONTENT",
+ "Request should succeed with correct content"
+ );
+
+ dump("**** Test passed: Hash mismatch retry works correctly\n");
+});
+
// Cleanup
add_task(async function cleanup() {
let lci = Services.loadContextInfo.custom(false, {
