Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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: 4 additions & 0 deletions docs/1.docs/7.cache.md
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,10 @@ The `defineCachedHandler` and `defineCachedFunction` functions accept the follow
::field{name="varies" type="string[]"}
An array of request headers to be considered for the cache, [learn more](https://github.com/nitrojs/nitro/issues/1031). If utilizing in a multi-tenant environment, you may want to pass `['host', 'x-forwarded-host']` to ensure these headers are not discarded and that the cache is unique per tenant.
::
::field{name="allowQuery" type="string[]"}
List of query parameter names to include in the cache key. If `undefined`, all query parameters are included. If an empty array `[]`, all query parameters are ignored (only the pathname is used for caching). Only applicable to cached event handlers. :br
Defaults to `undefined`.
::
::

## Cache keys and invalidation
Expand Down
15 changes: 14 additions & 1 deletion src/runtime/internal/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,20 @@ export function defineCachedHandler(
return escapeKey(customKey);
}
// Auto-generated key
const _path = event.url.pathname + event.url.search;
let _path: string;
if (opts.allowQuery) {
const params = new URLSearchParams();
for (const key of opts.allowQuery) {
const value = event.url.searchParams.get(key);
if (value !== null) {
params.set(key, value);
}
}
const search = params.size > 0 ? `?${params.toString()}` : "";
Comment on lines +205 to +213
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Preserve repeated allowed query values in the cache key.

Line 208 only reads the first value with searchParams.get(), so ?tag=a&tag=b and ?tag=a collapse to the same cache entry when tag is allowed. That can serve the wrong cached response for multi-value filters.

🐛 Proposed fix
         const params = new URLSearchParams();
         for (const key of opts.allowQuery) {
-          const value = event.url.searchParams.get(key);
-          if (value !== null) {
-            params.set(key, value);
+          for (const value of event.url.searchParams.getAll(key)) {
+            params.append(key, value);
           }
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (opts.allowQuery) {
const params = new URLSearchParams();
for (const key of opts.allowQuery) {
const value = event.url.searchParams.get(key);
if (value !== null) {
params.set(key, value);
}
}
const search = params.size > 0 ? `?${params.toString()}` : "";
if (opts.allowQuery) {
const params = new URLSearchParams();
for (const key of opts.allowQuery) {
for (const value of event.url.searchParams.getAll(key)) {
params.append(key, value);
}
}
const search = params.size > 0 ? `?${params.toString()}` : "";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/runtime/internal/cache.ts` around lines 205 - 213, The cache key builder
currently collapses repeated query values because it uses
event.url.searchParams.get() and params.set(), so e.g. ?tag=a&tag=b and ?tag=a
become identical; change the logic in the block that references opts.allowQuery
and event.url.searchParams to retrieve all values for each allowed key (use
getAll or iterate searchParams entries for that key) and append each value to
the local URLSearchParams (use params.append) so duplicate values and their
order are preserved when computing the search string assigned to the search
variable.

_path = event.url.pathname + search;
} else {
_path = event.url.pathname + event.url.search;
}
let _pathname: string;
try {
_pathname = escapeKey(decodeURI(parseURL(_path).pathname)).slice(0, 16) || "index";
Expand Down
7 changes: 7 additions & 0 deletions src/types/runtime/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,11 @@ export interface CachedEventHandlerOptions extends Omit<
> {
headersOnly?: boolean;
varies?: string[] | readonly string[];
/**
* List of query string parameter names that will be considered for caching.
* - If undefined, all query parameters are included in the cache key.
* - If an empty array `[]`, all query parameters are ignored (only pathname is used for caching).
* - If a list of parameter names, only those parameters are included in the cache key.
*/
allowQuery?: string[];
}
1 change: 1 addition & 0 deletions test/fixture/nitro.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export default defineConfig({
},
"/rules/dynamic": { cache: false, isr: false },
"/rules/redirect": { redirect: "/base" },
"/rules/allow-query/**": { cache: { swr: true, maxAge: 60, allowQuery: ["q"] } },
"/rules/isr/**": { isr: { allowQuery: ["q"] } },
"/rules/isr-ttl/**": { isr: 60 },
"/rules/swr/**": { swr: true },
Expand Down
10 changes: 10 additions & 0 deletions test/fixture/server/routes/api/cached-allow-query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { defineCachedHandler } from "nitro/cache";

export default defineCachedHandler(
(event) => {
return {
timestamp: Date.now(),
};
},
{ swr: true, maxAge: 60, allowQuery: ["q"] }
);
21 changes: 21 additions & 0 deletions test/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,27 @@ export function testNitro(
}
}
);

it.skipIf(ctx.isIsolated || (isWindows && ctx.preset === "nitro-dev"))(
"allowQuery should ignore unlisted query params in cache key",
async () => {
const { data: first } = await callHandler({
url: "/api/cached-allow-query?q=search&utm_source=email",
});

// Same q param, different unlisted param should hit cache
const { data: second } = await callHandler({
url: "/api/cached-allow-query?q=search&utm_source=twitter",
});
expect(second.timestamp).toBe(first.timestamp);

// Different q param should get a different cache entry
const { data: third } = await callHandler({
url: "/api/cached-allow-query?q=other&utm_source=email",
});
expect(third.timestamp).not.toBe(first.timestamp);
}
);
});

describe("scanned files", () => {
Expand Down
Loading