Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: avoid running load function on invalid requests #9752

Merged
merged 15 commits into from
Jul 4, 2023
Merged
5 changes: 5 additions & 0 deletions .changeset/giant-rockets-love.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: avoid running load function on invalid requests
36 changes: 33 additions & 3 deletions packages/kit/src/runtime/server/respond.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { render_page } from './page/index.js';
import { render_response } from './page/render.js';
import { respond_with_error } from './page/respond_with_error.js';
import { is_form_content_type } from '../../utils/http.js';
import { handle_fatal_error, redirect_response } from './utils.js';
import { handle_fatal_error, method_not_allowed, redirect_response } from './utils.js';
import {
decode_pathname,
decode_params,
Expand Down Expand Up @@ -42,6 +42,10 @@ const default_filter = () => false;
/** @type {import('types').RequiredResolveOptions['preload']} */
const default_preload = ({ type }) => type === 'js' || type === 'css';

const page_methods = new Set(['GET', 'HEAD', 'POST']);

const allowed_page_methods = new Set(['GET', 'HEAD', 'OPTIONS']);

/**
* @param {Request} request
* @param {import('types').SSROptions} options
Expand Down Expand Up @@ -343,7 +347,6 @@ export async function respond(request, options, manifest, state) {
}

/**
*
* @param {import('@sveltejs/kit').RequestEvent} event
* @param {import('@sveltejs/kit').ResolveOptions} [opts]
*/
Expand Down Expand Up @@ -379,6 +382,8 @@ export async function respond(request, options, manifest, state) {
}

if (route) {
const method = /** @type {import('types').HttpMethod} */ (event.request.method);

/** @type {Response} */
let response;

Expand All @@ -395,7 +400,32 @@ export async function respond(request, options, manifest, state) {
} else if (route.endpoint && (!route.page || is_endpoint_request(event))) {
response = await render_endpoint(event, await route.endpoint(), state);
} else if (route.page) {
response = await render_page(event, route.page, options, manifest, state, resolve_opts);
if (page_methods.has(method)) {
response = await render_page(event, route.page, options, manifest, state, resolve_opts);
} else {
const allowed_methods = new Set(allowed_page_methods);
const node = await manifest._.nodes[route.page.leaf]();
if (node?.server?.actions) {
allowed_methods.add('POST');
}

if (method === 'OPTIONS') {
// This will deny CORS preflight requests implicitly because we don't
// add the required CORS headers to the response.
response = new Response(null, {
status: 204,
headers: {
allow: Array.from(allowed_methods.values()).join(', ')
}
});
} else {
const mod = [...allowed_methods].reduce((acc, curr) => {
acc[curr] = true;
return acc;
}, /** @type {Record<string, any>} */ ({}));
response = method_not_allowed(mod, method);
}
}
} else {
// a route will always have a page or an endpoint, but TypeScript
// doesn't know that
Expand Down
21 changes: 21 additions & 0 deletions packages/kit/test/apps/basics/test/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,27 @@ test.describe('Load', () => {
await page.goto(`/load/fetch-origin-external?port=${port}`);
expect(await page.textContent('h1')).toBe(`origin: ${new URL(baseURL).origin}`);
});

test('does not run when using invalid request methods', async ({ request }) => {
const load_url = '/load';

let response = await request.fetch(load_url, {
method: 'OPTIONS'
});

expect(response.status()).toBe(204);
expect(await response.text()).toBe('');
expect(response.headers()['allow']).toBe('GET, HEAD, OPTIONS');

const actions_url = '/actions/enhance';
response = await request.fetch(actions_url, {
method: 'OPTIONS'
});

expect(response.status()).toBe(204);
expect(await response.text()).toBe('');
expect(response.headers()['allow']).toBe('GET, HEAD, OPTIONS, POST');
});
});

test.describe('Routing', () => {
Expand Down