Skip to content

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

Merged
clydin merged 1 commit into
angular:mainfrom
clydin:refactor/cache-namespace-encoding
Sep 10, 2026
Merged

refactor(@angular/build): prevent key collisions in cache namespaces#34061
clydin merged 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
@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 10, 2026
@clydin
clydin merged commit 44129f6 into angular:main Sep 10, 2026
41 checks passed
@clydin

clydin commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

This PR was merged into the repository. The changes were merged into the following branches:

@clydin
clydin deleted the refactor/cache-namespace-encoding branch September 10, 2026 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker 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.

2 participants