[chore] Make rawBody be the bytes (#2215)

* change type of rawBody to Uint8Array * update raw body test * update `parse_body` * remove debug log from taw body test * update `getRawBody` * update adapter-cloudflare-workers * update adapter-netlify * ensure `rawBody` is `Uint8Array` * add changeset * update types inside docs * make isContentType textual private * ensure `rawBody` of `Incoming` is bytes * remove unnecessary rawBody check * make `null` assignable to `RawBody` * use `Headers` type in `parse_body`
sveltejs · Aug 17, 2021 · 290a4ca · 290a4ca
1 parent 9c8f074
commit 290a4ca
Show file tree

Hide file tree

Showing 17 changed files with 58 additions and 71 deletions.
diff --git a/.changeset/short-beds-punch.md b/.changeset/short-beds-punch.md
@@ -0,0 +1,7 @@
+---
+'@sveltejs/adapter-cloudflare-workers': patch
+'@sveltejs/adapter-netlify': patch
+'@sveltejs/kit': patch
+---
+
+Ensure the raw body is an Uint8Array before passing it to request handlers
diff --git a/documentation/docs/01-routing.md b/documentation/docs/01-routing.md
@@ -60,7 +60,7 @@ type Request<Locals = Record<string, any>, Body = unknown> = {
 	path: string;
 	params: Record<string, string>;
 	query: URLSearchParams;
-	rawBody: string | Uint8Array;
+	rawBody: Uint8Array;
 	body: ParameterizedBody<Body>;
 	locals: Locals; // populated by hooks handle
 };

diff --git a/documentation/docs/04-hooks.md b/documentation/docs/04-hooks.md
@@ -24,7 +24,7 @@ type Request<Locals = Record<string, any>> = {
 	path: string;
 	params: Record<string, string>;
 	query: URLSearchParams;
-	rawBody: string | Uint8Array;
+	rawBody: Uint8Array;
 	body: ParameterizedBody<Body>;
 	locals: Locals; // populated by hooks handle
 };

diff --git a/packages/adapter-cloudflare-workers/files/entry.js b/packages/adapter-cloudflare-workers/files/entry.js
@@ -1,7 +1,6 @@
 // TODO hardcoding the relative location makes this brittle
 import { init, render } from '../output/server/app.js'; // eslint-disable-line import/no-unresolved
 import { getAssetFromKV, NotFoundError } from '@cloudflare/kv-asset-handler'; // eslint-disable-line import/no-unresolved
-import { isContentTypeTextual } from '@sveltejs/kit/adapter-utils'; // eslint-disable-line import/no-unresolved
 
 init();
 
@@ -34,7 +33,7 @@ async function handle(event) {
 			host: request_url.host,
 			path: request_url.pathname,
 			query: request_url.searchParams,
-			rawBody: request.body ? await read(request) : null,
+			rawBody: await read(request),
 			headers: Object.fromEntries(request.headers),
 			method: request.method
 		});
@@ -57,10 +56,5 @@ async function handle(event) {
 
 /** @param {Request} request */
 async function read(request) {
-	const type = request.headers.get('content-type') || '';
-	if (isContentTypeTextual(type)) {
-		return request.text();
-	}
-
 	return new Uint8Array(await request.arrayBuffer());
 }
diff --git a/packages/adapter-netlify/files/entry.js b/packages/adapter-netlify/files/entry.js
@@ -1,6 +1,5 @@
 // TODO hardcoding the relative location makes this brittle
 import { init, render } from '../output/server/app.js'; // eslint-disable-line import/no-unresolved
-import { isContentTypeTextual } from '@sveltejs/kit/adapter-utils'; // eslint-disable-line import/no-unresolved
 
 init();
 
@@ -9,13 +8,8 @@ export async function handler(event) {
 
 	const query = new URLSearchParams(rawQuery);
 
-	const type = headers['content-type'];
-	const rawBody =
-		type && isContentTypeTextual(type)
-			? isBase64Encoded
-				? Buffer.from(body, 'base64').toString()
-				: body
-			: new TextEncoder('base64').encode(body);
+	const encoding = isBase64Encoded ? 'base64' : headers['content-encoding'] || 'utf-8';
+	const rawBody = typeof body === 'string' ? Buffer.from(body, encoding) : body;
 
 	const rendered = await render({
 		method: httpMethod,

diff --git a/packages/kit/package.json b/packages/kit/package.json
@@ -77,9 +77,6 @@
 		"./install-fetch": {
 			"import": "./dist/install-fetch.js"
 		},
-		"./adapter-utils": {
-			"import": "./dist/adapter-utils.js"
-		},
 		"./types": "./types/index.d.ts"
 	},
 	"types": "types/index.d.ts",

diff --git a/packages/kit/rollup.config.js b/packages/kit/rollup.config.js
@@ -49,8 +49,7 @@ export default [
 			ssr: 'src/runtime/server/index.js',
 			node: 'src/core/node/index.js',
 			hooks: 'src/runtime/hooks.js',
-			'install-fetch': 'src/install-fetch.js',
-			'adapter-utils': 'src/core/adapter-utils.js'
+			'install-fetch': 'src/install-fetch.js'
 		},
 		output: {
 			dir: 'dist',

diff --git a/packages/kit/src/core/adapt/prerender.js b/packages/kit/src/core/adapt/prerender.js
@@ -156,7 +156,7 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a
 				method: 'GET',
 				headers: {},
 				path,
-				rawBody: '',
+				rawBody: null,
 				query: new URLSearchParams()
 			},
 			{
@@ -289,7 +289,7 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a
 				method: 'GET',
 				headers: {},
 				path: '[fallback]', // this doesn't matter, but it's easiest if it's a string
-				rawBody: '',
+				rawBody: null,
 				query: new URLSearchParams()
 			},
 			{

diff --git a/packages/kit/src/core/adapter-utils.js b/packages/kit/src/core/adapter-utils.js
diff --git a/packages/kit/src/core/node/index.js b/packages/kit/src/core/node/index.js
@@ -1,15 +1,13 @@
-import { isContentTypeTextual } from '../adapter-utils.js';
-
 /**
  * @param {import('http').IncomingMessage} req
- * @returns {Promise<import('types/hooks').StrictBody>}
+ * @returns {Promise<import('types/hooks').RawBody>}
  */
 export function getRawBody(req) {
 	return new Promise((fulfil, reject) => {
 		const h = req.headers;
 
 		if (!h['content-type']) {
-			return fulfil('');
+			return fulfil(null);
 		}
 
 		req.on('error', reject);
@@ -18,7 +16,7 @@ export function getRawBody(req) {
 
 		// https://github.com/jshttp/type-is/blob/c1f4388c71c8a01f79934e68f630ca4a15fffcd6/index.js#L81-L95
 		if (isNaN(length) && h['transfer-encoding'] == null) {
-			return fulfil('');
+			return fulfil(null);
 		}
 
 		let data = new Uint8Array(length || 0);
@@ -48,13 +46,6 @@ export function getRawBody(req) {
 		}
 
 		req.on('end', () => {
-			const [type] = (h['content-type'] || '').split(/;\s*/);
-
-			if (isContentTypeTextual(type)) {
-				const encoding = h['content-encoding'] || 'utf-8';
-				return fulfil(new TextDecoder(encoding).decode(data));
-			}
-
 			fulfil(data);
 		});
 	});

diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js
@@ -1,4 +1,3 @@
-import { isContentTypeTextual } from '../../core/adapter-utils.js';
 import { lowercase_keys } from './utils.js';
 
 /** @param {string} body */
@@ -15,6 +14,23 @@ function is_string(s) {
 	return typeof s === 'string' || s instanceof String;
 }
 
+/**
+ * Decides how the body should be parsed based on its mime type. Should match what's in parse_body
+ *
+ * @param {string | undefined | null} content_type The `content-type` header of a request/response.
+ * @returns {boolean}
+ */
+function is_content_type_textual(content_type) {
+	if (!content_type) return true; // defaults to json
+	const [type] = content_type.split(';'); // get the mime type
+	return (
+		type === 'text/plain' ||
+		type === 'application/json' ||
+		type === 'application/x-www-form-urlencoded' ||
+		type === 'multipart/form-data'
+	);
+}
+
 /**
  * @param {import('types/hooks').ServerRequest} request
  * @param {import('types/internal').SSREndpoint} route
@@ -48,7 +64,7 @@ export async function render_endpoint(request, route, match) {
 	headers = lowercase_keys(headers);
 	const type = headers['content-type'];
 
-	const is_type_textual = isContentTypeTextual(type);
+	const is_type_textual = is_content_type_textual(type);
 
 	if (!is_type_textual && !(body instanceof Uint8Array || is_string(body))) {
 		return error(

diff --git a/packages/kit/src/runtime/server/page/load_node.js b/packages/kit/src/runtime/server/page/load_node.js
@@ -149,7 +149,7 @@ export async function load_node({
 							method: opts.method || 'GET',
 							headers,
 							path: relative,
-							rawBody: /** @type {string} */ (opts.body),
+							rawBody: new TextEncoder().encode(/** @type {string} */ (opts.body)),
 							query: new URLSearchParams(search)
 						},
 						options,

diff --git a/packages/kit/src/runtime/server/parse_body/index.js b/packages/kit/src/runtime/server/parse_body/index.js
@@ -1,31 +1,34 @@
 import { read_only_form_data } from './read_only_form_data.js';
 
 /**
- * @param {import('types/hooks').StrictBody} raw
+ * @param {import('types/hooks.js').RawBody} raw
  * @param {import('types/helper').Headers} headers
  */
 export function parse_body(raw, headers) {
-	if (!raw || typeof raw !== 'string') return raw;
+	if (!raw) return raw;
 
-	const [type, ...directives] = headers['content-type'].split(/;\s*/);
+	const content_type = headers['content-type'];
+	const [type, ...directives] = content_type ? content_type.split(/;\s*/) : [];
+
+	const text = () => new TextDecoder(headers['content-encoding'] || 'utf-8').decode(raw);
 
 	switch (type) {
 		case 'text/plain':
-			return raw;
+			return text();
 
 		case 'application/json':
-			return JSON.parse(raw);
+			return JSON.parse(text());
 
 		case 'application/x-www-form-urlencoded':
-			return get_urlencoded(raw);
+			return get_urlencoded(text());
 
 		case 'multipart/form-data': {
 			const boundary = directives.find((directive) => directive.startsWith('boundary='));
 			if (!boundary) throw new Error('Missing boundary');
-			return get_multipart(raw, boundary.slice('boundary='.length));
+			return get_multipart(text(), boundary.slice('boundary='.length));
 		}
 		default:
-			throw new Error(`Invalid Content-Type ${type}`);
+			return raw;
 	}
 }
 

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
@@ -3,7 +3,7 @@ export function post(request) {
 	return {
 		body: {
 			body: /** @type {string} */ (request.body),
-			rawBody: /** @type {string} */ (request.rawBody)
+			rawBody: new TextDecoder().decode(/** @type {Uint8Array} */ (request.rawBody))
 		}
 	};
 }
diff --git a/packages/kit/types/helper.d.ts b/packages/kit/types/helper.d.ts
@@ -1,3 +1,5 @@
+import { RawBody } from './hooks';
+
 interface ReadOnlyFormData {
 	get(key: string): string;
 	getAll(key: string): string[];
@@ -8,7 +10,7 @@ interface ReadOnlyFormData {
 	[Symbol.iterator](): Generator<[string, string], void>;
 }
 
-type BaseBody = string | Uint8Array | ReadOnlyFormData;
+type BaseBody = string | RawBody | ReadOnlyFormData;
 export type ParameterizedBody<Body = unknown> = Body extends FormData
 	? ReadOnlyFormData
 	: BaseBody & Body;

diff --git a/packages/kit/types/hooks.d.ts b/packages/kit/types/hooks.d.ts
@@ -2,10 +2,12 @@ import { Headers, Location, MaybePromise, ParameterizedBody } from './helper';
 
 export type StrictBody = string | Uint8Array;
 
+export type RawBody = null | Uint8Array;
+
 export interface ServerRequest<Locals = Record<string, any>, Body = unknown> extends Location {
 	method: string;
 	headers: Headers;
-	rawBody: StrictBody;
+	rawBody: RawBody;
 	body: ParameterizedBody<Body>;
 	locals: Locals;
 }

diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts
@@ -4,10 +4,10 @@ import {
 	GetSession,
 	Handle,
 	HandleError,
+	RawBody,
 	ServerFetch,
 	ServerRequest,
-	ServerResponse,
-	StrictBody
+	ServerResponse
 } from './hooks';
 import { Load } from './page';
 
@@ -16,7 +16,7 @@ type PageId = string;
 export interface Incoming extends Omit<Location, 'params'> {
 	method: string;
 	headers: Headers;
-	rawBody: StrictBody;
+	rawBody: RawBody;
 	body?: ParameterizedBody;
 }