From 06a56ae587795113b17fec559ab49c93f40861e8 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 19 Jan 2023 18:13:57 -0500 Subject: [PATCH] feat: add content-length headers to generated responses, via new `text(...)` 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> --- .changeset/nine-walls-listen.md | 5 ++++ .changeset/old-weeks-reflect.md | 5 ++++ packages/kit/src/exports/index.js | 26 ++++++++++++++++++- packages/kit/src/runtime/server/data/index.js | 3 ++- packages/kit/src/runtime/server/page/index.js | 7 ++--- .../kit/src/runtime/server/page/render.js | 3 ++- packages/kit/src/runtime/server/respond.js | 12 ++++----- packages/kit/src/runtime/server/utils.js | 10 +++---- .../routes/content-length-header/+page.svelte | 0 .../kit/test/apps/basics/test/server.test.js | 12 +++++++++ packages/kit/types/index.d.ts | 9 ++++++- 11 files changed, 74 insertions(+), 18 deletions(-) create mode 100644 .changeset/nine-walls-listen.md create mode 100644 .changeset/old-weeks-reflect.md create mode 100644 packages/kit/test/apps/basics/src/routes/content-length-header/+page.svelte diff --git a/.changeset/nine-walls-listen.md b/.changeset/nine-walls-listen.md new file mode 100644 index 000000000000..ff046dacf7e0 --- /dev/null +++ b/.changeset/nine-walls-listen.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: add `Content-Length` header to SvelteKit-generated responses diff --git a/.changeset/old-weeks-reflect.md b/.changeset/old-weeks-reflect.md new file mode 100644 index 000000000000..21df75b53e80 --- /dev/null +++ b/.changeset/old-weeks-reflect.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': minor +--- + +feat: add `text(...)` helper for generating text responses diff --git a/packages/kit/src/exports/index.js b/packages/kit/src/exports/index.js index f5ba4abcf2fd..b14a6e9613b9 100644 --- a/packages/kit/src/exports/index.js +++ b/packages/kit/src/exports/index.js @@ -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 }); diff --git a/packages/kit/src/runtime/server/data/index.js b/packages/kit/src/runtime/server/data/index.js index de4113bdb8d6..8c0faa4ff247 100644 --- a/packages/kit/src/runtime/server/data/index.js +++ b/packages/kit/src/runtime/server/data/index.js @@ -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'; @@ -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', diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js index accae7c94d44..cec1e42ce52b 100644 --- a/packages/kit/src/runtime/server/page/index.js +++ b/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'; @@ -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 }); } @@ -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 }); } @@ -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 }); } diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js index e23e8ee006b0..55bba9a9d3c0 100644 --- a/packages/kit/src/runtime/server/page/render.js +++ b/packages/kit/src/runtime/server/page/render.js @@ -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 @@ -408,7 +409,7 @@ export async function render_response({ } } - return new Response(transformed, { + return text(transformed, { status, headers }); diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index b98a657629dd..07b7eab1d175 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -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__ */ @@ -53,7 +53,7 @@ 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 }); } } @@ -61,7 +61,7 @@ export async function respond(request, options, manifest, state) { 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} */ @@ -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) || '/'; } @@ -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 }); } @@ -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, diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js index d7e31bb30713..49f873e41375 100644 --- a/packages/kit/src/runtime/server/utils.js +++ b/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'; @@ -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 @@ -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 }); @@ -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 }); } diff --git a/packages/kit/test/apps/basics/src/routes/content-length-header/+page.svelte b/packages/kit/test/apps/basics/src/routes/content-length-header/+page.svelte new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/kit/test/apps/basics/test/server.test.js b/packages/kit/test/apps/basics/test/server.test.js index ed773bbc175e..9cbd17d95991 100644 --- a/packages/kit/test/apps/basics/test/server.test.js +++ b/packages/kit/test/apps/basics/test/server.test.js @@ -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) => { diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 1891ec283611..d80c735c5dca 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -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.