Skip to content

Commit

Permalink
remove ability for +page.server.js to respond to GET requests with JS…
Browse files Browse the repository at this point in the history
…ON (#6007)

* remove ability for +page.server.js to respond to GET requests with JSON

* Update .changeset/fresh-pigs-tease.md

Co-authored-by: Ignatius Bagus <ignatius.mbs@gmail.com>

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Co-authored-by: Ignatius Bagus <ignatius.mbs@gmail.com>
  • Loading branch information
3 people authored Aug 18, 2022
1 parent b9f8b73 commit dfeb672
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 92 deletions.
5 changes: 5 additions & 0 deletions .changeset/fresh-pigs-tease.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[breaking] remove ability for `+page.server.js` to respond to `GET` requests with JSON
18 changes: 7 additions & 11 deletions packages/kit/src/runtime/server/page/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ export async function render_page(event, route, options, state, resolve_opts) {
'application/json'
]);

if (accept === 'application/json') {
if (
accept === 'application/json' &&
event.request.method !== 'GET' &&
event.request.method !== 'HEAD'
) {
const node = await options.manifest._.nodes[route.leaf]();
if (node.server) {
return handle_json_request(event, options, node.server);
Expand Down Expand Up @@ -346,8 +350,8 @@ function get_page_config(leaf, options) {
* @param {import('types').SSRNode['server']} mod
*/
export async function handle_json_request(event, options, mod) {
const method = /** @type {import('types').HttpMethod} */ (event.request.method);
const handler = mod[method === 'HEAD' || method === 'GET' ? 'load' : method];
const method = /** @type {'POST' | 'PUT' | 'PATCH' | 'DELETE'} */ (event.request.method);
const handler = mod[method];

if (!handler) {
return method_not_allowed(mod, method);
Expand All @@ -357,14 +361,6 @@ export async function handle_json_request(event, options, mod) {
// @ts-ignore
const result = await handler.call(null, event);

if (method === 'HEAD') {
return new Response();
}

if (method === 'GET') {
return json(result);
}

if (result?.errors) {
// @ts-ignore
return json({ errors: result.errors }, { status: result.status || 400 });
Expand Down
81 changes: 0 additions & 81 deletions packages/kit/test/apps/basics/test/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,40 +124,6 @@ test.describe('Errors', () => {
expect(await response.text()).toMatch('thisvariableisnotdefined is not defined');
});

test('page endpoint thrown error respects `accept: application/json`', async ({ request }) => {
const response = await request.get('/errors/page-endpoint/get-implicit', {
headers: {
accept: 'application/json'
}
});

const { message, name, stack, fancy } = await response.json();

expect(response.status()).toBe(500);
expect(name).toBe('FancyError');
expect(message).toBe('oops');
expect(fancy).toBe(true);

if (process.env.DEV) {
expect(stack.split('\n').length).toBeGreaterThan(1);
} else {
expect(stack.split('\n').length).toBe(1);
}
});

test('page endpoint returned error respects `accept: application/json`', async ({ request }) => {
const response = await request.get('/errors/page-endpoint/get-explicit', {
headers: {
accept: 'application/json'
}
});

const { message } = await response.json();

expect(response.status()).toBe(400);
expect(message).toBe('oops');
});

test('returns 400 when accessing a malformed URI', async ({ page }) => {
const response = await page.goto('/%c0%ae%c0%ae/etc/passwd');
if (process.env.DEV) {
Expand Down Expand Up @@ -211,53 +177,6 @@ test.describe('Routing', () => {
});

test.describe('Shadowed pages', () => {
test('responds to HEAD requests from endpoint', async ({ request }) => {
const url = '/shadowed/simple';

const opts = {
headers: {
accept: 'application/json'
}
};

const responses = {
head: await request.head(url, opts),
get: await request.get(url, opts)
};

const headers = {
head: responses.head.headers(),
get: responses.get.headers()
};

expect(responses.head.status()).toBe(200);
expect(responses.get.status()).toBe(200);
expect(await responses.head.text()).toBe('');
expect(await responses.get.json()).toEqual({ answer: 42 });

['date', 'transfer-encoding'].forEach((name) => {
delete headers.head[name];
delete headers.get[name];
});

expect(headers.get).toEqual({
...headers.head,
'content-type': 'application/json'
});
});

test('Responds from endpoint if Accept includes application/json but not text/html', async ({
request
}) => {
const response = await request.get('/shadowed/simple', {
headers: {
accept: 'application/json'
}
});

expect(await response.json()).toEqual({ answer: 42 });
});

test('Action can return undefined', async ({ request }) => {
const response = await request.post('/shadowed/simple/post', {
headers: {
Expand Down

0 comments on commit dfeb672

Please sign in to comment.