Skip to content

Commit

Permalink
[next]: ensure unmatched action rewrites are routed to correct handler (
Browse files Browse the repository at this point in the history
#11686)

User defined rewrites are "normalized" so that our internal rewrites are still properly handled. Before normalizing these rewrites, the Next.js builder will attempt to match server action requests to a`.action` variant. Then the user-defined rewrites flow through the afterFiles normalization ([this part](https://github.com/vercel/vercel/blob/fix/unmatched-action-rewrites/packages/next/src/server-build.ts#L254-L279)) so that when we add `.action` in the builder, we don't drop the suffix. 

But this normalization can lead to a malformed `dest`. e.g., if I had rewrite like this:

```js
{
  source: '/greedy-rewrite/static/:path*',
  destination: '/static/:path*',
}
```

The builder would go through this flow on an action request to `/greedy-rewrite/static`:

1. It'll attempt to match it to a `.action` output, so `/greedy-rewrite/static` -> `/greedy-rewrite/static.action`
2. The afterFiles normalization will take place, so the original `dest` of `/static/$1` will become `/static/$1$rscsuff`
3. $1 will be an empty string, because it doesn't match the existing capture group. So now `/greedy-rewrite/static.action` -> `/greedy-rewrite/static/.action`
4. `static/.action` is not a valid output, so it'll 404 and the action will break. 

Existing handling exists for `.rsc` outputs for a similar reason, but only on the index route. I added a similar fix for this in #11688.
  • Loading branch information
ztanner committed Jun 4, 2024
1 parent 5dedc7b commit 7457767
Show file tree
Hide file tree
Showing 7 changed files with 148 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/mean-peas-visit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@vercel/next': patch
---

ensure unmatched action rewrites are routed to correct handler
19 changes: 19 additions & 0 deletions packages/next/src/server-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2089,6 +2089,25 @@ export async function serverBuild({
// with that routing section
...afterFilesRewrites,

// Ensure that after we normalize `afterFilesRewrites`, unmatched actions are routed to the correct handler
// e.g. /foo/.action -> /foo.action. This should only ever match in cases where we're routing to an action handler
// and the rewrite normalization led to something like /foo/$1$rscsuff, and $1 had no match.
// This is meant to have parity with the .rsc handling below.
...(hasActionOutputSupport
? [
{
src: `${path.posix.join('/', entryDirectory, '/\\.action$')}`,
dest: `${path.posix.join('/', entryDirectory, '/index.action')}`,
check: true,
},
{
src: `${path.posix.join('/', entryDirectory, '(.+)/\\.action$')}`,
dest: `${path.posix.join('/', entryDirectory, '$1.action')}`,
check: true,
},
]
: []),

// ensure non-normalized /.rsc from rewrites is handled
...(appPathRoutesManifest
? [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
"use client";

import { useState } from "react";
import { increment } from "../../actions";

export default function Home() {
const [count, setCount] = useState(0);

return (
<div>
{count}
<button
onClick={async () => {
const actionResult = await increment(count);
// @ts-ignore
setCount(actionResult);
console.log(actionResult);
}}
>
Trigger
</button>
Static
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
"use client";

import { useState } from "react";
import { increment } from "./actions";

export default function Home() {
const [count, setCount] = useState(0);

return (
<div>
{count}
<button
onClick={async () => {
const actionResult = await increment(count);
// @ts-ignore
setCount(actionResult);
console.log(actionResult);
}}
>
Trigger
</button>
Static
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
"use client";

import { useState } from "react";
import { increment } from "../actions";

export default function Home() {
const [count, setCount] = useState(0);

return (
<div>
{count}
<button
onClick={async () => {
const actionResult = await increment(count);
// @ts-ignore
setCount(actionResult);
console.log(actionResult);
}}
>
Trigger
</button>
Static
</div>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,43 @@ describe(`${__dirname.split(path.sep).pop()}`, () => {
expect(res.headers.get('x-edge-runtime')).toBe('1');
}
});

it('should work when a rewrite greedy matches an action rewrite', async () => {
const targetPath = `${basePath}/static`;
const canonicalPath = `/greedy-rewrite/${basePath}/static`;
const actionId = findActionId(targetPath, runtime);

const res = await fetch(
`${ctx.deploymentUrl}${canonicalPath}`,
generateFormDataPayload(actionId)
);

expect(res.status).toEqual(200);
expect(res.headers.get('x-matched-path')).toBe(targetPath + '.action');
expect(res.headers.get('content-type')).toBe('text/x-component');
if (runtime === 'node') {
expect(res.headers.get('x-vercel-cache')).toBe('MISS');
} else {
expect(res.headers.get('x-edge-runtime')).toBe('1');
}
});
});

describe('rewrite to index', () => {
it('should work when user has a rewrite to the index route', async () => {
const canonicalPath = '/rewritten-to-index';
const actionId = findActionId('', 'node');

const res = await fetch(
`${ctx.deploymentUrl}${canonicalPath}`,
generateFormDataPayload(actionId)
);

expect(res.status).toEqual(200);
expect(res.headers.get('x-matched-path')).toBe('/index.action');
expect(res.headers.get('content-type')).toBe('text/x-component');
expect(res.headers.get('x-vercel-cache')).toBe('MISS');
});
});

describe('pages', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@ module.exports = {
source: '/rewrite/edge/rsc/static',
destination: '/edge/rsc/static',
},
{
source: '/greedy-rewrite/static/:path*',
destination: '/static/:path*',
},
{
source: '/greedy-rewrite/edge/static/:path*',
destination: '/edge/static/:path*',
},
{
source: '/rewritten-to-index',
destination: '/?fromRewrite=1',
},
];
},
};

0 comments on commit 7457767

Please sign in to comment.