Use status 308 for non-GET redirects (#7186)

This commit is contained in:
Matthew Phillips 2023-05-24 08:39:23 -04:00 committed by GitHub
parent 2904ceddf6
commit c2f889bec6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 54 additions and 10 deletions

View file

@ -28,6 +28,7 @@ import {
createStylesheetElementSet,
} from '../render/ssr-element.js';
import { matchRoute } from '../routing/match.js';
import { RedirectComponentInstance } from '../redirects/index.js';
export { deserializeManifest } from './common.js';
const clientLocalsSymbol = Symbol.for('astro.locals');
@ -139,20 +140,20 @@ export class App {
defaultStatus = 404;
}
let mod = await this.#manifest.pageMap.get(routeData.component)!();
let mod = await this.#getModuleForRoute(routeData);
if (routeData.type === 'page') {
if (routeData.type === 'page' || routeData.type === 'redirect') {
let response = await this.#renderPage(request, routeData, mod, defaultStatus);
// If there was a known error code, try sending the according page (e.g. 404.astro / 500.astro).
if (response.status === 500 || response.status === 404) {
const errorPageData = matchRoute('/' + response.status, this.#manifestData);
if (errorPageData && errorPageData.route !== routeData.route) {
mod = await this.#manifest.pageMap.get(errorPageData.component)!();
const errorRouteData = matchRoute('/' + response.status, this.#manifestData);
if (errorRouteData && errorRouteData.route !== routeData.route) {
mod = await this.#getModuleForRoute(errorRouteData);
try {
let errorResponse = await this.#renderPage(
request,
errorPageData,
errorRouteData,
mod,
response.status
);
@ -172,6 +173,14 @@ export class App {
return getSetCookiesFromResponse(response);
}
async #getModuleForRoute(route: RouteData): Promise<ComponentInstance> {
if(route.type === 'redirect') {
return RedirectComponentInstance;
} else {
return await this.#manifest.pageMap.get(route.component)!();
}
}
async #renderPage(
request: Request,
routeData: RouteData,

View file

@ -35,7 +35,7 @@ import { debug, info } from '../logger/core.js';
import { callMiddleware } from '../middleware/callMiddleware.js';
import { createEnvironment, createRenderContext, renderPage } from '../render/index.js';
import { callGetStaticPaths } from '../render/route-cache.js';
import { getRedirectLocationOrThrow, routeIsRedirect } from '../redirects/index.js';
import { getRedirectLocationOrThrow, routeIsRedirect, RedirectComponentInstance } from '../redirects/index.js';
import {
createAssetLink,
createModuleScriptsSet,
@ -177,7 +177,7 @@ async function generatePage(
if(pageData.route.redirectRoute) {
pageModulePromise = ssrEntry.pageMap?.get(pageData.route.redirectRoute!.component);
} else {
pageModulePromise = () => Promise.resolve<any>({ default: () => {} });
pageModulePromise = () => Promise.resolve(RedirectComponentInstance);
}
}
if (!pageModulePromise) {

View file

@ -0,0 +1,10 @@
import type { ComponentInstance } from '../../@types/astro';
// A stub of a component instance for a given route
export const RedirectComponentInstance: ComponentInstance = {
default() {
return new Response(null, {
status: 301
});
}
};

View file

@ -18,10 +18,12 @@ export function redirectRouteGenerate(redirectRoute: RouteData, data: Params): s
return route.destination;
}
export function redirectRouteStatus(redirectRoute: RouteData): ValidRedirectStatus {
export function redirectRouteStatus(redirectRoute: RouteData, method = 'GET'): ValidRedirectStatus {
const routeData = redirectRoute.redirectRoute;
if(typeof routeData?.redirect === 'object') {
return routeData.redirect.status;
} else if(method !== 'GET') {
return 308;
}
return 301;
}

View file

@ -1,2 +1,3 @@
export { getRedirectLocationOrThrow } from './validate.js';
export { routeIsRedirect, redirectRouteGenerate, redirectRouteStatus } from './helpers.js';
export { RedirectComponentInstance } from './component.js';

View file

@ -114,7 +114,7 @@ export type RenderPage = {
export async function renderPage({ mod, renderContext, env, apiContext }: RenderPage) {
if(routeIsRedirect(renderContext.route)) {
return new Response(null, {
status: redirectRouteStatus(renderContext.route),
status: redirectRouteStatus(renderContext.route, renderContext.request.method),
headers: {
location: redirectRouteGenerate(renderContext.route, renderContext.params)
}

View file

@ -12,6 +12,9 @@ describe('Astro.redirect', () => {
root: './fixtures/ssr-redirect/',
output: 'server',
adapter: testAdapter(),
redirects: {
'/api/redirect': '/'
}
});
await fixture.build();
});
@ -37,6 +40,25 @@ describe('Astro.redirect', () => {
);
}
});
describe('Redirects config', () => {
it('Returns the redirect', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/api/redirect');
const response = await app.render(request);
expect(response.status).to.equal(301);
expect(response.headers.get('Location')).to.equal('/');
});
it('Uses 308 for non-GET methods', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/api/redirect', {
method: 'POST'
});
const response = await app.render(request);
expect(response.status).to.equal(308);
});
});
});
describe('output: "static"', () => {