Skip to content

fix(unstable_cache): avoid undefined coercion in invocation cache key #91077

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Neiland85 wants to merge 1 commit into
base: canary
Choose a base branch
from
+3 −1
Open

fix(unstable_cache): avoid undefined coercion in invocation cache key #91077

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/next/src/server/web/spec-extension/unstable-cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ export function unstable_cache<T extends Callback>(
// Construct the complete cache key for this function invocation
// @TODO stringify is likely not safe here. We will coerce undefined to null which will make
// the keyspace smaller than the execution space
Comment on lines 132 to 133
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The TODO comment about JSON.stringify coercing undefined to null is now outdated with the new replacer. Please update this comment to reflect the current behavior (and/or the remaining known issues with using JSON.stringify for cache-key material).

Suggested change
// @TODO stringify is likely not safe here. We will coerce undefined to null which will make
// the keyspace smaller than the execution space
// NOTE: JSON.stringify is still not a perfect cache-key serializer. We rely on a replacer
// to encode `undefined` values explicitly, but object property order and non-JSON types
// (e.g. functions, Symbols, BigInts) may still collapse distinct argument shapes.

Copilot uses AI. Check for mistakes.
const invocationKey = `${fixedKey}-${JSON.stringify(args)}`
const invocationKey = `${fixedKey}-${JSON.stringify(args, (_, value) =>
value === undefined ? '__undefined__' : value
)}`
Comment on lines +134 to +136
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

Using a literal string sentinel for undefined reintroduces a cache-key collision: an argument value that is exactly '__undefined__' will serialize identically to an actual undefined. Since this key is used to look up cached results, that can return an incorrect cached value. Consider using an unambiguous encoding (e.g., a tagged wrapper object / tuple with a reserved type marker, plus escaping for strings that might collide) instead of a plain string sentinel.

Copilot uses AI. Check for mistakes.
Comment on lines +134 to +136
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

This changes cache-key generation semantics for unstable_cache invocations with undefined values in args (including nested undefined inside objects). There are existing e2e tests for unstable_cache, but none appear to cover argument-key serialization; adding a regression test that demonstrates undefined vs null (and/or omitted properties) produce distinct cache keys would help prevent future collisions/regressions.

Copilot uses AI. Check for mistakes.
const cacheKey = await incrementalCache.generateCacheKey(invocationKey)
// $urlWithPath,$sortedQueryStringKeys,$hashOfEveryThingElse
const fetchUrl = `unstable_cache ${fetchUrlPrefix} ${cb.name ? ` ${cb.name}` : cacheKey}`
Expand Down
Loading