Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions packages/vinext/src/server/app-route-handler-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { ISRCacheEntry } from "./isr-cache.js";
import type { RouteHandlerMiddlewareContext } from "./app-route-handler-response.js";
import {
applyRouteHandlerMiddlewareContext,
assertSupportedAppRouteHandlerResponse,
buildAppRouteCacheValue,
buildRouteHandlerCachedResponse,
} from "./app-route-handler-response.js";
Expand Down Expand Up @@ -102,6 +103,7 @@ export async function readAppRouteHandlerCacheResponse(
});

options.setNavigationContext(null);
assertSupportedAppRouteHandlerResponse(response);

if (dynamicUsedInHandler) {
markKnownDynamicAppRoute(options.routePattern);
Expand Down
2 changes: 2 additions & 0 deletions packages/vinext/src/server/app-route-handler-execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
import {
applyRouteHandlerMiddlewareContext,
applyRouteHandlerRevalidateHeader,
assertSupportedAppRouteHandlerResponse,
buildAppRouteCacheValue,
finalizeRouteHandlerResponse,
markRouteHandlerCacheMiss,
Expand Down Expand Up @@ -109,6 +110,7 @@ export async function executeAppRouteHandler(

try {
const { dynamicUsedInHandler, response } = await runAppRouteHandler(options);
assertSupportedAppRouteHandlerResponse(response);
const handlerSetCacheControl = response.headers.has("cache-control");

if (dynamicUsedInHandler) {
Expand Down
17 changes: 17 additions & 0 deletions packages/vinext/src/server/app-route-handler-response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ type FinalizeRouteHandlerResponseOptions = {
// See .nextjs-ref/packages/next/src/server/lib/cache-control.ts.
const NEVER_CACHE_CONTROL = "private, no-cache, no-store, max-age=0, must-revalidate";

const APP_ROUTE_REWRITE_ERROR =
"NextResponse.rewrite() was used in a app route handler, this is not currently supported. Please remove the invocation to continue.";
const APP_ROUTE_NEXT_ERROR =
"NextResponse.next() was used in a app route handler, this is not supported. See here for more info: https://nextjs.org/docs/messages/next-response-next-in-app-route-handler";

function buildRouteHandlerCacheControl(
cacheState: BuildRouteHandlerCachedResponseOptions["cacheState"],
revalidateSeconds: number,
Expand Down Expand Up @@ -59,6 +64,18 @@ export function applyRouteHandlerMiddlewareContext(
});
}

export function assertSupportedAppRouteHandlerResponse(response: Response): void {
// NextResponse.next() and rewrite() are middleware control-flow signals.
// Once an App Route handler has returned, Next.js rejects those responses.
if (response.headers.has("x-middleware-rewrite")) {
throw new Error(APP_ROUTE_REWRITE_ERROR);
}

if (response.headers.get("x-middleware-next") === "1") {
throw new Error(APP_ROUTE_NEXT_ERROR);
}
}

export function buildRouteHandlerCachedResponse(
cachedValue: CachedRouteValue,
options: BuildRouteHandlerCachedResponseOptions,
Expand Down
61 changes: 61 additions & 0 deletions tests/app-route-handler-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,67 @@ describe("app route handler cache helpers", () => {
expect(isKnownDynamicAppRoute(routePattern)).toBe(true);
});

it("rejects invalid route handler responses during background regeneration", async () => {
const dynamicUsage = createDynamicUsageState();
const scheduledRegens: Array<() => Promise<void>> = [];
let wroteCache = false;

const response = await readAppRouteHandlerCacheResponse({
buildPageCacheTags(pathname, extraTags) {
return [pathname, ...extraTags];
},
cleanPathname: "/api/stale-invalid",
clearRequestContext() {},
consumeDynamicUsage: dynamicUsage.consumeDynamicUsage,
getCollectedFetchTags() {
return [];
},
handlerFn() {
return new Response("should not be cached", {
headers: { "x-middleware-next": "1" },
});
},
isAutoHead: false,
async isrGet() {
return buildISRCacheEntry(buildCachedRouteValue("from-stale"), true);
},
isrRouteKey(pathname) {
return "route:" + pathname;
},
async isrSet() {
wroteCache = true;
},
markDynamicUsage: dynamicUsage.markDynamicUsage,
middlewareContext: { headers: null, status: null },
params: {},
requestUrl: "https://example.com/api/stale-invalid",
revalidateSearchParams: new URLSearchParams(),
revalidateSeconds: 60,
routePattern: "/api/stale-invalid",
async runInRevalidationContext(renderFn) {
await renderFn();
},
scheduleBackgroundRegeneration(_key, renderFn) {
scheduledRegens.push(renderFn);
},
setNavigationContext() {},
});

expect(response?.headers.get("x-vinext-cache")).toBe("STALE");
await expect(response?.text()).resolves.toBe("from-stale");
expect(scheduledRegens).toHaveLength(1);

const scheduledRegenRun = scheduledRegens[0];
if (!scheduledRegenRun) {
throw new Error("Expected scheduled route regeneration");
}

await expect(scheduledRegenRun()).rejects.toThrow(
"NextResponse.next() was used in a app route handler",
);
expect(wroteCache).toBe(false);
});

it("falls through on cache read errors", async () => {
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});

Expand Down
88 changes: 88 additions & 0 deletions tests/app-route-handler-execution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,4 +297,92 @@ describe("app route handler execution helpers", () => {

errorSpy.mockRestore();
});

it("rejects middleware control responses returned from route handlers", async () => {
// The NextResponse.next() case is ported from Next.js:
// test/e2e/app-dir/app-routes/app-custom-routes.test.ts
// https://github.com/vercel/next.js/blob/canary/test/e2e/app-dir/app-routes/app-custom-routes.test.ts
// The NextResponse.rewrite() case mirrors the adjacent App Route module validation.
const cases = [
{
headerName: "x-middleware-next",
headerValue: "1",
message:
"NextResponse.next() was used in a app route handler, this is not supported. See here for more info: https://nextjs.org/docs/messages/next-response-next-in-app-route-handler",
},
{
headerName: "x-middleware-rewrite",
headerValue: "https://example.com/rewritten",
message:
"NextResponse.rewrite() was used in a app route handler, this is not currently supported. Please remove the invocation to continue.",
},
];

const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});

try {
for (const testCase of cases) {
const dynamicUsage = createDynamicUsageState();
const reportedErrors: Error[] = [];
let wroteCache = false;
let didClearRequestContext = false;

const response = await executeAppRouteHandler({
buildPageCacheTags(pathname, extraTags) {
return [pathname, ...extraTags];
},
cleanPathname: "/api/middleware-control",
clearRequestContext() {
didClearRequestContext = true;
},
consumeDynamicUsage: dynamicUsage.consumeDynamicUsage,
executionContext: null,
getAndClearPendingCookies() {
return [];
},
getCollectedFetchTags() {
return [];
},
getDraftModeCookieHeader() {
return null;
},
handler: { dynamic: "auto" },
handlerFn() {
return new Response("should not be sent", {
headers: { [testCase.headerName]: testCase.headerValue },
});
},
isAutoHead: false,
isProduction: true,
isrRouteKey(pathname) {
return "route:" + pathname;
},
async isrSet() {
wroteCache = true;
},
markDynamicUsage: dynamicUsage.markDynamicUsage,
method: "GET",
middlewareContext: { headers: null, status: null },
params: {},
reportRequestError(error) {
reportedErrors.push(error);
},
request: new Request("https://example.com/api/middleware-control"),
revalidateSeconds: 60,
routePattern: "/api/middleware-control",
setHeadersAccessPhase() {
return "render";
},
});

expect(response.status).toBe(500);
await expect(response.text()).resolves.toBe("");
expect(reportedErrors.map((error) => error.message)).toEqual([testCase.message]);
expect(wroteCache).toBe(false);
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.

Good coverage. The test verifies all the important properties: 500 status, empty body, error reported, no cache write, and request context cleanup. The finally block around errorSpy.mockRestore() is also correct.

One minor suggestion: consider adding a test case for the boundary condition where x-middleware-next has a value other than "1" (e.g., "0" or "true") to confirm those are not rejected. This would document the exact semantics and guard against future over-broad matching. Not blocking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added the boundary coverage in 19ef0e2b. assertSupportedAppRouteHandlerResponse() now has a focused unit test showing x-middleware-next: "0" and "true" are allowed, while "1" is rejected.

expect(didClearRequestContext).toBe(true);
}
} finally {
errorSpy.mockRestore();
}
});
});
25 changes: 25 additions & 0 deletions tests/app-route-handler-response.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { CachedRouteValue } from "../packages/vinext/src/shims/cache.js";
import {
applyRouteHandlerMiddlewareContext,
applyRouteHandlerRevalidateHeader,
assertSupportedAppRouteHandlerResponse,
buildAppRouteCacheValue,
buildRouteHandlerCachedResponse,
finalizeRouteHandlerResponse,
Expand Down Expand Up @@ -219,6 +220,30 @@ describe("app route handler response helpers", () => {
expect(response.headers.get("x-vinext-cache")).toBe("MISS");
});

it("only rejects the active x-middleware-next control signal", () => {
expect(() =>
assertSupportedAppRouteHandlerResponse(
new Response(null, {
headers: { "x-middleware-next": "0" },
}),
),
).not.toThrow();
expect(() =>
assertSupportedAppRouteHandlerResponse(
new Response(null, {
headers: { "x-middleware-next": "true" },
}),
),
).not.toThrow();
expect(() =>
assertSupportedAppRouteHandlerResponse(
new Response(null, {
headers: { "x-middleware-next": "1" },
}),
),
).toThrow("NextResponse.next() was used in a app route handler");
});

it("emits a no-store Cache-Control for revalidate = 0 route handlers", () => {
// A handler exporting `revalidate = 0` opts out of caching entirely.
// The Cache-Control must tell browsers and CDNs never to store the
Expand Down
14 changes: 14 additions & 0 deletions tests/app-router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,20 @@ describe("App Router integration", () => {
expect(body).toBe("");
});

it("rejects middleware control responses returned from route handlers", async () => {
// The NextResponse.next() case is ported from Next.js:
// test/e2e/app-dir/app-routes/app-custom-routes.test.ts
// https://github.com/vercel/next.js/blob/canary/test/e2e/app-dir/app-routes/app-custom-routes.test.ts
// The NextResponse.rewrite() case mirrors the adjacent App Route module validation.
const nextRes = await fetch(`${baseUrl}/api/invalid-next-response-next`);
expect(nextRes.status).toBe(500);
expect(await nextRes.text()).toBe("");

const rewriteRes = await fetch(`${baseUrl}/api/invalid-next-response-rewrite`);
expect(rewriteRes.status).toBe(500);
expect(await rewriteRes.text()).toBe("");
});

it("catches redirect() thrown in route handlers", async () => {
const res = await fetch(`${baseUrl}/api/redirect-route`, { redirect: "manual" });
expect(res.status).toBe(307);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { NextResponse } from "next/server";

export function GET() {
return NextResponse.next();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { NextResponse } from "next/server";

export function GET(request: Request) {
return NextResponse.rewrite(new URL("/api/hello", request.url));
}
Loading