mirror of
https://github.com/withastro/astro.git
synced 2024-12-23 21:53:55 -05:00
Fix endpoint response with immutable header (#9876)
* add test * added runtime endpoint test for `new Response` * added `try..catch` for reroute directive handling Fixes #9871 * added changeset * replaced `try..catch` with HTTP status code check based on the suggestion of @lilnasy * updated changeset description * added more tests for the endpoint reroute header * fixed grammar in `renderEndpoint` comment * updated endpoint tests to check for the reroute directive header in lower-case * updated changeset description
This commit is contained in:
parent
3a50cbf6ed
commit
e9027f194b
3 changed files with 103 additions and 1 deletions
5
.changeset/silver-peaches-pump.md
Normal file
5
.changeset/silver-peaches-pump.md
Normal file
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
"astro": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Fixes an issue where using `Response.redirect` in an endpoint led to an error.
|
|
@ -42,6 +42,12 @@ export async function renderEndpoint(
|
||||||
const response = await handler.call(mod, context);
|
const response = await handler.call(mod, context);
|
||||||
// Endpoints explicitly returning 404 or 500 response status should
|
// Endpoints explicitly returning 404 or 500 response status should
|
||||||
// NOT be subject to rerouting to 404.astro or 500.astro.
|
// NOT be subject to rerouting to 404.astro or 500.astro.
|
||||||
response.headers.set(REROUTE_DIRECTIVE_HEADER, 'no');
|
if (response.status === 404 || response.status === 500) {
|
||||||
|
// Only `Response.redirect` headers are immutable, therefore a `try..catch` is not necessary.
|
||||||
|
// Note: `Response.redirect` can only be called with HTTP status codes: 301, 302, 303, 307, 308.
|
||||||
|
// Source: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static#parameters
|
||||||
|
response.headers.set(REROUTE_DIRECTIVE_HEADER, 'no');
|
||||||
|
}
|
||||||
|
|
||||||
return response;
|
return response;
|
||||||
}
|
}
|
||||||
|
|
91
packages/astro/test/units/routing/endpoints.test.js
Normal file
91
packages/astro/test/units/routing/endpoints.test.js
Normal file
|
@ -0,0 +1,91 @@
|
||||||
|
import {
|
||||||
|
createBasicSettings,
|
||||||
|
createFs,
|
||||||
|
createRequestAndResponse,
|
||||||
|
defaultLogger,
|
||||||
|
} from '../test-utils.js';
|
||||||
|
import { fileURLToPath } from 'node:url';
|
||||||
|
import { expect } from 'chai';
|
||||||
|
import { createContainer } from '../../../dist/core/dev/container.js';
|
||||||
|
import testAdapter from '../../test-adapter.js';
|
||||||
|
|
||||||
|
const root = new URL('../../fixtures/api-routes/', import.meta.url);
|
||||||
|
const fileSystem = {
|
||||||
|
'/src/pages/response-redirect.ts': `export const GET = ({ url }) => Response.redirect("https://example.com/destination", 307)`,
|
||||||
|
'/src/pages/response.ts': `export const GET = ({ url }) => new Response(null, { headers: { Location: "https://example.com/destination" }, status: 307 })`,
|
||||||
|
'/src/pages/not-found.ts': `export const GET = ({ url }) => new Response('empty', { headers: { "Content-Type": "text/plain" }, status: 404 })`,
|
||||||
|
'/src/pages/internal-error.ts': `export const GET = ({ url }) => new Response('something went wrong', { headers: { "Content-Type": "text/plain" }, status: 500 })`,
|
||||||
|
};
|
||||||
|
|
||||||
|
describe('endpoints', () => {
|
||||||
|
let container;
|
||||||
|
let settings;
|
||||||
|
|
||||||
|
before(async () => {
|
||||||
|
const fs = createFs(fileSystem, root);
|
||||||
|
settings = await createBasicSettings({
|
||||||
|
root: fileURLToPath(root),
|
||||||
|
output: 'server',
|
||||||
|
adapter: testAdapter(),
|
||||||
|
});
|
||||||
|
container = await createContainer({
|
||||||
|
fs,
|
||||||
|
settings,
|
||||||
|
logger: defaultLogger,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
after(async () => {
|
||||||
|
await container.close();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return a redirect response with location header', async () => {
|
||||||
|
const { req, res, done } = createRequestAndResponse({
|
||||||
|
method: 'GET',
|
||||||
|
url: '/response-redirect',
|
||||||
|
});
|
||||||
|
container.handle(req, res);
|
||||||
|
await done;
|
||||||
|
const headers = res.getHeaders();
|
||||||
|
expect(headers).to.deep.include({ location: 'https://example.com/destination' });
|
||||||
|
expect(headers).not.to.deep.include({ 'x-astro-reroute': 'no' });
|
||||||
|
expect(res.statusCode).to.equal(307);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return a response with location header', async () => {
|
||||||
|
const { req, res, done } = createRequestAndResponse({
|
||||||
|
method: 'GET',
|
||||||
|
url: '/response',
|
||||||
|
});
|
||||||
|
container.handle(req, res);
|
||||||
|
await done;
|
||||||
|
const headers = res.getHeaders();
|
||||||
|
expect(headers).to.deep.include({ location: 'https://example.com/destination' });
|
||||||
|
expect(headers).not.to.deep.include({ 'x-astro-reroute': 'no' });
|
||||||
|
expect(res.statusCode).to.equal(307);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should append reroute header for HTTP status 404', async () => {
|
||||||
|
const { req, res, done } = createRequestAndResponse({
|
||||||
|
method: 'GET',
|
||||||
|
url: '/not-found',
|
||||||
|
});
|
||||||
|
container.handle(req, res);
|
||||||
|
await done;
|
||||||
|
const headers = res.getHeaders();
|
||||||
|
expect(headers).to.deep.include({ 'x-astro-reroute': 'no' });
|
||||||
|
expect(res.statusCode).to.equal(404);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should append reroute header for HTTP status 500', async () => {
|
||||||
|
const { req, res, done } = createRequestAndResponse({
|
||||||
|
method: 'GET',
|
||||||
|
url: '/internal-error',
|
||||||
|
});
|
||||||
|
container.handle(req, res);
|
||||||
|
await done;
|
||||||
|
const headers = res.getHeaders();
|
||||||
|
expect(headers).to.deep.include({ 'x-astro-reroute': 'no' });
|
||||||
|
expect(res.statusCode).to.equal(500);
|
||||||
|
});
|
||||||
|
});
|
Loading…
Reference in a new issue