mirror of
https://github.com/LukeHagar/vercel.git
synced 2025-12-09 12:57:46 +00:00
[dev] fix middleware rewrites to relative paths that do not exist (#8535)
When using a `rewrite` in middleware to a relative path that does not exist, the logic gets confused and falls back to the original path. This was caused by two changes that, in combination, caused this behavior. 1. `prevUrl` was created to keep track of route results that point to other routes, but falls back to `req.url` (`prevUrl`'s initial value): [#4033](40e4b69267 (diff-142c93a61d03a1718eb765cd25e5f18d07a95358bb27ce5d5d4da314ee2fa286R1283-R1284)) 2. `prevUrl` was reassigned when handling middleware: [#7973](ee1211416f (diff-00ef6e7b63ed4cae298afc2a8c84f49247855a2506270f748e4d3e41da97ad99R1538)) Because `prevUrl` was reset back to `req.url` after the first `phase` (null) was run, the updated `prevUrl` value from middleware was lost. This only matters if the second `phase` ("filesystem") was going to be hit. Further confusing matters, this was partially fixed by https://github.com/vercel/vercel/pull/8457 because it either returned a proxy pass (which doesn't have this problem) or assigned `req.url` to include the `rewritePath`, which meant that later when `prevUrl` would default to `req.url`, it still had come from `rewritePath`. So, this is fixed for the absolute URL case, but not the relative path case. Given all that, I think the fix we need is to keep `prevUrl` at its current value when it's not being updated based on the `routeResult`. --- So, I made that change here. I added a test that exercises this specific behavior.
This commit is contained in:
@@ -1579,7 +1579,9 @@ export default class DevServer {
|
|||||||
const devServerParsed = new URL(this.address);
|
const devServerParsed = new URL(this.address);
|
||||||
if (devServerParsed.origin === rewriteUrlParsed.origin) {
|
if (devServerParsed.origin === rewriteUrlParsed.origin) {
|
||||||
// remove origin, leaving the path
|
// remove origin, leaving the path
|
||||||
req.url = rewritePath.slice(rewriteUrlParsed.origin.length);
|
req.url =
|
||||||
|
rewritePath.slice(rewriteUrlParsed.origin.length) || '/';
|
||||||
|
prevUrl = req.url;
|
||||||
} else {
|
} else {
|
||||||
// Proxy to absolute URL with different origin
|
// Proxy to absolute URL with different origin
|
||||||
debug(`ProxyPass: ${rewritePath}`);
|
debug(`ProxyPass: ${rewritePath}`);
|
||||||
@@ -1642,12 +1644,16 @@ export default class DevServer {
|
|||||||
missRoutes,
|
missRoutes,
|
||||||
phase
|
phase
|
||||||
);
|
);
|
||||||
prevUrl =
|
|
||||||
routeResult.continue && routeResult.dest
|
if (routeResult.continue) {
|
||||||
? getReqUrl(routeResult)
|
if (routeResult.dest) {
|
||||||
: req.url;
|
prevUrl = getReqUrl(routeResult);
|
||||||
prevHeaders =
|
}
|
||||||
routeResult.continue && routeResult.headers ? routeResult.headers : {};
|
|
||||||
|
if (routeResult.headers) {
|
||||||
|
prevHeaders = routeResult.headers;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (routeResult.isDestUrl) {
|
if (routeResult.isDestUrl) {
|
||||||
// Mix the `routes` result dest query params into the req path
|
// Mix the `routes` result dest query params into the req path
|
||||||
|
|||||||
@@ -0,0 +1,7 @@
|
|||||||
|
export const config = {
|
||||||
|
runtime: 'experimental-edge',
|
||||||
|
};
|
||||||
|
|
||||||
|
export default async function edge(request: Request, event: Event) {
|
||||||
|
return new Response('heyo');
|
||||||
|
}
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
<h1>Hello from Index</h1>
|
||||||
@@ -0,0 +1,21 @@
|
|||||||
|
export const config = {
|
||||||
|
runtime: 'experimental-edge',
|
||||||
|
};
|
||||||
|
|
||||||
|
export default async function edge(request: Request, event: Event) {
|
||||||
|
if (request.url.indexOf('/index.html') > -1) {
|
||||||
|
return new Response(null, {
|
||||||
|
headers: {
|
||||||
|
'x-middleware-rewrite': '/does-not-exist.html',
|
||||||
|
},
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
if (request.url.indexOf('/api/edge') > -1) {
|
||||||
|
return new Response(null, {
|
||||||
|
headers: {
|
||||||
|
'x-middleware-rewrite': '/api/does-not-exist',
|
||||||
|
},
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -510,6 +510,14 @@ test(
|
|||||||
})
|
})
|
||||||
);
|
);
|
||||||
|
|
||||||
|
test(
|
||||||
|
'[vercel dev] Middleware that rewrites to 404s',
|
||||||
|
testFixtureStdio('middleware-rewrite-404', async (testPath: any) => {
|
||||||
|
await testPath(404, '/api/edge', /NOT_FOUND/);
|
||||||
|
await testPath(404, '/index.html', /NOT_FOUND/);
|
||||||
|
})
|
||||||
|
);
|
||||||
|
|
||||||
test(
|
test(
|
||||||
'[vercel dev] Middleware that redirects',
|
'[vercel dev] Middleware that redirects',
|
||||||
testFixtureStdio('middleware-redirect', async (testPath: any) => {
|
testFixtureStdio('middleware-redirect', async (testPath: any) => {
|
||||||
|
|||||||
Reference in New Issue
Block a user