refactor(@angular/build): prevent key collisions in cache namespaces

Previously, Cache formatted keys by joining the namespace and key with a single colon delimiter (${namespace}:${key}). This allowed potential key collisions if a namespace contained colons (such as 'a' with key 'b:c' versus 'a:b' with key 'c').

While this was not an issue in existing usages because all current namespaces are fixed, colon-free identifiers and keys are hash digests, it presented an architectural risk as caching usages expand.

Namespacing is now encapsulated in a dedicated NamespacedCacheStore wrapper that frames keys using length-prefix encoding (<length>:<namespace>:<key>). The length prefix eliminates delimiter ambiguity regardless of what characters appear in the namespace or key. Additionally, Cache has been decoupled from namespace management, simplifying MemoryCache and internal request tracking.

Author: clydin

packages/angular/build/src/tools/esbuild/cache.ts

@@ -51,6 +51,39 @@ export interface PersistentCacheStore<V = any> extends CacheStore<V> {
   close(): void | Promise<void>;
 }
 
+/**
+ * A backing data store wrapper that namespaces all keys using length-prefix framing.
+ * Prevents key collisions between namespaces regardless of characters (such as colons)
+ * in the namespace or key.
+ */
+export class NamespacedCacheStore<V> implements CacheStore<V> {
+  readonly #prefix: string;
+
+  constructor(
+    private readonly store: CacheStore<V>,
+    readonly namespace: string,
+  ) {
+    this.#prefix = `${namespace.length}:${namespace}:`;
+  }
+
+  get(key: string): V | undefined | Promise<V | undefined> {
+    return this.store.get(this.#prefix + key);
+  }
+
+  has(key: string): boolean | Promise<boolean> {
+    return this.store.has(this.#prefix + key);
+  }
+
+  set(key: string, value: V): this | Promise<this> {
+    const result = this.store.set(this.#prefix + key, value);
+    if (result instanceof Promise) {
+      return result.then(() => this);
+    }
+
+    return this;
+  }
+}
+
 /**
  * A cache object that allows accessing and storing key/value pairs in
  * an underlying CacheStore. This class is the primary method for consumers
@@ -64,10 +97,7 @@ export class Cache<V, S extends CacheStore<V> = CacheStore<V>> {
   // Count the number of active, pending getOrCreate operations per key to avoid memory leaks.
   readonly #pendingGets = new Map<string, number>();
 
-  constructor(
-    protected readonly store: S,
-    readonly namespace?: string,
-  ) {}
+  constructor(protected readonly store: S) {}
 
   #incrementWrite(key: string) {
     // Only track write counts if there is a pending getOrCreate operation active for the key.
@@ -77,19 +107,6 @@ export class Cache<V, S extends CacheStore<V> = CacheStore<V>> {
     }
   }
 
-  /**
-   * Prefixes a key with the cache namespace if present.
-   * @param key A key string to prefix.
-   * @returns A prefixed key if a namespace is present. Otherwise the provided key.
-   */
-  protected withNamespace(key: string): string {
-    if (this.namespace) {
-      return `${this.namespace}:${key}`;
-    }
-
-    return key;
-  }
-
   /**
    * Gets the value associated with a provided key if available.
    * Otherwise, creates a value using the factory creator function, puts the value
@@ -99,27 +116,25 @@ export class Cache<V, S extends CacheStore<V> = CacheStore<V>> {
    * @returns A value associated with the provided key.
    */
   async getOrCreate(key: string, creator: () => V | Promise<V>): Promise<V> {
-    const namespacedKey = this.withNamespace(key);
-
     // 1. If another call is already running the creator for this key, share its promise.
-    let activeRequest = this.#requests.get(namespacedKey);
+    let activeRequest = this.#requests.get(key);
     if (activeRequest !== undefined) {
       return activeRequest;
     }
 
     // Increment pending gets count to enable write-tracking for this key.
-    const currentPending = this.#pendingGets.get(namespacedKey) || 0;
-    this.#pendingGets.set(namespacedKey, currentPending + 1);
+    const currentPending = this.#pendingGets.get(key) || 0;
+    this.#pendingGets.set(key, currentPending + 1);
 
     try {
-      const startWriteCount = this.#writeCounts.get(namespacedKey) || 0;
+      const startWriteCount = this.#writeCounts.get(key) || 0;
 
       // 2. Query the backing store. Since store.get can be async, we yield to the event loop.
-      const value = await this.store.get(namespacedKey);
+      const value = await this.store.get(key);
 
       // If a write (e.g. put) occurred during the store.get await gap, we must abort
       // the current execution and restart to ensure we return the newly written value.
-      if ((this.#writeCounts.get(namespacedKey) || 0) !== startWriteCount) {
+      if ((this.#writeCounts.get(key) || 0) !== startWriteCount) {
         return this.getOrCreate(key, creator);
       }
 
@@ -129,7 +144,7 @@ export class Cache<V, S extends CacheStore<V> = CacheStore<V>> {
 
       // 3. Recheck active request after the await gap in case another concurrent call
       // initiated a creator during the store.get wait.
-      activeRequest = this.#requests.get(namespacedKey);
+      activeRequest = this.#requests.get(key);
       if (activeRequest !== undefined) {
         return activeRequest;
       }
@@ -139,34 +154,34 @@ export class Cache<V, S extends CacheStore<V> = CacheStore<V>> {
         async (newValue) => {
           // Ensure this request is still the active one before writing back to the store
           // (prevents overwriting newer data if put() was called before resolution).
-          if (this.#requests.get(namespacedKey) === activeRequest) {
-            this.#incrementWrite(namespacedKey);
-            await this.store.set(namespacedKey, newValue);
-            this.#requests.delete(namespacedKey);
+          if (this.#requests.get(key) === activeRequest) {
+            this.#incrementWrite(key);
+            await this.store.set(key, newValue);
+            this.#requests.delete(key);
           }
 
           return newValue;
         },
         (error) => {
           // Clean up the active request if the creator fails.
-          if (this.#requests.get(namespacedKey) === activeRequest) {
-            this.#requests.delete(namespacedKey);
+          if (this.#requests.get(key) === activeRequest) {
+            this.#requests.delete(key);
           }
           throw error;
         },
       );
 
-      this.#requests.set(namespacedKey, activeRequest);
+      this.#requests.set(key, activeRequest);
 
       return activeRequest;
     } finally {
       // Clean up write counts and pending gets once all concurrent gets for this key finish.
-      const current = this.#pendingGets.get(namespacedKey) || 0;
+      const current = this.#pendingGets.get(key) || 0;
       if (current <= 1) {
-        this.#pendingGets.delete(namespacedKey);
-        this.#writeCounts.delete(namespacedKey);
+        this.#pendingGets.delete(key);
+        this.#writeCounts.delete(key);
       } else {
-        this.#pendingGets.set(namespacedKey, current - 1);
+        this.#pendingGets.set(key, current - 1);
       }
     }
   }
@@ -177,7 +192,7 @@ export class Cache<V, S extends CacheStore<V> = CacheStore<V>> {
    * @returns A value associated with the provided key if present. Otherwise, `undefined`.
    */
   async get(key: string): Promise<V | undefined> {
-    const value = await this.store.get(this.withNamespace(key));
+    const value = await this.store.get(key);
 
     return value;
   }
@@ -189,19 +204,18 @@ export class Cache<V, S extends CacheStore<V> = CacheStore<V>> {
    * @param value A value to put in the cache.
    */
   async put(key: string, value: V): Promise<void> {
-    const namespacedKey = this.withNamespace(key);
-    this.#requests.delete(namespacedKey);
-    this.#incrementWrite(namespacedKey);
-    await this.store.set(namespacedKey, value);
+    this.#requests.delete(key);
+    this.#incrementWrite(key);
+    await this.store.set(key, value);
   }
 
   /**
-   * Clears internal state for a specific namespaced key (requests, write counts, and pending gets).
+   * Clears internal state for a specific key (requests, write counts, and pending gets).
    */
-  protected deleteInternal(namespacedKey: string): void {
-    this.#requests.delete(namespacedKey);
-    this.#writeCounts.delete(namespacedKey);
-    this.#pendingGets.delete(namespacedKey);
+  protected deleteInternal(key: string): void {
+    this.#requests.delete(key);
+    this.#writeCounts.delete(key);
+    this.#pendingGets.delete(key);
   }
 
   /**
@@ -228,10 +242,9 @@ export class MemoryCache<V> extends Cache<V, Map<string, V>> {
    * @returns True if an element in the Map existed and has been removed, or false if the element does not exist.
    */
   delete(key: string): boolean {
-    const namespacedKey = this.withNamespace(key);
-    this.deleteInternal(namespacedKey);
+    this.deleteInternal(key);
 
-    return this.store.delete(namespacedKey);
+    return this.store.delete(key);
   }
 
   /**

packages/angular/build/src/tools/esbuild/cache_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import { MemoryCache } from './cache';
+import { Cache, CacheStore, MemoryCache, NamespacedCacheStore } from './cache';
 
 describe('MemoryCache', () => {
   let cache: MemoryCache<string>;
@@ -158,4 +158,110 @@ describe('MemoryCache', () => {
   it('should return false when deleting a non-existent key', () => {
     expect(cache.delete('non-existent')).toBeFalse();
   });
+
+  it('should return unencoded keys in entries() and allow deletion', async () => {
+    await cache.put('component/style.scss', 'content');
+
+    const entries = Array.from(cache.entries());
+    expect(entries).toEqual([['component/style.scss', 'content']]);
+
+    expect(cache.delete(entries[0][0])).toBeTrue();
+    expect(await cache.get('component/style.scss')).toBeUndefined();
+  });
+});
+
+describe('NamespacedCacheStore', () => {
+  class TestStore implements CacheStore<string> {
+    readonly map = new Map<string, string>();
+
+    get(key: string): string | undefined {
+      return this.map.get(key);
+    }
+
+    has(key: string): boolean {
+      return this.map.has(key);
+    }
+
+    set(key: string, value: string): this {
+      this.map.set(key, value);
+
+      return this;
+    }
+  }
+
+  let store: TestStore;
+
+  beforeEach(() => {
+    store = new TestStore();
+  });
+
+  it('should encode namespaced keys with <length>:<namespace>:<key>', async () => {
+    const namespacedStore = new NamespacedCacheStore(store, 'test-ns');
+    const cache = new Cache<string>(namespacedStore);
+    await cache.put('my-key', 'my-val');
+
+    expect(store.map.has('7:test-ns:my-key')).toBeTrue();
+    expect(await cache.get('my-key')).toBe('my-val');
+  });
+
+  it('should prevent collisions between namespaces containing colons', async () => {
+    const cacheA = new Cache<string>(new NamespacedCacheStore(store, 'a'));
+    const cacheB = new Cache<string>(new NamespacedCacheStore(store, 'a:b'));
+
+    await cacheA.put('b:c', 'val-a');
+    await cacheB.put('c', 'val-b');
+
+    expect(await cacheA.get('b:c')).toBe('val-a');
+    expect(await cacheB.get('c')).toBe('val-b');
+    expect(store.map.get('1:a:b:c')).toBe('val-a');
+    expect(store.map.get('3:a:b:c')).toBe('val-b');
+  });
+
+  it('should encode empty string namespace as 0::<key>', async () => {
+    const namespacedStore = new NamespacedCacheStore(store, '');
+    const cache = new Cache<string>(namespacedStore);
+    await cache.put('key', 'val');
+
+    expect(store.map.has('0::key')).toBeTrue();
+    expect(await cache.get('key')).toBe('val');
+  });
+
+  it('should forward get, has, and set calls with the namespaced prefix', async () => {
+    const namespacedStore = new NamespacedCacheStore(store, 'custom');
+    await namespacedStore.set('hello', 'world');
+
+    expect(store.map.has('6:custom:hello')).toBeTrue();
+    expect(await namespacedStore.has('hello')).toBeTrue();
+    expect(await namespacedStore.get('hello')).toBe('world');
+  });
+
+  it('should return this when the underlying store set is asynchronous', async () => {
+    class AsyncStore implements CacheStore<string> {
+      readonly map = new Map<string, string>();
+
+      get(key: string): Promise<string | undefined> {
+        return Promise.resolve(this.map.get(key));
+      }
+
+      has(key: string): Promise<boolean> {
+        return Promise.resolve(this.map.has(key));
+      }
+
+      async set(key: string, value: string): Promise<this> {
+        this.map.set(key, value);
+
+        return this;
+      }
+    }
+
+    const asyncStore = new AsyncStore();
+    const namespacedStore = new NamespacedCacheStore(asyncStore, 'async-ns');
+    const setPromise = namespacedStore.set('foo', 'bar');
+
+    expect(setPromise instanceof Promise).toBeTrue();
+    expect(await setPromise).toBe(namespacedStore);
+    expect(asyncStore.map.get('8:async-ns:foo')).toBe('bar');
+    expect(await namespacedStore.get('foo')).toBe('bar');
+    expect(await namespacedStore.has('foo')).toBeTrue();
+  });
 });

packages/angular/build/src/tools/esbuild/lmdb-cache-store.ts

@@ -7,7 +7,7 @@
  */
 
 import { RootDatabase, open } from 'lmdb';
-import { Cache, PersistentCacheStore } from './cache';
+import { Cache, NamespacedCacheStore, PersistentCacheStore } from './cache';
 
 export class LmdbCacheStore implements PersistentCacheStore<unknown> {
   readonly #cacheFileUrl;
@@ -46,7 +46,7 @@ export class LmdbCacheStore implements PersistentCacheStore<unknown> {
   }
 
   createCache<V = unknown>(namespace: string): Cache<V> {
-    return new Cache(this, namespace);
+    return new Cache<V>(new NamespacedCacheStore<V>(this, namespace));
   }
 
   async close() {

packages/angular/build/src/tools/esbuild/sqlite-cache-store.ts

@@ -12,7 +12,7 @@ import { DatabaseSync, StatementSync } from 'node:sqlite';
 import { promisify } from 'node:util';
 import { deserialize, serialize } from 'node:v8';
 import { deflateRaw, inflateRawSync } from 'node:zlib';
-import { Cache, PersistentCacheStore } from './cache';
+import { Cache, NamespacedCacheStore, PersistentCacheStore } from './cache';
 
 const deflateRawAsync = promisify(deflateRaw);
 
@@ -315,7 +315,7 @@ export class SqliteCacheStore implements PersistentCacheStore<unknown> {
   }
 
   createCache<V = unknown>(namespace: string): Cache<V> {
-    return new Cache(this, namespace);
+    return new Cache<V>(new NamespacedCacheStore<V>(this, namespace));
   }
 
   close(): void {