From 0ecac665383e85a38fa0c19be2f29a79be1e41af Mon Sep 17 00:00:00 2001 From: Bruno Nascimento Date: Sat, 7 Oct 2023 23:02:36 -0300 Subject: [PATCH 1/3] fix: unhandled error thrown by busboy As per, https://fetch.spec.whatwg.org/#dom-body-formdata, TypeErrors are expected to be thrown. Busboy errors were being unhandled. --- packages/core/src/standards/http.ts | 80 +++++++++++++---------- packages/core/test/standards/http.spec.ts | 23 ++++++- 2 files changed, 67 insertions(+), 36 deletions(-) diff --git a/packages/core/src/standards/http.ts b/packages/core/src/standards/http.ts index a4b921743..cd5389a30 100644 --- a/packages/core/src/standards/http.ts +++ b/packages/core/src/standards/http.ts @@ -311,41 +311,51 @@ export class Body { ); } const formData = new FormData(); - await new Promise(async (resolve) => { - const Busboy: typeof import("busboy") = require("busboy"); - const busboy = Busboy({ - headers: headers as http.IncomingHttpHeaders, - preservePath: true, - }); - busboy.on("field", (name, value) => { - formData.append(name, value); - }); - busboy.on("file", (name, value, info) => { - const { filename, encoding, mimeType } = info; - const base64 = encoding.toLowerCase() === "base64"; - const chunks: Buffer[] = []; - let totalLength = 0; - value.on("data", (chunk: Buffer) => { - if (base64) chunk = Buffer.from(chunk.toString(), "base64"); - chunks.push(chunk); - totalLength += chunk.byteLength; - }); - value.on("end", () => { - if (this[kFormDataFiles]) { - const file = new File(chunks, filename, { type: mimeType }); - formData.append(name, file); - } else { - const text = Buffer.concat(chunks, totalLength).toString(); - formData.append(name, text); - } - }); - }); - busboy.on("finish", resolve); - - const body = this[_kInner].body; - if (body !== null) for await (const chunk of body) busboy.write(chunk); - busboy.end(); - }); + const busboyPromiseResult = await new Promise( + async (resolve, reject) => { + try { + const Busboy: typeof import("busboy") = require("busboy"); + const busboy = Busboy({ + headers: headers as http.IncomingHttpHeaders, + preservePath: true, + }); + busboy.on("field", (name, value) => { + formData.append(name, value); + }); + busboy.on("file", (name, value, info) => { + const { filename, encoding, mimeType } = info; + const base64 = encoding.toLowerCase() === "base64"; + const chunks: Buffer[] = []; + let totalLength = 0; + value.on("data", (chunk: Buffer) => { + if (base64) chunk = Buffer.from(chunk.toString(), "base64"); + chunks.push(chunk); + totalLength += chunk.byteLength; + }); + value.on("end", () => { + if (this[kFormDataFiles]) { + const file = new File(chunks, filename, { type: mimeType }); + formData.append(name, file); + } else { + const text = Buffer.concat(chunks, totalLength).toString(); + formData.append(name, text); + } + }); + }); + busboy.on("finish", resolve); + + const body = this[_kInner].body; + if (body !== null) + for await (const chunk of body) busboy.write(chunk); + busboy.end(); + } catch (e) { + reject(e); + } + } + ); + if (busboyPromiseResult instanceof Error) { + throw new TypeError(busboyPromiseResult.message); + } if (this[kInputGated]) await waitForOpenInputGate(); return formData; } diff --git a/packages/core/test/standards/http.spec.ts b/packages/core/test/standards/http.spec.ts index e41f057ef..e2be6735e 100644 --- a/packages/core/test/standards/http.spec.ts +++ b/packages/core/test/standards/http.spec.ts @@ -382,7 +382,28 @@ test("Body: formData: respects Content-Transfer-Encoding header for base64 encod const formData = await body.formData(); t.is(formData.get("key"), "test"); }); - +test("Body: formData: throw error on missing boundary in Content-Type header", async (t) => { + // Check with multipart/form-data Content-Type + let body = new Body( + new BaseResponse(["second value2", "--boundary--"].join("\r\n"), { + headers: { "content-type": "multipart/form-data" }, + }) + ); + let formData = await body.formData().catch((e) => e); + t.is(formData instanceof Error, true); + t.is(formData.message, "Multipart: Boundary not found"); +}); +test("Body: formData: throw error on unsupported Content-Type header", async (t) => { + // Check with multipart/form-data Content-Type + let body = new Body( + new BaseResponse(["second value2", "--boundary--"].join("\r\n"), { + headers: { "content-type": "application/json" }, + }) + ); + let formData = await body.formData().catch((e) => e); + t.is(formData instanceof Error, true); + t.is(formData.message, "Unsupported content type: application/json"); +}); test("Request: constructing from BaseRequest doesn't create new BaseRequest unless required", (t) => { // Check properties of Request are same as BaseRequest if not RequestInit passed const controller = new AbortController(); From c450b3799801575d7dfd9913bb6ac632e9d26909 Mon Sep 17 00:00:00 2001 From: Bruno Nascimento Date: Tue, 10 Oct 2023 11:04:45 -0300 Subject: [PATCH 2/3] chore: apply suggestions from mrbbot's code review Co-authored-by: MrBBot --- packages/core/src/standards/http.ts | 78 +++++++++++------------ packages/core/test/standards/http.spec.ts | 21 +++--- 2 files changed, 48 insertions(+), 51 deletions(-) diff --git a/packages/core/src/standards/http.ts b/packages/core/src/standards/http.ts index cd5389a30..6c14c500f 100644 --- a/packages/core/src/standards/http.ts +++ b/packages/core/src/standards/http.ts @@ -311,51 +311,45 @@ export class Body { ); } const formData = new FormData(); - const busboyPromiseResult = await new Promise( - async (resolve, reject) => { - try { - const Busboy: typeof import("busboy") = require("busboy"); - const busboy = Busboy({ - headers: headers as http.IncomingHttpHeaders, - preservePath: true, + await new Promise(async (resolve, reject) => { + try { + const Busboy: typeof import("busboy") = require("busboy"); + const busboy = Busboy({ + headers: headers as http.IncomingHttpHeaders, + preservePath: true, + }); + busboy.on("field", (name, value) => { + formData.append(name, value); + }); + busboy.on("file", (name, value, info) => { + const { filename, encoding, mimeType } = info; + const base64 = encoding.toLowerCase() === "base64"; + const chunks: Buffer[] = []; + let totalLength = 0; + value.on("data", (chunk: Buffer) => { + if (base64) chunk = Buffer.from(chunk.toString(), "base64"); + chunks.push(chunk); + totalLength += chunk.byteLength; }); - busboy.on("field", (name, value) => { - formData.append(name, value); + value.on("end", () => { + if (this[kFormDataFiles]) { + const file = new File(chunks, filename, { type: mimeType }); + formData.append(name, file); + } else { + const text = Buffer.concat(chunks, totalLength).toString(); + formData.append(name, text); + } }); - busboy.on("file", (name, value, info) => { - const { filename, encoding, mimeType } = info; - const base64 = encoding.toLowerCase() === "base64"; - const chunks: Buffer[] = []; - let totalLength = 0; - value.on("data", (chunk: Buffer) => { - if (base64) chunk = Buffer.from(chunk.toString(), "base64"); - chunks.push(chunk); - totalLength += chunk.byteLength; - }); - value.on("end", () => { - if (this[kFormDataFiles]) { - const file = new File(chunks, filename, { type: mimeType }); - formData.append(name, file); - } else { - const text = Buffer.concat(chunks, totalLength).toString(); - formData.append(name, text); - } - }); - }); - busboy.on("finish", resolve); - - const body = this[_kInner].body; - if (body !== null) - for await (const chunk of body) busboy.write(chunk); - busboy.end(); - } catch (e) { - reject(e); - } + }); + busboy.on("finish", resolve); + + const body = this[_kInner].body; + if (body !== null) for await (const chunk of body) busboy.write(chunk); + busboy.end(); + } catch (e) { + reject(new TypeError(e instanceof Error ? e.message : String(e))); } - ); - if (busboyPromiseResult instanceof Error) { - throw new TypeError(busboyPromiseResult.message); - } + }); if (this[kInputGated]) await waitForOpenInputGate(); return formData; } diff --git a/packages/core/test/standards/http.spec.ts b/packages/core/test/standards/http.spec.ts index e2be6735e..7075076d2 100644 --- a/packages/core/test/standards/http.spec.ts +++ b/packages/core/test/standards/http.spec.ts @@ -384,25 +384,28 @@ test("Body: formData: respects Content-Transfer-Encoding header for base64 encod }); test("Body: formData: throw error on missing boundary in Content-Type header", async (t) => { // Check with multipart/form-data Content-Type - let body = new Body( + const body = new Body( new BaseResponse(["second value2", "--boundary--"].join("\r\n"), { headers: { "content-type": "multipart/form-data" }, }) ); - let formData = await body.formData().catch((e) => e); - t.is(formData instanceof Error, true); - t.is(formData.message, "Multipart: Boundary not found"); + await t.throwsAsync(body.formData(), { + instanceOf: TypeError, + message: "Multipart: Boundary not found", + }); }); test("Body: formData: throw error on unsupported Content-Type header", async (t) => { - // Check with multipart/form-data Content-Type - let body = new Body( + // Check with application/json Content-Type + const body = new Body( new BaseResponse(["second value2", "--boundary--"].join("\r\n"), { headers: { "content-type": "application/json" }, }) ); - let formData = await body.formData().catch((e) => e); - t.is(formData instanceof Error, true); - t.is(formData.message, "Unsupported content type: application/json"); + await t.throwsAsync(body.formData()), + { + instanceOf: TypeError, + message: "Unsupported content type: application/json", + }; }); test("Request: constructing from BaseRequest doesn't create new BaseRequest unless required", (t) => { // Check properties of Request are same as BaseRequest if not RequestInit passed From b84e8d141b4da8724de775518d6903e4b50b9116 Mon Sep 17 00:00:00 2001 From: Bruno Marques Date: Tue, 24 Oct 2023 20:59:27 -0300 Subject: [PATCH 3/3] fix: test for formData unsupported Content-Type header The expectations weren't included in the `throwAsync()`. --- packages/core/test/standards/http.spec.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/core/test/standards/http.spec.ts b/packages/core/test/standards/http.spec.ts index 7075076d2..861b43598 100644 --- a/packages/core/test/standards/http.spec.ts +++ b/packages/core/test/standards/http.spec.ts @@ -401,11 +401,10 @@ test("Body: formData: throw error on unsupported Content-Type header", async (t) headers: { "content-type": "application/json" }, }) ); - await t.throwsAsync(body.formData()), - { - instanceOf: TypeError, - message: "Unsupported content type: application/json", - }; + await t.throwsAsync(body.formData(), { + instanceOf: TypeError, + message: "Unsupported content type: application/json", + }); }); test("Request: constructing from BaseRequest doesn't create new BaseRequest unless required", (t) => { // Check properties of Request are same as BaseRequest if not RequestInit passed