Skip to content

Commit

Permalink
Use status 308 for non-GET redirects
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewp committed May 23, 2023
1 parent 2904ced commit 3ce8db3
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 10 deletions.
21 changes: 15 additions & 6 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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
);
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
10 changes: 10 additions & 0 deletions packages/astro/src/core/redirects/component.ts
Original file line number Diff line number Diff line change
@@ -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
});
}
};
4 changes: 3 additions & 1 deletion packages/astro/src/core/redirects/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
1 change: 1 addition & 0 deletions packages/astro/src/core/redirects/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export { getRedirectLocationOrThrow } from './validate.js';
export { routeIsRedirect, redirectRouteGenerate, redirectRouteStatus } from './helpers.js';
export { RedirectComponentInstance } from './component.js';
2 changes: 1 addition & 1 deletion packages/astro/src/core/render/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
22 changes: 22 additions & 0 deletions packages/astro/test/redirects.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ describe('Astro.redirect', () => {
root: './fixtures/ssr-redirect/',
output: 'server',
adapter: testAdapter(),
redirects: {
'/api/redirect': '/'
}
});
await fixture.build();
});
Expand All @@ -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"', () => {
Expand Down

0 comments on commit 3ce8db3

Please sign in to comment.