From 11e7840c67c2cae17940b0d9664ca0fb3da91eb9 Mon Sep 17 00:00:00 2001
From: Rich Harris <rich.harris@hey.com>
Date: Sun, 9 May 2021 12:48:53 -0400
Subject: [PATCH] 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
---
 .changeset/tame-rings-cry.md                  |  5 ++
 .changeset/tidy-turkeys-rule.md               |  6 +++
 .changeset/wet-bees-exist.md                  |  5 ++
 documentation/docs/01-routing.md              |  4 +-
 documentation/docs/04-hooks.md                |  4 +-
 .../adapter-cloudflare-workers/files/entry.js |  5 +-
 packages/adapter-netlify/files/entry.js       |  2 +-
 packages/kit/src/core/adapt/prerender.js      |  4 +-
 packages/kit/src/core/dev/index.js            |  3 +-
 packages/kit/src/core/http/index.js           |  7 ++-
 packages/kit/src/core/start/index.js          |  3 +-
 packages/kit/src/runtime/server/endpoint.js   | 48 ++++++++++++++-----
 packages/kit/src/runtime/server/index.js      |  9 +---
 packages/kit/src/runtime/server/page/index.js |  2 +-
 .../kit/src/runtime/server/page/respond.js    |  2 +-
 .../apps/basics/src/routes/etag/_tests.js     | 46 ++++++++++++++++++
 .../apps/basics/src/routes/etag/binary.js     |  9 ++++
 .../test/apps/basics/src/routes/etag/text.js  |  6 +++
 .../basics/src/routes/load/raw-body.json.js   |  4 +-
 packages/kit/types/endpoint.d.ts              | 17 +++++--
 packages/kit/types/hooks.d.ts                 | 10 +++-
 packages/kit/types/index.d.ts                 |  4 +-
 packages/kit/types/internal.d.ts              |  4 +-
 23 files changed, 161 insertions(+), 48 deletions(-)
 create mode 100644 .changeset/tame-rings-cry.md
 create mode 100644 .changeset/tidy-turkeys-rule.md
 create mode 100644 .changeset/wet-bees-exist.md
 create mode 100644 packages/kit/test/apps/basics/src/routes/etag/_tests.js
 create mode 100644 packages/kit/test/apps/basics/src/routes/etag/binary.js
 create mode 100644 packages/kit/test/apps/basics/src/routes/etag/text.js

diff --git a/.changeset/tame-rings-cry.md b/.changeset/tame-rings-cry.md
new file mode 100644
index 000000000000..57ac1aad2c12
--- /dev/null
+++ 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
new file mode 100644
index 000000000000..cfafc79aabce
--- /dev/null
+++ 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
new file mode 100644
index 000000000000..a96164a3e2f3
--- /dev/null
+++ 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
index 3c3217b412a3..38b7a219461f 100644
--- 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
index d780e0305183..cffd484a788a 100644
--- 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
index e52606ff401f..d5749d7f83cb 100644
--- 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
index ee7fbf4ed692..0b6e5ccae6de 100644
--- 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
index db52c5245478..831cb64b0ae2 100644
--- 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
index f4e4c9204a6a..504f8b919802 100644
--- 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
index fe54bf220db5..21e6de345f7a 100644
--- 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
index fe187cc51457..334455455c10 100644
--- 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
index ea89baf4d32c..4bd9daeac7e1 100644
--- 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
index 2f887e72b0b4..56c6b595f104 100644
--- 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
index 8356f8b96c1d..df62567e1a6d 100644
--- 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
index 40243b56020a..d4b39b0e62bb 100644
--- 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
new file mode 100644
index 000000000000..0aee3ac6d2c7
--- /dev/null
+++ 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
new file mode 100644
index 000000000000..71709c5dda7e
--- /dev/null
+++ 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
new file mode 100644
index 000000000000..e03c17e46b70
--- /dev/null
+++ 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
index 9c874fef4662..bb8c11432caf 100644
--- 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
index 68260e08ed90..3ea8ad462161 100644
--- 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
index 23f776c81904..15a12c47bed9 100644
--- a/packages/kit/types/hooks.d.ts
+++ b/packages/kit/types/hooks.d.ts
@@ -1,5 +1,5 @@
 import { BaseBody, Headers } from './helper';
-import { ServerRequest, ServerResponse } from './endpoint';
+import { ServerRequest } from './endpoint';
 
 export type Incoming = {
 	method: string;
@@ -7,10 +7,16 @@ export type Incoming = {
 	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
index 7726c26d9ba4..832cbdb40fed 100644
--- 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
index 7f5c4c9b53b2..8a778c933e14 100644
--- 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;