Skip to content

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

Open
clydin wants to merge 1 commit into
angular:mainfrom
clydin:refactor/cache-namespace-encoding
Open

refactor(@angular/build): prevent key collisions in cache namespaces#34061
clydin wants to merge 1 commit into
angular:mainfrom
clydin:refactor/cache-namespace-encoding

Conversation

@clydin

@clydin clydin commented Sep 9, 2026

Copy link
Copy Markdown
Member

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.

@clydin clydin added the target: minor This PR is targeted for the next minor release label Sep 9, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new NamespacedCacheStore class to handle key namespacing using length-prefix framing, which prevents key collisions. It refactors the Cache class to delegate namespacing to this wrapper and updates both LmdbCacheStore and SqliteCacheStore to use it. The feedback suggests decoupling NamespacedCacheStore from the specific value type of the underlying store by using CacheStore<any>, and explicitly specifying the type argument <V> when instantiating Cache and NamespacedCacheStore in the cache stores to ensure strict type safety.

Comment thread packages/angular/build/src/tools/esbuild/cache.ts
Comment thread packages/angular/build/src/tools/esbuild/lmdb-cache-store.ts Outdated
Comment thread packages/angular/build/src/tools/esbuild/sqlite-cache-store.ts Outdated
@clydin
clydin force-pushed the refactor/cache-namespace-encoding branch from 7f90071 to d6c0591 Compare September 9, 2026 21:50
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.
@clydin
clydin force-pushed the refactor/cache-namespace-encoding branch from d6c0591 to d8150f1 Compare September 9, 2026 22:32
@clydin clydin added the action: review The PR is still awaiting reviews from at least one requested reviewer label Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: review The PR is still awaiting reviews from at least one requested reviewer area: @angular/build target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant