Skip to content

Commit

Permalink
feat: add content-length headers to generated responses, via new `tex…
Browse files Browse the repository at this point in the history
…t(...)` helper (#8371)

* add content-length headers to generated responses, via new `text(...)` helper - closes #5749

* correctly determine length

* fix

* Update packages/kit/types/index.d.ts

* Update .changeset/nine-walls-listen.md

* Update .changeset/old-weeks-reflect.md

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
  • Loading branch information
3 people committed Jan 19, 2023
1 parent 2726e7c commit 06a56ae
Show file tree
Hide file tree
Showing 11 changed files with 74 additions and 18 deletions.
5 changes: 5 additions & 0 deletions .changeset/nine-walls-listen.md
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: add `Content-Length` header to SvelteKit-generated responses
5 changes: 5 additions & 0 deletions .changeset/old-weeks-reflect.md
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': minor
---

feat: add `text(...)` helper for generating text responses
26 changes: 25 additions & 1 deletion packages/kit/src/exports/index.js
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
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 @@ -139,7 +140,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
7 changes: 4 additions & 3 deletions packages/kit/src/runtime/server/page/index.js
@@ -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 @@ -32,7 +33,7 @@ import { respond_with_error } from './respond_with_error.js';
export async function render_page(event, route, page, options, manifest, 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 @@ -239,7 +240,7 @@ export async function render_page(event, route, page, options, manifest, state,
});

state.prerendering.dependencies.set(data_pathname, {
response: new Response(body),
response: text(body),
body
});
}
Expand Down Expand Up @@ -294,7 +295,7 @@ export async function render_page(event, route, page, options, manifest, state,
.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
Expand Up @@ -9,6 +9,7 @@ import { uneval_action_response } from './actions.js';
import { clarify_devalue_error } from '../utils.js';
import { assets, base, version } from '../../shared.js';
import { env } from '../../env-public.js';
import { text } from '../../../exports/index.js';

// TODO rename this function/module

Expand Down Expand Up @@ -408,7 +409,7 @@ export async function render_response({
}
}

return new Response(transformed, {
return text(transformed, {
status,
headers
});
Expand Down
12 changes: 6 additions & 6 deletions packages/kit/src/runtime/server/respond.js
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';
import * as paths from '../shared.js';

/* global __SVELTEKIT_ADAPTER_NAME__ */
Expand Down Expand Up @@ -53,15 +53,15 @@ export async function respond(request, options, manifest, 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 @@ -72,7 +72,7 @@ export async function respond(request, options, manifest, state) {

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

if (state.initiator === GENERIC_ERROR) {
return new Response('Internal Server Error', {
return text('Internal Server Error', {
status: 500
});
}
Expand All @@ -394,7 +394,7 @@ export async function respond(request, options, manifest, 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
10 changes: 5 additions & 5 deletions packages/kit/src/runtime/server/utils.js
@@ -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 @@ -27,7 +28,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 @@ -75,7 +76,7 @@ export function get_option(nodes, option) {
* @param {string} message
*/
export function static_error_page(options, status, message) {
return new Response(options.templates.error({ status, message }), {
return text(options.templates.error({ status, message }), {
headers: { 'content-type': 'text/html; charset=utf-8' },
status
});
Expand All @@ -98,9 +99,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
Empty file.
12 changes: 12 additions & 0 deletions packages/kit/test/apps/basics/test/server.test.js
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, start_server }) => {
const { port } = await start_server(async (req, res) => {
Expand Down
9 changes: 8 additions & 1 deletion packages/kit/types/index.d.ts
Expand Up @@ -1123,10 +1123,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 used as-is.
* @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.
* @param status The [HTTP status code](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status#client_error_responses). Must be in the range 400-599.
Expand Down

0 comments on commit 06a56ae

Please sign in to comment.