Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
33 changes: 27 additions & 6 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 @@ -39,6 +39,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 @@ -324,7 +328,6 @@ export async function respond(request, options, manifest, state) {
}

/**
*
* @param {import('types').RequestEvent} event
* @param {import('types').ResolveOptions} [opts]
*/
Expand Down Expand Up @@ -360,6 +363,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 @@ -375,12 +380,28 @@ 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) {
} else if (route.page && page_methods.has(method)) {
console.log(page_methods.has(method));
response = await render_page(event, route.page, options, manifest, state, resolve_opts);
} else if (route.page && method === 'OPTIONS') {
// if no OPTIONS endpoint exists, fallback to handling it gracefully.
const allowed_methods = new Set(allowed_page_methods);

const node = await manifest._.nodes[route.page.leaf]();
if (node?.server?.actions) {
allowed_methods.add('POST');
}

response = new Response(null, {
status: 204,
headers: {
allow: Array.from(allowed_methods.values()).join(', ')
}
});
} else {
// a route will always have a page or an endpoint, but TypeScript
// doesn't know that
throw new Error('This should never happen');
// if the request method is not allowed by us for the page / endpoint.
const mod = route.endpoint ? await route.endpoint() : {};
return method_not_allowed(mod, method);
}

return response;
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