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

fix: improve getRawBody parsing & handle error(s) #1528

Merged
merged 1 commit into from
May 28, 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
7 changes: 7 additions & 0 deletions .changeset/odd-ligers-swim.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@sveltejs/adapter-node': patch
'@sveltejs/adapter-vercel': patch
'@sveltejs/kit': patch
---

ensure `content-length` limit respected; handle `getRawBody` error(s)
14 changes: 12 additions & 2 deletions packages/adapter-node/src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,22 @@ export function createServer({ render }) {
prerendered_handler,
async (req, res) => {
const parsed = new URL(req.url || '', 'http://localhost');

let body;

try {
body = await getRawBody(req);
} catch (err) {
res.statusCode = err.status || 400;
return res.end(err.reason || 'Invalid request body');
}

const rendered = await render({
method: req.method,
headers: req.headers, // TODO: what about repeated headers, i.e. string[]
path: parsed.pathname,
rawBody: await getRawBody(req),
query: parsed.searchParams
query: parsed.searchParams,
rawBody: body
});

if (rendered) {
Expand Down
11 changes: 10 additions & 1 deletion packages/adapter-vercel/files/entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,21 @@ import { render } from '../output/server/app.js'; // eslint-disable-line import/
export default async (req, res) => {
const { pathname, searchParams } = new URL(req.url || '', 'http://localhost');

let body;

try {
body = await getRawBody(req);
} catch (err) {
res.statusCode = err.status || 400;
return res.end(err.reason || 'Invalid request body');
}

const rendered = await render({
method: req.method,
headers: req.headers,
path: pathname,
query: searchParams,
rawBody: await getRawBody(req)
rawBody: body
});

if (rendered) {
Expand Down
11 changes: 9 additions & 2 deletions packages/kit/src/core/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,14 @@ class Watcher extends EventEmitter {
const root = (await this.vite.ssrLoadModule(`/${this.dir}/generated/root.svelte`))
.default;

const rawBody = await getRawBody(req);
let body;

try {
body = await getRawBody(req);
} catch (err) {
res.statusCode = err.status || 400;
return res.end(err.reason || 'Invalid request body');
}

const host = /** @type {string} */ (this.config.kit.host ||
req.headers[this.config.kit.hostHeader || 'host']);
Expand All @@ -165,7 +172,7 @@ class Watcher extends EventEmitter {
host,
path: parsed.pathname,
query: new URLSearchParams(parsed.query),
rawBody
rawBody: body
},
{
amp: this.config.kit.amp,
Expand Down
47 changes: 24 additions & 23 deletions packages/kit/src/core/node/index.js
Original file line number Diff line number Diff line change
@@ -1,44 +1,45 @@
/**
* @param {import('http').IncomingMessage} req
* @returns {Promise<import('types/hooks').StrictBody>}
* @returns {Promise<import('types/hooks').StrictBody | null>}
*/
export function getRawBody(req) {
return new Promise((fulfil, reject) => {
const h = req.headers;

if (!h['content-type']) {
fulfil(null);
return;
return fulfil(null);
}

req.on('error', reject);

const length = Number(h['content-length']);

/** @type {Uint8Array} */
let data;

if (!isNaN(length)) {
data = new Uint8Array(length);
// https://github.com/jshttp/type-is/blob/c1f4388c71c8a01f79934e68f630ca4a15fffcd6/index.js#L81-L95
if (isNaN(length) && h['transfer-encoding'] == null) {
return fulfil(null);
}

let i = 0;
let data = new Uint8Array(length || 0);

if (length > 0) {
let offset = 0;
req.on('data', (chunk) => {
data.set(chunk, i);
i += chunk.length;
});
} else {
// https://github.com/jshttp/type-is/blob/c1f4388c71c8a01f79934e68f630ca4a15fffcd6/index.js#L81-L95
if (h['transfer-encoding'] === undefined) {
fulfil(null);
return;
}
const new_len = offset + Buffer.byteLength(chunk);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the chunk.length was a bug. Incoming chunks can be strings, but even if they're still Buffer instances, length is not always the same as byteLength, which is what content-length tracks


data = new Uint8Array(0);
if (new_len > length) {
return reject({
status: 413,
reason: 'Exceeded "Content-Length" limit'
});
}

data.set(chunk, offset);
offset = new_len;
});
} else {
req.on('data', (chunk) => {
const new_data = new Uint8Array(data.length + chunk.length);
new_data.set(data);
new_data.set(data, 0);
new_data.set(chunk, data.length);
data = new_data;
});
Expand All @@ -48,11 +49,11 @@ export function getRawBody(req) {
const [type] = h['content-type'].split(/;\s*/);

if (type === 'application/octet-stream') {
fulfil(data);
return fulfil(data);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this return, then decoding continued (below)

}

const decoder = new TextDecoder(h['content-encoding'] || 'utf-8');
fulfil(decoder.decode(data));
const encoding = h['content-encoding'] || 'utf-8';
fulfil(new TextDecoder(encoding).decode(data));
});
});
}
13 changes: 11 additions & 2 deletions packages/kit/src/core/start/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,23 @@ export async function start({ port, host, config, https: use_https = false, cwd

assets_handler(req, res, () => {
static_handler(req, res, async () => {
let body;

try {
body = await getRawBody(req);
} catch (err) {
res.statusCode = err.status || 400;
return res.end(err.reason || 'Invalid request body');
}

const rendered = await app.render({
host: /** @type {string} */ (config.kit.host ||
req.headers[config.kit.hostHeader || 'host']),
method: req.method,
headers: /** @type {import('types/helper').Headers} */ (req.headers),
path: decodeURIComponent(parsed.pathname),
rawBody: await getRawBody(req),
query: new URLSearchParams(parsed.query || '')
query: new URLSearchParams(parsed.query || ''),
rawBody: body
});

if (rendered) {
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/types/hooks.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ export type StrictBody = string | Uint8Array;
export type Incoming = Omit<Location, 'params'> & {
method: string;
headers: Headers;
rawBody: StrictBody;
rawBody: StrictBody | null;
body?: ParameterizedBody;
};

export type ServerRequest<Locals = Record<string, any>, Body = unknown> = Location & {
method: string;
headers: Headers;
rawBody: StrictBody;
rawBody: StrictBody | null;
body: ParameterizedBody<Body>;
locals: Locals;
};
Expand Down