Skip to content

Commit

Permalink
fix: avoid running load function on invalid requests (#9752)
Browse files Browse the repository at this point in the history
  • Loading branch information
eltigerchino committed Jul 4, 2023
1 parent 42c9b93 commit 67a0d86
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 3 deletions.
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

0 comments on commit 67a0d86

Please sign in to comment.