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

feat: add content-length headers to generated responses, via new text(...) helper #8371

Merged
merged 9 commits into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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/nine-walls-listen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Add `Content-Length` header to SvelteKit-generated responses
5 changes: 5 additions & 0 deletions .changeset/old-weeks-reflect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': minor
---

Add `text(...)` helper for generating text responses
26 changes: 25 additions & 1 deletion packages/kit/src/exports/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,36 @@ export function redirect(status, location) {
export function json(data, init) {
// TODO deprecate this in favour of `Response.json` when it's
// more widely supported
const body = JSON.stringify(data);

// we can't just do `text(JSON.stringify(data), init)` because
// it will set a default `content-type` header. duplicated code
// means less duplicated work
const headers = new Headers(init?.headers);
if (!headers.has('content-length')) {
headers.set('content-length', encoder.encode(body).byteLength.toString());
}

if (!headers.has('content-type')) {
headers.set('content-type', 'application/json');
}

return new Response(JSON.stringify(data), {
return new Response(body, {
...init,
headers
});
}

const encoder = new TextEncoder();

/** @type {import('@sveltejs/kit').text} */
export function text(body, init) {
const headers = new Headers(init?.headers);
if (!headers.has('content-length')) {
headers.set('content-length', encoder.encode(body).byteLength.toString());
}

return new Response(body, {
...init,
headers
});
Expand Down
3 changes: 2 additions & 1 deletion packages/kit/src/runtime/server/data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { once } from '../../../utils/functions.js';
import { load_server_data } from '../page/load_data.js';
import { clarify_devalue_error, handle_error_and_jsonify, serialize_data_node } from '../utils.js';
import { normalize_path } from '../../../utils/url.js';
import { text } from '../../../exports/index.js';

export const INVALIDATED_PARAM = 'x-sveltekit-invalidated';

Expand Down Expand Up @@ -136,7 +137,7 @@ export async function render_data(
* @param {number} [status]
*/
function json_response(json, status = 200) {
return new Response(json, {
return text(json, {
status,
headers: {
'content-type': 'application/json',
Expand Down
12 changes: 6 additions & 6 deletions packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
validate_page_server_exports,
validate_server_exports
} from '../../utils/exports.js';
import { error, json } from '../../exports/index.js';
import { error, json, text } from '../../exports/index.js';

/* global __SVELTEKIT_ADAPTER_NAME__ */

Expand Down Expand Up @@ -52,15 +52,15 @@ export async function respond(request, options, state) {
if (request.headers.get('accept') === 'application/json') {
return json(csrf_error.body, { status: csrf_error.status });
}
return new Response(csrf_error.body.message, { status: csrf_error.status });
return text(csrf_error.body.message, { status: csrf_error.status });
}
}

let decoded;
try {
decoded = decode_pathname(url.pathname);
} catch {
return new Response('Malformed URI', { status: 400 });
return text('Malformed URI', { status: 400 });
}

/** @type {import('types').SSRRoute | null} */
Expand All @@ -71,7 +71,7 @@ export async function respond(request, options, state) {

if (options.paths.base && !state.prerendering?.fallback) {
if (!decoded.startsWith(options.paths.base)) {
return new Response('Not found', { status: 404 });
return text('Not found', { status: 404 });
}
decoded = decoded.slice(options.paths.base.length) || '/';
}
Expand Down Expand Up @@ -364,7 +364,7 @@ export async function respond(request, options, state) {
}

if (state.initiator === GENERIC_ERROR) {
return new Response('Internal Server Error', {
return text('Internal Server Error', {
status: 500
});
}
Expand All @@ -383,7 +383,7 @@ export async function respond(request, options, state) {
}

if (state.prerendering) {
return new Response('not found', { status: 404 });
return text('not found', { status: 404 });
}

// we can't load the endpoint from our own manifest,
Expand Down
7 changes: 4 additions & 3 deletions packages/kit/src/runtime/server/page/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { text } from '../../../exports/index.js';
import { compact } from '../../../utils/array.js';
import { normalize_error } from '../../../utils/error.js';
import { add_data_suffix } from '../../../utils/url.js';
Expand Down Expand Up @@ -31,7 +32,7 @@ import { respond_with_error } from './respond_with_error.js';
export async function render_page(event, route, page, options, state, resolve_opts) {
if (state.initiator === route) {
// infinite request cycle detected
return new Response(`Not found: ${event.url.pathname}`, {
return text(`Not found: ${event.url.pathname}`, {
status: 404
});
}
Expand Down Expand Up @@ -237,7 +238,7 @@ export async function render_page(event, route, page, options, state, resolve_op
});

state.prerendering.dependencies.set(data_pathname, {
response: new Response(body),
response: text(body),
body
});
}
Expand Down Expand Up @@ -291,7 +292,7 @@ export async function render_page(event, route, page, options, state, resolve_op
.join(',')}]}`;

state.prerendering.dependencies.set(data_pathname, {
response: new Response(body),
response: text(body),
body
});
}
Expand Down
3 changes: 2 additions & 1 deletion packages/kit/src/runtime/server/page/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { s } from '../../../utils/misc.js';
import { Csp } from './csp.js';
import { uneval_action_response } from './actions.js';
import { clarify_devalue_error } from '../utils.js';
import { text } from '../../../exports/index.js';

// TODO rename this function/module

Expand Down Expand Up @@ -386,7 +387,7 @@ export async function render_response({
}
}

return new Response(html, {
return text(html, {
status,
headers
});
Expand Down
10 changes: 5 additions & 5 deletions packages/kit/src/runtime/server/utils.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as devalue from 'devalue';
import { json, text } from '../../exports/index.js';
import { coalesce_to_error } from '../../utils/error.js';
import { negotiate } from '../../utils/http.js';
import { has_data_suffix } from '../../utils/url.js';
Expand Down Expand Up @@ -26,7 +27,7 @@ export const GENERIC_ERROR = {
* @param {import('types').HttpMethod} method
*/
export function method_not_allowed(mod, method) {
return new Response(`${method} method not allowed`, {
return text(`${method} method not allowed`, {
status: 405,
headers: {
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/405
Expand Down Expand Up @@ -74,7 +75,7 @@ export function get_option(nodes, option) {
* @param {string} message
*/
export function static_error_page(options, status, message) {
return new Response(options.error_template({ status, message }), {
return text(options.error_template({ status, message }), {
headers: { 'content-type': 'text/html; charset=utf-8' },
status
});
Expand All @@ -97,9 +98,8 @@ export async function handle_fatal_error(event, options, error) {
]);

if (has_data_suffix(new URL(event.request.url).pathname) || type === 'application/json') {
return new Response(JSON.stringify(body), {
status,
headers: { 'content-type': 'application/json; charset=utf-8' }
return json(body, {
status
});
}

Expand Down
12 changes: 12 additions & 0 deletions packages/kit/test/apps/basics/test/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ test.describe('Content-Type', () => {
});
});

test.describe('Content-Length', () => {
test('sets Content-Length on page', async ({ request }) => {
const response = await request.get('/content-length-header');

// TODO this would ideally be a unit test of `Server`,
// as would most of the tests in this file
if (!response.headers()['content-encoding']) {
expect(+response.headers()['content-length']).toBeGreaterThan(1000);
}
});
});

test.describe('Cookies', () => {
test('does not forward cookies from external domains', async ({ request }) => {
const { close, port } = await start_server(async (req, res) => {
Expand Down
9 changes: 8 additions & 1 deletion packages/kit/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1122,10 +1122,17 @@ export interface Redirect {
/**
* Create a JSON `Response` object from the supplied data.
* @param data The value that will be serialized as JSON.
* @param init Options such as `status` and `headers` that will be added to the response. A `Content-Type: application/json` header will be added automatically.
* @param init Options such as `status` and `headers` that will be added to the response. `Content-Type: application/json` and `Content-Length` headers will be added automatically.
*/
export function json(data: any, init?: ResponseInit): Response;

/**
* Create a `Response` object from the supplied body.
* @param body The value that will be serialized as JSON.
* @param init Options such as `status` and `headers` that will be added to the response. A `Content-Length` header will be added automatically.
*/
export function text(body: string, init?: ResponseInit): Response;

/**
* Create an `ActionFailure` object.
*/
Expand Down