Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stricter body types, etags for binary responses #1382

Merged
merged 9 commits into from
May 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tame-rings-cry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Generate ETags for binary response bodies
6 changes: 6 additions & 0 deletions .changeset/tidy-turkeys-rule.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@sveltejs/adapter-cloudflare-workers': patch
'@sveltejs/adapter-netlify': patch
---

Ensure rawBody is a string or Uint8Array
5 changes: 5 additions & 0 deletions .changeset/wet-bees-exist.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Update request/response body types
4 changes: 2 additions & 2 deletions documentation/docs/01-routing.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

Expand Down
4 changes: 2 additions & 2 deletions documentation/docs/04-hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
5 changes: 3 additions & 2 deletions packages/adapter-cloudflare-workers/files/entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion packages/adapter-netlify/files/entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/src/core/adapt/prerender.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion packages/kit/src/core/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
7 changes: 5 additions & 2 deletions packages/kit/src/core/http/index.js
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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');
Expand Down
3 changes: 2 additions & 1 deletion packages/kit/src/core/start/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
48 changes: 35 additions & 13 deletions packages/kit/src/runtime/server/endpoint.js
Original file line number Diff line number Diff line change
@@ -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();
Expand All @@ -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 };
}
}
}
9 changes: 1 addition & 8 deletions packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
2 changes: 1 addition & 1 deletion packages/kit/src/runtime/server/page/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/runtime/server/page/respond.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
46 changes: 46 additions & 0 deletions packages/kit/test/apps/basics/src/routes/etag/_tests.js
Original file line number Diff line number Diff line change
@@ -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
}
);
}
9 changes: 9 additions & 0 deletions packages/kit/test/apps/basics/src/routes/etag/binary.js
Original file line number Diff line number Diff line change
@@ -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])
};
}
6 changes: 6 additions & 0 deletions packages/kit/test/apps/basics/src/routes/etag/text.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/** @type {import('@sveltejs/kit').RequestHandler} */
export function get() {
return {
body: 'some text'
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
};
}
17 changes: 13 additions & 4 deletions packages/kit/types/endpoint.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
10 changes: 8 additions & 2 deletions packages/kit/types/hooks.d.ts
Original file line number Diff line number Diff line change
@@ -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>;
};
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
4 changes: 2 additions & 2 deletions packages/kit/types/internal.d.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand Down