Stricter body types, etags for binary responses (#1382)

* incoming rawBody is string or Uint8Array * make body types stricter * add etag tests * generate etags for binary bodies - fixes #1136 * oops * rawBody is a Uint8Array * changesets * use shared hash helper
sveltejs · May 9, 2021 · 11e7840 · 11e7840
1 parent 2562ca0
commit 11e7840
Show file tree

Hide file tree

Showing 23 changed files with 161 additions and 48 deletions.
diff --git a/.changeset/tame-rings-cry.md b/.changeset/tame-rings-cry.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+Generate ETags for binary response bodies
diff --git a/.changeset/tidy-turkeys-rule.md b/.changeset/tidy-turkeys-rule.md
@@ -0,0 +1,6 @@
+---
+'@sveltejs/adapter-cloudflare-workers': patch
+'@sveltejs/adapter-netlify': patch
+---
+
+Ensure rawBody is a string or Uint8Array
diff --git a/.changeset/wet-bees-exist.md b/.changeset/wet-bees-exist.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+Update request/response body types
diff --git a/documentation/docs/01-routing.md b/documentation/docs/01-routing.md
@@ -55,8 +55,8 @@ type Request<Context = any> = {
 	path: string;
 	params: Record<string, string | string[]>;
 	query: URLSearchParams;
-	rawBody: string | ArrayBuffer;
-	body: string | ArrayBuffer | ReadOnlyFormData | any;
+	rawBody: string | Uint8Array;
+	body: string | Uint8Array | JSONValue;
 	locals: Record<string, any>; // see below
 };
 

diff --git a/documentation/docs/04-hooks.md b/documentation/docs/04-hooks.md
@@ -22,8 +22,8 @@ type Request<Locals = Record<string, any>> = {
 	path: string;
 	params: Record<string, string>;
 	query: URLSearchParams;
-	rawBody: string | ArrayBuffer;
-	body: string | ArrayBuffer | ReadOnlyFormData | any;
+	rawBody: string | Uint8Array;
+	body: string | Uint8Array | ReadOnlyFormData | JSONValue;
 	locals: Locals;
 };
 

diff --git a/packages/adapter-cloudflare-workers/files/entry.js b/packages/adapter-cloudflare-workers/files/entry.js
@@ -52,10 +52,11 @@ async function handle(event) {
 	});
 }
 
-function read(request) {
+/** @param {Request} request */
+async function read(request) {
 	const type = request.headers.get('content-type') || '';
 	if (type.includes('application/octet-stream')) {
-		return request.arrayBuffer();
+		return new Uint8Array(await request.arrayBuffer());
 	}
 
 	return request.text();

diff --git a/packages/adapter-netlify/files/entry.js b/packages/adapter-netlify/files/entry.js
@@ -16,7 +16,7 @@ export async function handler(event) {
 
 	const rawBody =
 		headers['content-type'] === 'application/octet-stream'
-			? new TextEncoder('base64').encode(body).buffer
+			? new TextEncoder('base64').encode(body)
 			: isBase64Encoded
 			? Buffer.from(body, 'base64').toString()
 			: body;

diff --git a/packages/kit/src/core/adapt/prerender.js b/packages/kit/src/core/adapt/prerender.js
@@ -98,7 +98,7 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a
 		if (seen.has(path)) return;
 		seen.add(path);
 
-		/** @type {Map<string, import('types/endpoint').ServerResponse>} */
+		/** @type {Map<string, import('types/hooks').ServerResponse>} */
 		const dependencies = new Map();
 
 		const rendered = await app.render(
@@ -172,7 +172,7 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a
 			});
 
 			if (is_html && config.kit.prerender.crawl) {
-				const cleaned = clean_html(rendered.body);
+				const cleaned = clean_html(/** @type {string} */ (rendered.body));
 
 				let match;
 				const pattern = /<(a|img|link|source)\s+([\s\S]+?)>/gm;

diff --git a/packages/kit/src/core/dev/index.js b/packages/kit/src/core/dev/index.js
@@ -296,7 +296,8 @@ class Watcher extends EventEmitter {
 
 					if (rendered) {
 						res.writeHead(rendered.status, rendered.headers);
-						res.end(rendered.body);
+						if (rendered.body) res.write(rendered.body);
+						res.end();
 					} else {
 						res.statusCode = 404;
 						res.end('Not found');

diff --git a/packages/kit/src/core/http/index.js b/packages/kit/src/core/http/index.js
@@ -1,4 +1,7 @@
-/** @param {import('http').IncomingMessage} req */
+/**
+ * @param {import('http').IncomingMessage} req
+ * @returns {Promise<string | Uint8Array>}
+ */
 export function getRawBody(req) {
 	return new Promise((fulfil, reject) => {
 		const h = req.headers;
@@ -45,7 +48,7 @@ export function getRawBody(req) {
 			const [type] = h['content-type'].split(/;\s*/);
 
 			if (type === 'application/octet-stream') {
-				fulfil(data.buffer);
+				fulfil(data);
 			}
 
 			const decoder = new TextDecoder(h['content-encoding'] || 'utf-8');

diff --git a/packages/kit/src/core/start/index.js b/packages/kit/src/core/start/index.js
@@ -65,7 +65,8 @@ export async function start({ port, host, config, https: use_https = false, cwd
 
 				if (rendered) {
 					res.writeHead(rendered.status, rendered.headers);
-					res.end(rendered.body);
+					if (rendered.body) res.write(rendered.body);
+					res.end();
 				} else {
 					res.statusCode = 404;
 					res.end('Not found');

diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js
@@ -1,9 +1,18 @@
 import { lowercase_keys } from './utils.js';
 
+/** @param {string} body */
+function error(body) {
+	return {
+		status: 500,
+		body,
+		headers: {}
+	};
+}
+
 /**
  * @param {import('types/endpoint').ServerRequest} request
  * @param {import('types/internal').SSREndpoint} route
- * @returns {Promise<import('types/endpoint').ServerResponse>}
+ * @returns {Promise<import('types/hooks').ServerResponse>}
  */
 export default async function render_route(request, route) {
 	const mod = await route.load();
@@ -19,27 +28,40 @@ export default async function render_route(request, route) {
 
 		if (response) {
 			if (typeof response !== 'object') {
-				return {
-					status: 500,
-					body: `Invalid response from route ${request.path};
-						 expected an object, got ${typeof response}`,
-					headers: {}
-				};
+				return error(
+					`Invalid response from route ${request.path}: expected an object, got ${typeof response}`
+				);
 			}
 
 			let { status = 200, body, headers = {} } = response;
 
 			headers = lowercase_keys(headers);
+			const type = headers['content-type'];
+
+			// validation
+			if (type === 'application/octet-stream' && !(body instanceof Uint8Array)) {
+				return error(
+					`Invalid response from route ${request.path}: body must be an instance of Uint8Array if content type is application/octet-stream`
+				);
+			}
+
+			if (body instanceof Uint8Array && type !== 'application/octet-stream') {
+				return error(
+					`Invalid response from route ${request.path}: Uint8Array body must be accompanied by content-type: application/octet-stream header`
+				);
+			}
+
+			/** @type {string | Uint8Array} */
+			let normalized_body;
 
-			if (
-				typeof body === 'object' &&
-				(!('content-type' in headers) || headers['content-type'] === 'application/json')
-			) {
+			if (typeof body === 'object' && (!type || type === 'application/json')) {
 				headers = { ...headers, 'content-type': 'application/json' };
-				body = JSON.stringify(body);
+				normalized_body = JSON.stringify(body);
+			} else {
+				normalized_body = /** @type {string | Uint8Array} */ (body);
 			}
 
-			return { status, body, headers };
+			return { status, body: normalized_body, headers };
 		}
 	}
 }
diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js
@@ -3,6 +3,7 @@ import { render_response } from './page/render.js';
 import render_endpoint from './endpoint.js';
 import { parse_body } from './parse_body/index.js';
 import { lowercase_keys } from './utils.js';
+import { hash } from '../hash.js';
 
 /**
  * @param {import('types/hooks').Incoming} incoming
@@ -86,11 +87,3 @@ export async function respond(incoming, options, state = {}) {
 		};
 	}
 }
-
-/** @param {string} str */
-function hash(str) {
-	let hash = 5381,
-		i = str.length;
-	while (i) hash = (hash * 33) ^ str.charCodeAt(--i);
-	return (hash >>> 0).toString(36);
-}
diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js
@@ -6,7 +6,7 @@ import { respond_with_error } from './respond_with_error.js';
  * @param {import('types/internal').SSRPage} route
  * @param {import('types/internal').SSRRenderOptions} options
  * @param {import('types/internal').SSRRenderState} state
- * @returns {Promise<import('types/endpoint').ServerResponse>}
+ * @returns {Promise<import('types/hooks').ServerResponse>}
  */
 export default async function render_page(request, route, options, state) {
 	if (state.initiator === route) {

diff --git a/packages/kit/src/runtime/server/page/respond.js b/packages/kit/src/runtime/server/page/respond.js
@@ -12,7 +12,7 @@ import { respond_with_error } from './respond_with_error.js';
  *   $session: any;
  *   route: import('types/internal').SSRPage;
  * }} opts
- * @returns {Promise<import('types/endpoint').ServerResponse>}
+ * @returns {Promise<import('types/hooks').ServerResponse>}
  */
 export async function respond({ request, options, state, $session, route }) {
 	const match = route.pattern.exec(request.path);

diff --git a/packages/kit/test/apps/basics/src/routes/etag/_tests.js b/packages/kit/test/apps/basics/src/routes/etag/_tests.js
@@ -0,0 +1,46 @@
+import * as assert from 'uvu/assert';
+
+/** @type {import('test').TestMaker} */
+export default function (test) {
+	test(
+		'generates etag/304 for text body',
+		null,
+		async ({ response, fetch }) => {
+			const r1 = await fetch('/etag/text');
+			const etag = r1.headers.get('etag');
+			assert.ok(!!etag);
+
+			const r2 = await fetch('/etag/text', {
+				headers: {
+					'if-none-match': etag
+				}
+			});
+
+			assert.equal(r2.status, 304);
+		},
+		{
+			js: false
+		}
+	);
+
+	test(
+		'generates etag/304 for binary body',
+		null,
+		async ({ fetch }) => {
+			const r1 = await fetch('/etag/binary');
+			const etag = r1.headers.get('etag');
+			assert.ok(!!etag);
+
+			const r2 = await fetch('/etag/binary', {
+				headers: {
+					'if-none-match': etag
+				}
+			});
+
+			assert.equal(r2.status, 304);
+		},
+		{
+			js: false
+		}
+	);
+}
diff --git a/packages/kit/test/apps/basics/src/routes/etag/binary.js b/packages/kit/test/apps/basics/src/routes/etag/binary.js
@@ -0,0 +1,9 @@
+/** @type {import('@sveltejs/kit').RequestHandler} */
+export function get() {
+	return {
+		headers: {
+			'content-type': 'application/octet-stream'
+		},
+		body: new Uint8Array([1, 2, 3, 4, 5])
+	};
+}
diff --git a/packages/kit/test/apps/basics/src/routes/etag/text.js b/packages/kit/test/apps/basics/src/routes/etag/text.js
@@ -0,0 +1,6 @@
+/** @type {import('@sveltejs/kit').RequestHandler} */
+export function get() {
+	return {
+		body: 'some text'
+	};
+}
diff --git a/packages/kit/test/apps/basics/src/routes/load/raw-body.json.js b/packages/kit/test/apps/basics/src/routes/load/raw-body.json.js
@@ -2,8 +2,8 @@
 export function post(request) {
 	return {
 		body: {
-			body: request.body,
-			rawBody: request.rawBody
+			body: /** @type {string} */ (request.body),
+			rawBody: /** @type {string} */ (request.rawBody)
 		}
 	};
 }
diff --git a/packages/kit/types/endpoint.d.ts b/packages/kit/types/endpoint.d.ts
@@ -7,17 +7,26 @@ export type ServerRequest<Locals = Record<string, any>, Body = unknown> = {
 	path: string;
 	params: Record<string, string>;
 	query: URLSearchParams;
-	rawBody: string | ArrayBuffer;
+	rawBody: string | Uint8Array;
 	body: ParameterizedBody<Body>;
 	locals: Locals;
 };
 
-export type ServerResponse = {
+type JSONValue =
+	| string
+	| number
+	| boolean
+	| null
+	| Date
+	| JSONValue[]
+	| { [key: string]: JSONValue };
+
+export type EndpointOutput = {
 	status?: number;
 	headers?: Headers;
-	body?: any;
+	body?: string | Uint8Array | JSONValue;
 };
 
 export type RequestHandler<Locals = Record<string, any>, Body = unknown> = (
 	request: ServerRequest<Locals, Body>
-) => void | ServerResponse | Promise<ServerResponse>;
+) => void | EndpointOutput | Promise<EndpointOutput>;
diff --git a/packages/kit/types/hooks.d.ts b/packages/kit/types/hooks.d.ts
@@ -1,16 +1,22 @@
 import { BaseBody, Headers } from './helper';
-import { ServerRequest, ServerResponse } from './endpoint';
+import { ServerRequest } from './endpoint';
 
 export type Incoming = {
 	method: string;
 	host: string;
 	headers: Headers;
 	path: string;
 	query: URLSearchParams;
-	rawBody: string | ArrayBuffer;
+	rawBody: string | Uint8Array;
 	body?: BaseBody;
 };
 
+export type ServerResponse = {
+	status: number;
+	headers: Headers;
+	body?: string | Uint8Array;
+};
+
 export type GetSession<Locals = Record<string, any>, Session = any> = {
 	(request: ServerRequest<Locals>): Session | Promise<Session>;
 };

diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts
@@ -5,5 +5,5 @@ import './ambient-modules';
 
 export { Adapter, AdapterUtils, Config } from './config';
 export { ErrorLoad, Load, Page } from './page';
-export { Incoming, GetSession, Handle } from './hooks';
-export { ServerRequest as Request, ServerResponse as Response, RequestHandler } from './endpoint';
+export { Incoming, GetSession, Handle, ServerResponse as Response } from './hooks';
+export { ServerRequest as Request, EndpointOutput, RequestHandler } from './endpoint';
diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts
@@ -1,6 +1,6 @@
 import { Load } from './page';
-import { Incoming, GetSession, Handle } from './hooks';
-import { RequestHandler, ServerResponse } from './endpoint';
+import { Incoming, GetSession, Handle, ServerResponse } from './hooks';
+import { RequestHandler } from './endpoint';
 
 type PageId = string;