From 1d62b7bc1e10c9a3efdf0d5384f4c5647bd5de43 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 22 Nov 2022 13:36:09 -0500 Subject: [PATCH 1/5] disallow status codes outside 400-599 --- .changeset/four-lamps-walk.md | 5 +++ packages/kit/src/exports/index.js | 4 +++ packages/kit/src/runtime/control.js | 53 ----------------------------- 3 files changed, 9 insertions(+), 53 deletions(-) create mode 100644 .changeset/four-lamps-walk.md diff --git a/.changeset/four-lamps-walk.md b/.changeset/four-lamps-walk.md new file mode 100644 index 000000000000..274abce807de --- /dev/null +++ b/.changeset/four-lamps-walk.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +Disallow status codes outside 400-599 diff --git a/packages/kit/src/exports/index.js b/packages/kit/src/exports/index.js index b0ae3243e0aa..4717bf369023 100644 --- a/packages/kit/src/exports/index.js +++ b/packages/kit/src/exports/index.js @@ -8,6 +8,10 @@ import { HttpError, Redirect, ValidationError } from '../runtime/control.js'; * @param {any} message */ export function error(status, message) { + if (status < 400 || status > 599) { + throw new Error(`HTTP error status codes must be between 400 and 599 — ${status} is invalid`); + } + return new HttpError(status, message); } diff --git a/packages/kit/src/runtime/control.js b/packages/kit/src/runtime/control.js index 2bcf963985a2..16ea65416416 100644 --- a/packages/kit/src/runtime/control.js +++ b/packages/kit/src/runtime/control.js @@ -43,56 +43,3 @@ export class ValidationError { this.data = data; } } - -/** - * Creates an `HttpError` object with an HTTP status code and an optional message. - * This object, if thrown during request handling, will cause SvelteKit to - * return an error response without invoking `handleError` - * @param {number} status - * @param {string | undefined} [message] - */ -export function error(status, message) { - return new HttpError(status, message); -} - -/** - * Creates a `Redirect` object. If thrown during request handling, SvelteKit will - * return a redirect response. - * @param {300 | 301 | 302 | 303 | 304 | 305 | 306 | 307 | 308} status - * @param {string} location - */ -export function redirect(status, location) { - if (isNaN(status) || status < 300 || status > 308) { - throw new Error('Invalid status code'); - } - - return new Redirect(status, location); -} - -/** - * Generates a JSON `Response` object from the supplied data. - * @param {any} data - * @param {ResponseInit} [init] - */ -export function json(data, init) { - // TODO deprecate this in favour of `Response.json` when it's - // more widely supported - const headers = new Headers(init?.headers); - if (!headers.has('content-type')) { - headers.set('content-type', 'application/json'); - } - - return new Response(JSON.stringify(data), { - ...init, - headers - }); -} - -/** - * Generates a `ValidationError` object. - * @param {number} status - * @param {Record | undefined} [data] - */ -export function invalid(status, data) { - return new ValidationError(status, data); -} From 71c832f35b6a44ae4b6373de8a6d6e5ec56fc602 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 22 Nov 2022 13:44:09 -0500 Subject: [PATCH 2/5] dev/server only --- packages/kit/src/exports/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/exports/index.js b/packages/kit/src/exports/index.js index 4717bf369023..6b4549f60ccf 100644 --- a/packages/kit/src/exports/index.js +++ b/packages/kit/src/exports/index.js @@ -8,7 +8,7 @@ import { HttpError, Redirect, ValidationError } from '../runtime/control.js'; * @param {any} message */ export function error(status, message) { - if (status < 400 || status > 599) { + if ((!__SVELTEKIT_BROWSER__ || __SVELTEKIT_DEV__) && (status < 400 || status > 599)) { throw new Error(`HTTP error status codes must be between 400 and 599 — ${status} is invalid`); } From 6c994c3a1d7416a84cbc04030ba6b192faf37fa0 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 22 Nov 2022 20:00:38 +0100 Subject: [PATCH 3/5] Update .changeset/four-lamps-walk.md Co-authored-by: Conduitry --- .changeset/four-lamps-walk.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/four-lamps-walk.md b/.changeset/four-lamps-walk.md index 274abce807de..d2abb0ef2b2a 100644 --- a/.changeset/four-lamps-walk.md +++ b/.changeset/four-lamps-walk.md @@ -2,4 +2,4 @@ '@sveltejs/kit': patch --- -Disallow status codes outside 400-599 +[breaking] Disallow error status codes outside 400-599 From a412b0bddc1bdf9d152cf74aa65a91ecbef758da Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 22 Nov 2022 14:12:28 -0500 Subject: [PATCH 4/5] apply dead code elimination to redirect function --- packages/kit/src/exports/index.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/kit/src/exports/index.js b/packages/kit/src/exports/index.js index 6b4549f60ccf..0ad15b8d92fa 100644 --- a/packages/kit/src/exports/index.js +++ b/packages/kit/src/exports/index.js @@ -8,7 +8,10 @@ import { HttpError, Redirect, ValidationError } from '../runtime/control.js'; * @param {any} message */ export function error(status, message) { - if ((!__SVELTEKIT_BROWSER__ || __SVELTEKIT_DEV__) && (status < 400 || status > 599)) { + if ( + (!__SVELTEKIT_BROWSER__ || __SVELTEKIT_DEV__) && + (isNaN(status) || status < 400 || status > 599) + ) { throw new Error(`HTTP error status codes must be between 400 and 599 — ${status} is invalid`); } @@ -17,7 +20,10 @@ export function error(status, message) { /** @type {import('@sveltejs/kit').redirect} */ export function redirect(status, location) { - if (isNaN(status) || status < 300 || status > 308) { + if ( + (!__SVELTEKIT_BROWSER__ || __SVELTEKIT_DEV__) && + (isNaN(status) || status < 300 || status > 308) + ) { throw new Error('Invalid status code'); } From c4a9a554373a452dd4aaf159e20eaafa2be25a84 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 22 Nov 2022 14:54:10 -0500 Subject: [PATCH 5/5] oops, need to check for a different error in browser+prod now --- packages/kit/test/apps/basics/test/test.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index be47fcb68a93..8678d1b1a6b6 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1295,28 +1295,32 @@ test.describe('Redirects', () => { await clicknav('[href="/redirect/missing-status/a"]'); + const message = process.env.DEV || !javaScriptEnabled ? 'Invalid status code' : 'Redirect loop'; + expect(page.url()).toBe(`${baseURL}/redirect/missing-status/a`); expect(await page.textContent('h1')).toBe('500'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Invalid status code"' + `This is your custom error page saying: "${message}"` ); if (!javaScriptEnabled) { // handleError is not invoked for client-side navigation const lines = read_errors('/redirect/missing-status/a').stack.split('\n'); - expect(lines[0]).toBe('Error: Invalid status code'); + expect(lines[0]).toBe(`Error: ${message}`); } }); - test('errors on invalid status', async ({ baseURL, page, clicknav }) => { + test('errors on invalid status', async ({ baseURL, page, clicknav, javaScriptEnabled }) => { await page.goto('/redirect'); await clicknav('[href="/redirect/missing-status/b"]'); + const message = process.env.DEV || !javaScriptEnabled ? 'Invalid status code' : 'Redirect loop'; + expect(page.url()).toBe(`${baseURL}/redirect/missing-status/b`); expect(await page.textContent('h1')).toBe('500'); expect(await page.textContent('#message')).toBe( - 'This is your custom error page saying: "Invalid status code"' + `This is your custom error page saying: "${message}"` ); });