Skip to content

Commit 10d7b44

Browse files
Merge commit from fork
* fix: safer redirects * changeset * types * better location for fix
1 parent 3202ed6 commit 10d7b44

8 files changed

Lines changed: 70 additions & 11 deletions

File tree

.changeset/brown-eggs-lose.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@sveltejs/kit': patch
3+
---
4+
5+
fix: better validation for `redirect` inputs

packages/kit/src/exports/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ export function isHttpError(e, status) {
106106
* @param {300 | 301 | 302 | 303 | 304 | 305 | 306 | 307 | 308 | ({} & number)} status The [HTTP status code](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status#redirection_messages). Must be in the range 300-308.
107107
* @param {string | URL} location The location to redirect to.
108108
* @throws {Redirect} This error instructs SvelteKit to redirect to the specified location.
109-
* @throws {Error} If the provided status is invalid.
109+
* @throws {Error} If the provided status is invalid or the location cannot be used as a header value.
110110
* @return {never}
111111
*/
112112
export function redirect(status, location) {

packages/kit/src/exports/index.spec.js

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { normalizeUrl } from './index.js';
1+
import { isRedirect, normalizeUrl, redirect } from './index.js';
22
import { assert, describe, it } from 'vitest';
33

44
describe('normalizeUrl', () => {
@@ -51,3 +51,26 @@ describe('normalizeUrl', () => {
5151
assert.equal(denormalize('/baz').href, 'http://example.com/baz/__route.js');
5252
});
5353
});
54+
55+
describe('redirect', () => {
56+
it('throws Redirect for valid locations', () => {
57+
try {
58+
redirect(307, '/valid-location');
59+
assert.fail('Expected redirect to throw');
60+
} catch (e) {
61+
if (!isRedirect(e)) {
62+
assert.fail('Expected a Redirect error');
63+
}
64+
65+
assert.equal(e.status, 307);
66+
assert.equal(e.location, '/valid-location');
67+
}
68+
});
69+
70+
it('throws a descriptive error for invalid redirect locations', () => {
71+
assert.throws(
72+
() => redirect(307, '/invalid\r\nset-cookie: x=y'),
73+
'Invalid redirect location "/invalid\\r\\nset-cookie: x=y": this string contains characters that cannot be used in HTTP headers'
74+
);
75+
});
76+
});

packages/kit/src/exports/internal/index.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,15 @@ export class Redirect {
2727
* @param {string} location
2828
*/
2929
constructor(status, location) {
30+
try {
31+
new Headers({ location });
32+
} catch {
33+
throw new Error(
34+
`Invalid redirect location ${JSON.stringify(location)}: ` +
35+
'this string contains characters that cannot be used in HTTP headers'
36+
);
37+
}
38+
3039
this.status = status;
3140
this.location = location;
3241
}

packages/kit/src/runtime/server/respond.js

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -525,14 +525,18 @@ export async function internal_respond(request, options, manifest, state) {
525525
return response;
526526
} catch (e) {
527527
if (e instanceof Redirect) {
528-
const response =
529-
is_data_request || remote_id
530-
? redirect_json_response(e)
531-
: route?.page && is_action_json_request(event)
532-
? action_json_redirect(e)
533-
: redirect_response(e.status, e.location);
534-
add_cookies_to_headers(response.headers, new_cookies.values());
535-
return response;
528+
try {
529+
const response =
530+
is_data_request || remote_id
531+
? redirect_json_response(e)
532+
: route?.page && is_action_json_request(event)
533+
? action_json_redirect(e)
534+
: redirect_response(e.status, e.location);
535+
add_cookies_to_headers(response.headers, new_cookies.values());
536+
return response;
537+
} catch (err) {
538+
return await handle_fatal_error(event, event_state, options, err);
539+
}
536540
}
537541
return await handle_fatal_error(event, event_state, options, e);
538542
}

packages/kit/test/apps/basics/src/hooks.server.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,11 @@ export const handle = sequence(
141141
},
142142
async ({ event, resolve }) => {
143143
if (event.url.pathname.includes('/redirect/in-handle')) {
144+
const location = event.url.searchParams.get('location');
145+
if (location) {
146+
redirect(307, location);
147+
}
148+
144149
if (event.url.search === '?throw') {
145150
redirect(307, event.url.origin + '/redirect/c');
146151
} else if (event.url.search.includes('cookies')) {

packages/kit/test/apps/basics/test/cross-platform/test.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,19 @@ test.describe('Redirects', () => {
603603
expect(page.url()).toBe(`${baseURL}/redirect`);
604604
});
605605

606+
test('invalid redirect location in handle hook returns 500 without crashing server', async ({
607+
request
608+
}) => {
609+
const bad_redirect = await request.get(
610+
'/redirect/in-handle?location=%2Fredirect%2Fc%0D%0Aset-cookie%3A%20evil%3D1'
611+
);
612+
613+
expect(bad_redirect.status()).toBe(500);
614+
615+
const follow_up = await request.get('/');
616+
expect(follow_up.status()).toBe(200);
617+
});
618+
606619
test('sets cookies when redirect in handle hook', async ({ page, app, javaScriptEnabled }) => {
607620
await page.goto('/cookies/set');
608621
let span = page.locator('#cookie-value');

packages/kit/types/index.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2728,7 +2728,7 @@ declare module '@sveltejs/kit' {
27282728
* @param status The [HTTP status code](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status#redirection_messages). Must be in the range 300-308.
27292729
* @param location The location to redirect to.
27302730
* @throws {Redirect} This error instructs SvelteKit to redirect to the specified location.
2731-
* @throws {Error} If the provided status is invalid.
2731+
* @throws {Error} If the provided status is invalid or the location cannot be used as a header value.
27322732
* */
27332733
export function redirect(status: 300 | 301 | 302 | 303 | 304 | 305 | 306 | 307 | 308 | ({} & number), location: string | URL): never;
27342734
/**

0 commit comments

Comments
 (0)