commit 10b61670450d05feb62452d3e459b15b49dbc1d5
parent 38a6c8cd9ce26478d2a63f0274a9fc33842edbb7
Author: Randell Jesup <rjesup@mozilla.com>
Date: Sat, 13 Dec 2025 17:35:08 +0000
Bug 2005749: Fix problem with inability to remove dictionaries from origins after a restart r=necko-reviewers,valentin
Differential Revision: https://phabricator.services.mozilla.com/D276346
Diffstat:
5 files changed, 554 insertions(+), 8 deletions(-)
diff --git a/netwerk/cache2/CacheStorageService.cpp b/netwerk/cache2/CacheStorageService.cpp
@@ -2458,4 +2458,14 @@ CacheStorageService::Flush(nsIObserver* aObserver) {
return thread->Dispatch(r, CacheIOThread::WRITE);
}
+NS_IMETHODIMP
+CacheStorageService::ClearDictionaryCacheMemory() {
+ LOG(("CacheStorageService::ClearDictionaryCacheMemory"));
+ RefPtr<DictionaryCache> cache = DictionaryCache::GetInstance();
+ if (cache) {
+ cache->Clear();
+ }
+ return NS_OK;
+}
+
} // namespace mozilla::net
diff --git a/netwerk/cache2/Dictionary.cpp b/netwerk/cache2/Dictionary.cpp
@@ -728,18 +728,16 @@ NS_IMETHODIMP DictionaryOriginReader::OnCacheEntryAvailable(
return NS_OK;
}
+ mOrigin->SetCacheEntry(aCacheEntry);
AUTO_PROFILER_FLOW_MARKER("DictionaryOriginReader::VisitMetaData", NETWORK,
Flow::FromPointer(this));
bool empty = false;
aCacheEntry->GetIsEmpty(&empty);
- if (empty) {
- // New cache entry, set type
- mOrigin->SetCacheEntry(aCacheEntry);
- } else {
+ if (!empty) {
// There's no data in the cache entry, just metadata
nsCOMPtr<nsICacheEntryMetaDataVisitor> metadata(mOrigin);
aCacheEntry->VisitMetaData(metadata);
- }
+ } // else new cache entry
// This list is the only thing keeping us alive
RefPtr<DictionaryOriginReader> safety(this);
@@ -905,9 +903,12 @@ nsresult DictionaryCache::RemoveEntry(nsIURI* aURI, const nsACString& aKey) {
}
void DictionaryCache::Clear() {
- // There may be active Prefetch()es running, note, and active
- // fetches using dictionaries. They will stay alive until the
- // channels using them go away.
+ // There may be active Prefetch()es running, note, and active fetches
+ // using dictionaries. They will stay alive until the channels using
+ // them go away. However, no new requests will see them.
+
+ // This can be used to make the cache return to the state it has at
+ // startup, especially useful for tests.
mDictionaryCache.Clear();
}
diff --git a/netwerk/cache2/nsICacheTesting.idl b/netwerk/cache2/nsICacheTesting.idl
@@ -17,4 +17,9 @@ interface nsICacheTesting : nsISupports
void suspendCacheIOThread(in uint32_t aLevel);
void resumeCacheIOThread();
void flush(in nsIObserver aObserver);
+ /**
+ * Clear the in-memory DictionaryCache but keep disk data intact.
+ * This forces dictionary entries to be reloaded from disk on next access.
+ */
+ void clearDictionaryCacheMemory();
};
diff --git a/netwerk/test/unit/test_dictionary_replacement.js b/netwerk/test/unit/test_dictionary_replacement.js
@@ -0,0 +1,527 @@
+/**
+ * Tests for HTTP Compression Dictionary replacement functionality
+ * - Verify that when a dictionary resource is reloaded without Use-As-Dictionary,
+ * the dictionary metadata is properly removed
+ * - Test that Available-Dictionary header is no longer sent for matching resources
+ * after dictionary is replaced with non-dictionary content
+ *
+ * This tests the fix for the race condition in DictionaryOriginReader::OnCacheEntryAvailable
+ * where mEntry was not set for existing origins loaded from disk.
+ */
+
+"use strict";
+
+// Load cache helpers
+Services.scriptloader.loadSubScript("resource://test/head_cache.js", this);
+
+const { NodeHTTPSServer } = ChromeUtils.importESModule(
+ "resource://testing-common/NodeServer.sys.mjs"
+);
+
+const DICTIONARY_CONTENT = "DICTIONARY_CONTENT_FOR_COMPRESSION_TEST_DATA";
+const REPLACEMENT_CONTENT = "REPLACED_CONTENT_WITHOUT_DICTIONARY_HEADER";
+
+let server = null;
+
+add_setup(async function () {
+ Services.prefs.setBoolPref("network.http.dictionaries.enable", true);
+
+ server = new NodeHTTPSServer();
+ await server.start();
+
+ // Clear any existing cache
+ let lci = Services.loadContextInfo.custom(false, {
+ partitionKey: `(https,localhost)`,
+ });
+ evict_cache_entries("all", lci);
+
+ registerCleanupFunction(async () => {
+ try {
+ await server.stop();
+ } catch (e) {
+ // Ignore server stop errors during cleanup
+ }
+ });
+});
+
+function makeChan(url, bypassCache = false) {
+ let chan = NetUtil.newChannel({
+ uri: url,
+ loadUsingSystemPrincipal: true,
+ contentPolicyType: Ci.nsIContentPolicy.TYPE_DOCUMENT,
+ }).QueryInterface(Ci.nsIHttpChannel);
+
+ if (bypassCache) {
+ chan.loadFlags |= Ci.nsIRequest.LOAD_BYPASS_CACHE;
+ }
+
+ return chan;
+}
+
+function channelOpenPromise(chan) {
+ return new Promise(resolve => {
+ function finish(req, buffer) {
+ resolve([req, buffer]);
+ }
+ chan.asyncOpen(new ChannelListener(finish, null, CL_ALLOW_UNKNOWN_CL));
+ });
+}
+
+function verifyDictionaryStored(url, shouldExist) {
+ return new Promise(resolve => {
+ let lci = Services.loadContextInfo.custom(false, {
+ partitionKey: `(https,localhost)`,
+ });
+ asyncCheckCacheEntryPresence(url, "disk", shouldExist, lci, resolve);
+ });
+}
+
+function syncCache() {
+ return new Promise(resolve => {
+ syncWithCacheIOThread(resolve, true);
+ });
+}
+
+// Clear in-memory DictionaryCache and purge cache entries from memory.
+// This forces dictionary origin entries to be reloaded from disk on next access,
+// triggering DictionaryOriginReader::OnCacheEntryAvailable.
+async function clearDictionaryCacheAndPurgeMemory() {
+ // Clear the DictionaryCache in-memory hashmap
+ let testingInterface = Services.cache2.QueryInterface(Ci.nsICacheTesting);
+ testingInterface.clearDictionaryCacheMemory();
+
+ // Force GC to release references to cache entries. Probably not strictly needed
+ gc();
+}
+
+/**
+ * Test that replacing a dictionary resource with non-dictionary content
+ * properly removes the dictionary metadata.
+ *
+ * Steps:
+ * 1. Load a resource with Use-As-Dictionary header (creates dictionary entry)
+ * 2. Verify Available-Dictionary is sent for matching resources
+ * 3. Force-reload the dictionary resource WITHOUT Use-As-Dictionary
+ * 4. Verify Available-Dictionary is NO LONGER sent for matching resources
+ */
+add_task(async function test_dictionary_replacement_removes_metadata() {
+ // Track Available-Dictionary headers received by server
+ let receivedAvailableDictionary = null;
+
+ // Register dictionary endpoint that returns dictionary content
+ await server.registerPathHandler(
+ "/dict/resource",
+ function (request, response) {
+ response.writeHead(200, {
+ "Content-Type": "application/octet-stream",
+ "Use-As-Dictionary": 'match="/matching/*", id="test-dict", type=raw',
+ "Cache-Control": "max-age=3600",
+ });
+ response.end("DICTIONARY_CONTENT_FOR_COMPRESSION_TEST_DATA", "binary");
+ }
+ );
+
+ // Register matching resource endpoint
+ await server.registerPathHandler(
+ "/matching/test",
+ function (request, response) {
+ // Store the Available-Dictionary header value in global for later retrieval
+ global.lastAvailableDictionary =
+ request.headers["available-dictionary"] || null;
+ response.writeHead(200, {
+ "Content-Type": "text/plain",
+ "Cache-Control": "no-cache",
+ });
+ response.end("CONTENT_THAT_SHOULD_MATCH_DICTIONARY", "binary");
+ }
+ );
+
+ dump("**** Step 1: Load dictionary resource with Use-As-Dictionary\n");
+
+ let dictUrl = `https://localhost:${server.port()}/dict/resource`;
+ let chan = makeChan(dictUrl);
+ let [, data] = await channelOpenPromise(chan);
+
+ Assert.equal(data, DICTIONARY_CONTENT, "Dictionary content should match");
+
+ // Verify dictionary is stored in cache
+ await verifyDictionaryStored(dictUrl, true);
+
+ // Sync to ensure everything is written to disk
+ await syncCache();
+
+ dump("**** Step 1.5: Clear in-memory caches to force reload from disk\n");
+
+ // Clear in-memory DictionaryCache and purge cache entries from memory.
+ // This forces dictionary entries to be reloaded from disk via
+ // DictionaryOriginReader::OnCacheEntryAvailable, which is the code path
+ // with the bug we're testing.
+ await clearDictionaryCacheAndPurgeMemory();
+
+ dump(
+ "**** Step 2: Verify Available-Dictionary is sent for matching resource\n"
+ );
+
+ let matchingUrl = `https://localhost:${server.port()}/matching/test`;
+ chan = makeChan(matchingUrl);
+ await channelOpenPromise(chan);
+
+ // Get the Available-Dictionary value from the server
+ receivedAvailableDictionary = await server.execute(
+ "global.lastAvailableDictionary"
+ );
+
+ Assert.notStrictEqual(
+ receivedAvailableDictionary,
+ null,
+ "Available-Dictionary header should be sent for matching resource"
+ );
+ Assert.ok(
+ receivedAvailableDictionary.includes(":"),
+ "Available-Dictionary should contain a hash"
+ );
+
+ dump(`**** Received Available-Dictionary: ${receivedAvailableDictionary}\n`);
+
+ dump(
+ "**** Step 3: Force-reload dictionary resource WITHOUT Use-As-Dictionary\n"
+ );
+
+ // Re-register the dictionary endpoint to return content WITHOUT Use-As-Dictionary
+ await server.registerPathHandler(
+ "/dict/resource",
+ function (request, response) {
+ response.writeHead(200, {
+ "Content-Type": "application/octet-stream",
+ "Cache-Control": "max-age=3600",
+ // No Use-As-Dictionary header!
+ });
+ response.end("REPLACED_CONTENT_WITHOUT_DICTIONARY_HEADER", "binary");
+ }
+ );
+
+ chan = makeChan(dictUrl, true /* bypassCache */);
+ [, data] = await channelOpenPromise(chan);
+
+ Assert.equal(data, REPLACEMENT_CONTENT, "Replacement content should match");
+
+ // Sync to ensure cache operations complete
+ await syncCache();
+
+ dump("**** Step 4: Verify Available-Dictionary is NOT sent anymore\n");
+
+ // Reset the server's stored value
+ await server.execute("global.lastAvailableDictionary = null");
+
+ // Now request the matching resource again
+ chan = makeChan(matchingUrl);
+ await channelOpenPromise(chan);
+
+ receivedAvailableDictionary = await server.execute(
+ "global.lastAvailableDictionary"
+ );
+
+ Assert.equal(
+ receivedAvailableDictionary,
+ null,
+ "Available-Dictionary header should NOT be sent after dictionary is replaced"
+ );
+
+ dump("**** Test passed: Dictionary metadata was properly removed\n");
+});
+
+/**
+ * Test the same scenario but with gzip-compressed replacement content.
+ * This simulates the real-world case where a server might return
+ * compressed content without Use-As-Dictionary.
+ */
+add_task(async function test_dictionary_replacement_with_compressed_content() {
+ dump("**** Clear cache and start fresh\n");
+ let lci = Services.loadContextInfo.custom(false, {
+ partitionKey: `(https,localhost)`,
+ });
+ evict_cache_entries("all", lci);
+
+ // Also clear in-memory DictionaryCache to start fresh
+ let testingInterface = Services.cache2.QueryInterface(Ci.nsICacheTesting);
+ testingInterface.clearDictionaryCacheMemory();
+
+ await syncCache();
+
+ let receivedAvailableDictionary = null;
+
+ // Register dictionary endpoint
+ await server.registerPathHandler(
+ "/dict/compressed",
+ function (request, response) {
+ response.writeHead(200, {
+ "Content-Type": "application/octet-stream",
+ "Use-As-Dictionary":
+ 'match="/compressed-match/*", id="compressed-dict", type=raw',
+ "Cache-Control": "max-age=3600",
+ });
+ response.end("DICTIONARY_FOR_COMPRESSED_TEST", "binary");
+ }
+ );
+
+ // Register matching resource endpoint
+ await server.registerPathHandler(
+ "/compressed-match/test",
+ function (request, response) {
+ global.lastCompressedAvailDict =
+ request.headers["available-dictionary"] || null;
+ response.writeHead(200, {
+ "Content-Type": "text/plain",
+ "Cache-Control": "no-cache",
+ });
+ response.end("MATCHING_CONTENT", "binary");
+ }
+ );
+
+ dump("**** Step 1: Load dictionary resource with Use-As-Dictionary\n");
+
+ let dictUrl = `https://localhost:${server.port()}/dict/compressed`;
+ let chan = makeChan(dictUrl);
+ let [, data] = await channelOpenPromise(chan);
+
+ Assert.equal(
+ data,
+ "DICTIONARY_FOR_COMPRESSED_TEST",
+ "Dictionary content should match"
+ );
+ await verifyDictionaryStored(dictUrl, true);
+ await syncCache();
+
+ dump("**** Step 1.5: Clear in-memory caches to force reload from disk\n");
+ await clearDictionaryCacheAndPurgeMemory();
+
+ dump("**** Step 2: Verify Available-Dictionary is sent\n");
+
+ let matchingUrl = `https://localhost:${server.port()}/compressed-match/test`;
+ chan = makeChan(matchingUrl);
+ await channelOpenPromise(chan);
+
+ receivedAvailableDictionary = await server.execute(
+ "global.lastCompressedAvailDict"
+ );
+ Assert.notStrictEqual(
+ receivedAvailableDictionary,
+ null,
+ "Available-Dictionary should be sent initially"
+ );
+
+ dump(
+ "**** Step 3: Force-reload with gzip-compressed content (no Use-As-Dictionary)\n"
+ );
+
+ // Re-register to return gzip-compressed content without Use-As-Dictionary
+ await server.registerPathHandler(
+ "/dict/compressed",
+ function (request, response) {
+ // Gzip-compressed version of "GZIP_COMPRESSED_REPLACEMENT"
+ // Using Node.js zlib in the handler
+ const zlib = require("zlib");
+ const compressed = zlib.gzipSync("GZIP_COMPRESSED_REPLACEMENT");
+
+ response.writeHead(200, {
+ "Content-Type": "application/octet-stream",
+ "Content-Encoding": "gzip",
+ "Cache-Control": "max-age=3600",
+ // No Use-As-Dictionary header!
+ });
+ response.end(compressed);
+ }
+ );
+
+ chan = makeChan(dictUrl, true /* bypassCache */);
+ [, data] = await channelOpenPromise(chan);
+
+ // Content should be decompressed by the channel
+ Assert.equal(
+ data,
+ "GZIP_COMPRESSED_REPLACEMENT",
+ "Decompressed replacement content should match"
+ );
+
+ dump("**** Step 4: Verify Available-Dictionary is NOT sent anymore\n");
+
+ await server.execute("global.lastCompressedAvailDict = null");
+ chan = makeChan(matchingUrl);
+ await channelOpenPromise(chan);
+
+ receivedAvailableDictionary = await server.execute(
+ "global.lastCompressedAvailDict"
+ );
+
+ Assert.equal(
+ receivedAvailableDictionary,
+ null,
+ "Available-Dictionary should NOT be sent after dictionary replaced with compressed content"
+ );
+
+ dump(
+ "**** Test passed: Dictionary metadata removed even with compressed replacement\n"
+ );
+});
+
+/**
+ * Test that multiple sequential replacements work correctly.
+ * Dictionary -> Non-dictionary -> Dictionary -> Non-dictionary
+ */
+add_task(async function test_dictionary_multiple_replacements() {
+ dump("**** Clear cache and start fresh\n");
+ let lci = Services.loadContextInfo.custom(false, {
+ partitionKey: `(https,localhost)`,
+ });
+ evict_cache_entries("all", lci);
+
+ // Also clear in-memory DictionaryCache to start fresh
+ let testingInterface = Services.cache2.QueryInterface(Ci.nsICacheTesting);
+ testingInterface.clearDictionaryCacheMemory();
+
+ await syncCache();
+
+ let receivedAvailableDictionary = null;
+
+ // Register matching resource endpoint
+ await server.registerPathHandler(
+ "/multi-match/test",
+ function (request, response) {
+ global.lastMultiAvailDict =
+ request.headers["available-dictionary"] || null;
+ response.writeHead(200, {
+ "Content-Type": "text/plain",
+ "Cache-Control": "no-cache",
+ });
+ response.end("MULTI_MATCHING_CONTENT", "binary");
+ }
+ );
+
+ let dictUrl = `https://localhost:${server.port()}/dict/multi`;
+ let matchingUrl = `https://localhost:${server.port()}/multi-match/test`;
+
+ // === First: Load as dictionary ===
+ dump("**** Load as dictionary (first time)\n");
+ await server.registerPathHandler("/dict/multi", function (request, response) {
+ response.writeHead(200, {
+ "Content-Type": "application/octet-stream",
+ "Use-As-Dictionary":
+ 'match="/multi-match/*", id="multi-dict-1", type=raw',
+ "Cache-Control": "max-age=3600",
+ });
+ response.end("DICTIONARY_CONTENT_V1", "binary");
+ });
+
+ let chan = makeChan(dictUrl);
+ await channelOpenPromise(chan);
+ await syncCache();
+
+ // Clear in-memory caches to force reload from disk
+ await clearDictionaryCacheAndPurgeMemory();
+
+ chan = makeChan(matchingUrl);
+ await channelOpenPromise(chan);
+ receivedAvailableDictionary = await server.execute(
+ "global.lastMultiAvailDict"
+ );
+ Assert.notStrictEqual(
+ receivedAvailableDictionary,
+ null,
+ "Available-Dictionary should be sent (first dictionary)"
+ );
+
+ // === Second: Replace with non-dictionary ===
+ dump("**** Replace with non-dictionary\n");
+ await server.registerPathHandler("/dict/multi", function (request, response) {
+ response.writeHead(200, {
+ "Content-Type": "application/octet-stream",
+ "Cache-Control": "max-age=3600",
+ });
+ response.end("NON_DICTIONARY_CONTENT", "binary");
+ });
+
+ chan = makeChan(dictUrl, true);
+ await channelOpenPromise(chan);
+ await syncCache();
+ await new Promise(resolve => do_timeout(200, resolve));
+
+ await server.execute("global.lastMultiAvailDict = null");
+ chan = makeChan(matchingUrl);
+ await channelOpenPromise(chan);
+ receivedAvailableDictionary = await server.execute(
+ "global.lastMultiAvailDict"
+ );
+ Assert.equal(
+ receivedAvailableDictionary,
+ null,
+ "Available-Dictionary should NOT be sent (after first replacement)"
+ );
+
+ // === Third: Load as dictionary again ===
+ dump("**** Load as dictionary (second time)\n");
+ await server.registerPathHandler("/dict/multi", function (request, response) {
+ response.writeHead(200, {
+ "Content-Type": "application/octet-stream",
+ "Use-As-Dictionary":
+ 'match="/multi-match/*", id="multi-dict-2", type=raw',
+ "Cache-Control": "max-age=3600",
+ });
+ response.end("DICTIONARY_CONTENT_V2", "binary");
+ });
+
+ chan = makeChan(dictUrl, true);
+ await channelOpenPromise(chan);
+ await syncCache();
+
+ // Clear in-memory caches to force reload from disk
+ await clearDictionaryCacheAndPurgeMemory();
+
+ chan = makeChan(matchingUrl);
+ await channelOpenPromise(chan);
+ receivedAvailableDictionary = await server.execute(
+ "global.lastMultiAvailDict"
+ );
+ Assert.notStrictEqual(
+ receivedAvailableDictionary,
+ null,
+ "Available-Dictionary should be sent (second dictionary)"
+ );
+
+ // === Fourth: Replace with non-dictionary again ===
+ dump("**** Replace with non-dictionary again\n");
+ await server.registerPathHandler("/dict/multi", function (request, response) {
+ response.writeHead(200, {
+ "Content-Type": "application/octet-stream",
+ "Cache-Control": "max-age=3600",
+ });
+ response.end("NON_DICTIONARY_CONTENT_V2", "binary");
+ });
+
+ chan = makeChan(dictUrl, true);
+ await channelOpenPromise(chan);
+ await syncCache();
+ await new Promise(resolve => do_timeout(200, resolve));
+
+ await server.execute("global.lastMultiAvailDict = null");
+ chan = makeChan(matchingUrl);
+ await channelOpenPromise(chan);
+ receivedAvailableDictionary = await server.execute(
+ "global.lastMultiAvailDict"
+ );
+ Assert.equal(
+ receivedAvailableDictionary,
+ null,
+ "Available-Dictionary should NOT be sent (after second replacement)"
+ );
+
+ dump("**** Test passed: Multiple replacements work correctly\n");
+});
+
+// Cleanup
+add_task(async function cleanup() {
+ let lci = Services.loadContextInfo.custom(false, {
+ partitionKey: `(https,localhost)`,
+ });
+ evict_cache_entries("all", lci);
+ dump("**** All dictionary replacement tests completed\n");
+});
diff --git a/netwerk/test/unit/xpcshell.toml b/netwerk/test/unit/xpcshell.toml
@@ -598,6 +598,9 @@ prefs = ["content.cors.use_triggering_principal=true"] # See bug 1982916.
["test_dictionary_compression_dcb.js"]
run-sequentially = ["true"]
+["test_dictionary_replacement.js"]
+run-sequentially = ["true"]
+
["test_dictionary_retrieval.js"]
run-sequentially = ["true"]
