From a5cbef7eeb8399b5d0672f95af1349e8a0d16b8c Mon Sep 17 00:00:00 2001 From: Cameron Robey Date: Wed, 17 Aug 2022 19:03:52 +0100 Subject: [PATCH 01/10] add support for multipart/form-data --- lib/fetch/body.js | 33 +++++++++++++++++++++++++++++++-- test/fetch/client-fetch.js | 35 ++++++++++++++++++++++++++--------- 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/lib/fetch/body.js b/lib/fetch/body.js index 08d22310a38..cf4ae42ce03 100644 --- a/lib/fetch/body.js +++ b/lib/fetch/body.js @@ -1,5 +1,6 @@ 'use strict' +const Busboy = require('busboy') const util = require('../core/util') const { ReadableStreamFrom, toUSVString, isBlobLike } = require('./util') const { FormData } = require('./formdata') @@ -8,9 +9,9 @@ const { webidl } = require('./webidl') const { Blob } = require('buffer') const { kBodyUsed } = require('../core/symbols') const assert = require('assert') -const { NotSupportedError } = require('../core/errors') const { isErrored } = require('../core/util') const { isUint8Array, isArrayBuffer } = require('util/types') +const { File } = require('./file') let ReadableStream @@ -397,7 +398,35 @@ function bodyMixinMethods (instance) { // If mimeType’s essence is "multipart/form-data", then: if (/multipart\/form-data/.test(contentType)) { - throw new NotSupportedError('multipart/form-data not supported') + const headers = {} + for (const [key, value] of this.headers) headers[key.toLowerCase()] = value + + const responseFormData = new FormData() + + const busboy = Busboy({ headers }) + busboy.on('field', (name, value) => { + responseFormData.append(name, value) + }) + busboy.on('file', (name, value, info) => { + const { filename, encoding, mimeType } = info + const base64 = encoding.toLowerCase() === 'base64' + const chunks = [] + value.on('data', (chunk) => { + if (base64) chunk = Buffer.from(chunk.toString(), 'base64') + chunks.push(chunk) + }) + value.on('end', () => { + const file = new File(chunks, filename, { type: mimeType }) + responseFormData.append(name, file) + }) + }) + const busboyResolve = new Promise((resolve) => busboy.on('finish', resolve)) + + if (this.body !== null) for await (const chunk of consumeBody(this[kState].body)) busboy.write(chunk) + busboy.end() + await busboyResolve + + return responseFormData } else if (/application\/x-www-form-urlencoded/.test(contentType)) { // Otherwise, if mimeType’s essence is "application/x-www-form-urlencoded", then: diff --git a/test/fetch/client-fetch.js b/test/fetch/client-fetch.js index 4ec4545d544..b9670a0c9a4 100644 --- a/test/fetch/client-fetch.js +++ b/test/fetch/client-fetch.js @@ -165,21 +165,38 @@ test('unsupported formData 1', (t) => { }) }) -test('unsupported formData 2', (t) => { - t.plan(1) +test('multipart formdata', async (t) => { + t.plan(2) + + // Construct example form data, with text and file fields + const formData = new FormData() + formData.append('field1', 'value1') + const blob = new Blob(['example\ntext file'], { type: 'text/plain' }) + formData.append('field2', blob, 'file.txt') + + const tempRes = new Response(formData) + const boundary = tempRes.headers.get('content-type').split('boundary=')[1] + const formRaw = await tempRes.text() const server = createServer((req, res) => { - res.setHeader('content-type', 'multipart/form-data') + res.setHeader('content-type', 'multipart/form-data; boundary=' + boundary) + res.write(formRaw) res.end() }) t.teardown(server.close.bind(server)) - server.listen(0, () => { - fetch(`http://localhost:${server.address().port}`) - .then(res => res.formData()) - .catch(err => { - t.equal(err.name, 'NotSupportedError') - }) + await new Promise((resolve) => { + server.listen(0, () => { + fetch(`http://localhost:${server.address().port}`) + .then(res => res.formData()) + .then(formData => { + t.equal(formData.get('field1'), 'value1') + return formData.get('field2').text() + }).then(text => { + t.equal(text, 'example\ntext file') + resolve() + }) + }) }) }) From 6f9dd5301eeb6b23dcdd138b449b6abdcd4fd1a6 Mon Sep 17 00:00:00 2001 From: Cameron Robey Date: Fri, 19 Aug 2022 12:05:53 +0100 Subject: [PATCH 02/10] Handle busboy errors --- lib/fetch/body.js | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/lib/fetch/body.js b/lib/fetch/body.js index cf4ae42ce03..04764bbb44f 100644 --- a/lib/fetch/body.js +++ b/lib/fetch/body.js @@ -402,29 +402,34 @@ function bodyMixinMethods (instance) { for (const [key, value] of this.headers) headers[key.toLowerCase()] = value const responseFormData = new FormData() - - const busboy = Busboy({ headers }) - busboy.on('field', (name, value) => { - responseFormData.append(name, value) - }) - busboy.on('file', (name, value, info) => { - const { filename, encoding, mimeType } = info - const base64 = encoding.toLowerCase() === 'base64' - const chunks = [] - value.on('data', (chunk) => { - if (base64) chunk = Buffer.from(chunk.toString(), 'base64') - chunks.push(chunk) + + try { + const busboy = Busboy({ headers }) + busboy.on('field', (name, value) => { + responseFormData.append(name, value) }) - value.on('end', () => { - const file = new File(chunks, filename, { type: mimeType }) - responseFormData.append(name, file) + busboy.on('file', (name, value, info) => { + const { filename, encoding, mimeType } = info + const base64 = encoding.toLowerCase() === 'base64' + const chunks = [] + value.on('data', (chunk) => { + if (base64) chunk = Buffer.from(chunk.toString(), 'base64') + chunks.push(chunk) + }) + value.on('end', () => { + const file = new File(chunks, filename, { type: mimeType }) + responseFormData.append(name, file) + }) }) - }) - const busboyResolve = new Promise((resolve) => busboy.on('finish', resolve)) + const busboyResolve = new Promise((resolve) => busboy.on('finish', resolve)) - if (this.body !== null) for await (const chunk of consumeBody(this[kState].body)) busboy.write(chunk) - busboy.end() - await busboyResolve + if (this.body !== null) for await (const chunk of consumeBody(this[kState].body)) busboy.write(chunk) + busboy.end() + await busboyResolve + } catch (err) { + // busboy may fail to parse a malformed body, so throw a type error + throw Object.assign(new TypeError(), { cause: err }) + } return responseFormData } else if (/application\/x-www-form-urlencoded/.test(contentType)) { From ff3a0798123eaea6d468fdce0283121ecb4fb48e Mon Sep 17 00:00:00 2001 From: Cameron Robey Date: Fri, 19 Aug 2022 12:10:23 +0100 Subject: [PATCH 03/10] linting --- lib/fetch/body.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fetch/body.js b/lib/fetch/body.js index 04764bbb44f..562f309b1a2 100644 --- a/lib/fetch/body.js +++ b/lib/fetch/body.js @@ -402,7 +402,7 @@ function bodyMixinMethods (instance) { for (const [key, value] of this.headers) headers[key.toLowerCase()] = value const responseFormData = new FormData() - + try { const busboy = Busboy({ headers }) busboy.on('field', (name, value) => { From 4bdc4289177c8bcc2c8c05b9ddd713bf8dcb4684 Mon Sep 17 00:00:00 2001 From: Cameron Robey Date: Mon, 22 Aug 2022 17:30:30 +0100 Subject: [PATCH 04/10] Catch emitted error --- lib/fetch/body.js | 53 +++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/lib/fetch/body.js b/lib/fetch/body.js index 562f309b1a2..1121d080491 100644 --- a/lib/fetch/body.js +++ b/lib/fetch/body.js @@ -403,33 +403,40 @@ function bodyMixinMethods (instance) { const responseFormData = new FormData() + let busboy + try { - const busboy = Busboy({ headers }) - busboy.on('field', (name, value) => { - responseFormData.append(name, value) + busboy = Busboy({ headers }) + } catch(err) { + // Error due to headers: + throw Object.assign(new TypeError(), { cause: err }) + } + + busboy.on('field', (name, value) => { + responseFormData.append(name, value) + }) + busboy.on('file', (name, value, info) => { + const { filename, encoding, mimeType } = info + const base64 = encoding.toLowerCase() === 'base64' + const chunks = [] + value.on('data', (chunk) => { + if (base64) chunk = Buffer.from(chunk.toString(), 'base64') + chunks.push(chunk) }) - busboy.on('file', (name, value, info) => { - const { filename, encoding, mimeType } = info - const base64 = encoding.toLowerCase() === 'base64' - const chunks = [] - value.on('data', (chunk) => { - if (base64) chunk = Buffer.from(chunk.toString(), 'base64') - chunks.push(chunk) - }) - value.on('end', () => { - const file = new File(chunks, filename, { type: mimeType }) - responseFormData.append(name, file) - }) + value.on('end', () => { + const file = new File(chunks, filename, { type: mimeType }) + responseFormData.append(name, file) }) - const busboyResolve = new Promise((resolve) => busboy.on('finish', resolve)) - - if (this.body !== null) for await (const chunk of consumeBody(this[kState].body)) busboy.write(chunk) - busboy.end() - await busboyResolve - } catch (err) { - // busboy may fail to parse a malformed body, so throw a type error + }) + busboy.on('error', (err) => { throw Object.assign(new TypeError(), { cause: err }) - } + }) + + const busboyResolve = new Promise((resolve) => busboy.on('finish', resolve)) + + if (this.body !== null) for await (const chunk of consumeBody(this[kState].body)) busboy.write(chunk) + busboy.end() + await busboyResolve return responseFormData } else if (/application\/x-www-form-urlencoded/.test(contentType)) { From 746d2883252b7b53e45b18191c79b4b4368625d0 Mon Sep 17 00:00:00 2001 From: Cameron Robey Date: Tue, 23 Aug 2022 17:17:46 +0100 Subject: [PATCH 05/10] reject promise instead of throwing error --- lib/fetch/body.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/fetch/body.js b/lib/fetch/body.js index 1121d080491..6d5f7b0bfe4 100644 --- a/lib/fetch/body.js +++ b/lib/fetch/body.js @@ -407,7 +407,7 @@ function bodyMixinMethods (instance) { try { busboy = Busboy({ headers }) - } catch(err) { + } catch (err) { // Error due to headers: throw Object.assign(new TypeError(), { cause: err }) } @@ -428,11 +428,11 @@ function bodyMixinMethods (instance) { responseFormData.append(name, file) }) }) - busboy.on('error', (err) => { - throw Object.assign(new TypeError(), { cause: err }) + + const busboyResolve = new Promise((resolve, reject) => { + busboy.on('finish', resolve) + busboy.on('error', (err) => reject(err)) }) - - const busboyResolve = new Promise((resolve) => busboy.on('finish', resolve)) if (this.body !== null) for await (const chunk of consumeBody(this[kState].body)) busboy.write(chunk) busboy.end() From 05807676bff0d8e1bc2b150b2ac2ff3b69177d7f Mon Sep 17 00:00:00 2001 From: Cameron Robey Date: Fri, 26 Aug 2022 15:50:39 +0100 Subject: [PATCH 06/10] Add test for base64 encoded multipart/form-data Thanks for the help @mrbbot ! --- test/fetch/client-fetch.js | 52 +++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/test/fetch/client-fetch.js b/test/fetch/client-fetch.js index b9670a0c9a4..00680d5fef8 100644 --- a/test/fetch/client-fetch.js +++ b/test/fetch/client-fetch.js @@ -165,10 +165,10 @@ test('unsupported formData 1', (t) => { }) }) -test('multipart formdata', async (t) => { +test('multipart formdata not base64', async (t) => { t.plan(2) - // Construct example form data, with text and file fields + // Construct example form data, with text and blob fields const formData = new FormData() formData.append('field1', 'value1') const blob = new Blob(['example\ntext file'], { type: 'text/plain' }) @@ -186,16 +186,44 @@ test('multipart formdata', async (t) => { t.teardown(server.close.bind(server)) await new Promise((resolve) => { - server.listen(0, () => { - fetch(`http://localhost:${server.address().port}`) - .then(res => res.formData()) - .then(formData => { - t.equal(formData.get('field1'), 'value1') - return formData.get('field2').text() - }).then(text => { - t.equal(text, 'example\ntext file') - resolve() - }) + server.listen(0, async () => { + const response = await fetch(`http://localhost:${server.address().port}`) + const form = await response.formData() + + // Text field + const field1 = form.get('field1') + t.equal(field1, 'value1') + + // Blob field + const field2 = form.get('field2') + const field2Text = await field2.text() + t.equal(field2Text, 'example\ntext file') + resolve() + }) + }) +}) + +test('multipart formdata base64', async (t) => { + t.plan(1) + + // Example form data with base64 encoding + const formRaw = '------formdata-undici-0.5786922755719377\r\nContent-Disposition: form-data; name="key"; filename="test.txt"\r\nContent-Type: text/plain\r\nContent-Transfer-Encoding: base64\r\n\r\ndmFsdWU=\r\n------formdata-undici-0.5786922755719377--' + const server = createServer((req, res) => { + res.setHeader('content-type', 'multipart/form-data; boundary=----formdata-undici-0.5786922755719377') + res.write(formRaw) + res.end() + }) + t.teardown(server.close.bind(server)) + + await new Promise((resolve) => { + server.listen(0, async () => { + const response = await fetch(`http://localhost:${server.address().port}`) + const form = await response.formData() + + const text = await form.get('key').text() + console.log(text) + t.equal(text, 'value') + resolve() }) }) }) From 0a9785bd7ee28446b551b82b8ab4ba1012bea3c7 Mon Sep 17 00:00:00 2001 From: Cameron Robey Date: Fri, 9 Sep 2022 15:02:41 +0100 Subject: [PATCH 07/10] Move busboy from devDependencies to dependencies --- package.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 0b9d4f713e4..3ce23d0bc31 100644 --- a/package.json +++ b/package.json @@ -68,7 +68,6 @@ "@types/node": "^17.0.29", "abort-controller": "^3.0.0", "atomic-sleep": "^1.0.0", - "busboy": "^1.6.0", "chai": "^4.3.4", "chai-as-promised": "^7.1.1", "chai-iterator": "^3.0.2", @@ -122,5 +121,8 @@ "testMatch": [ "/test/jest/**" ] + }, + "dependencies": { + "busboy": "^1.6.0" } } From 0b4c50587ee1d77582e4702b1fec2af1ef0e9d86 Mon Sep 17 00:00:00 2001 From: Cameron Robey Date: Fri, 9 Sep 2022 15:12:32 +0100 Subject: [PATCH 08/10] Add test for busboy emitting error --- test/fetch/client-fetch.js | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/fetch/client-fetch.js b/test/fetch/client-fetch.js index 00680d5fef8..631f0137232 100644 --- a/test/fetch/client-fetch.js +++ b/test/fetch/client-fetch.js @@ -203,6 +203,37 @@ test('multipart formdata not base64', async (t) => { }) }) +test('busboy emit error', async (t) => { + t.plan(1) + + const formData = new FormData() + formData.append('field1', 'value1') + + const tempRes = new Response(formData) + const formRaw = await tempRes.text() + + const server = createServer((req, res) => { + res.setHeader('content-type', 'multipart/form-data; boundary=wrongboundary') + res.write(formRaw) + res.end() + }) + t.teardown(server.close.bind(server)) + + await new Promise((resolve) => { + server.listen(0, async () => { + const response = await fetch(`http://localhost:${server.address().port}`) + + try { + await response.formData() + } catch (err) { + t.equal(err.message, 'Unexpected end of form') + } + + resolve() + }) + }) +}) + test('multipart formdata base64', async (t) => { t.plan(1) From b079acb0948e32c656f576c7eeadf53db5c5c21b Mon Sep 17 00:00:00 2001 From: Cameron Robey Date: Sat, 10 Sep 2022 00:00:25 +0100 Subject: [PATCH 09/10] Rewrite tests --- test/fetch/client-fetch.js | 85 +++++++++++++++----------------------- 1 file changed, 34 insertions(+), 51 deletions(-) diff --git a/test/fetch/client-fetch.js b/test/fetch/client-fetch.js index 631f0137232..e6c619130da 100644 --- a/test/fetch/client-fetch.js +++ b/test/fetch/client-fetch.js @@ -166,8 +166,6 @@ test('unsupported formData 1', (t) => { }) test('multipart formdata not base64', async (t) => { - t.plan(2) - // Construct example form data, with text and blob fields const formData = new FormData() formData.append('field1', 'value1') @@ -185,76 +183,61 @@ test('multipart formdata not base64', async (t) => { }) t.teardown(server.close.bind(server)) - await new Promise((resolve) => { - server.listen(0, async () => { - const response = await fetch(`http://localhost:${server.address().port}`) - const form = await response.formData() - - // Text field - const field1 = form.get('field1') - t.equal(field1, 'value1') - - // Blob field - const field2 = form.get('field2') - const field2Text = await field2.text() - t.equal(field2Text, 'example\ntext file') - resolve() - }) + server.listen(0, () => { + fetch(`http://localhost:${server.address().port}`) + .then(res => res.formData()) + .then(form => { + const field1 = form.get('field1') + t.equal(field1, 'value1') + const field2 = form.get('field2') + return field2.text() + }) + .then(text => { + t.equal(text, 'example\ntext file') + }) }) }) -test('busboy emit error', async (t) => { +test('multipart formdata base64', (t) => { t.plan(1) - const formData = new FormData() - formData.append('field1', 'value1') - - const tempRes = new Response(formData) - const formRaw = await tempRes.text() - + // Example form data with base64 encoding + const formRaw = '------formdata-undici-0.5786922755719377\r\nContent-Disposition: form-data; name="key"; filename="test.txt"\r\nContent-Type: text/plain\r\nContent-Transfer-Encoding: base64\r\n\r\ndmFsdWU=\r\n------formdata-undici-0.5786922755719377--' const server = createServer((req, res) => { - res.setHeader('content-type', 'multipart/form-data; boundary=wrongboundary') + res.setHeader('content-type', 'multipart/form-data; boundary=----formdata-undici-0.5786922755719377') res.write(formRaw) res.end() }) t.teardown(server.close.bind(server)) - await new Promise((resolve) => { - server.listen(0, async () => { - const response = await fetch(`http://localhost:${server.address().port}`) - - try { - await response.formData() - } catch (err) { - t.equal(err.message, 'Unexpected end of form') - } - - resolve() - }) + server.listen(0, () => { + fetch(`http://localhost:${server.address().port}`) + .then(res => res.formData()) + .then(form => form.get('key').text()) + .then(text => { + t.equal(text, 'value') + }) }) }) -test('multipart formdata base64', async (t) => { - t.plan(1) +test('busboy emit error', async (t) => { + const formData = new FormData() + formData.append('field1', 'value1') + + const tempRes = new Response(formData) + const formRaw = await tempRes.text() - // Example form data with base64 encoding - const formRaw = '------formdata-undici-0.5786922755719377\r\nContent-Disposition: form-data; name="key"; filename="test.txt"\r\nContent-Type: text/plain\r\nContent-Transfer-Encoding: base64\r\n\r\ndmFsdWU=\r\n------formdata-undici-0.5786922755719377--' const server = createServer((req, res) => { - res.setHeader('content-type', 'multipart/form-data; boundary=----formdata-undici-0.5786922755719377') + res.setHeader('content-type', 'multipart/form-data; boundary=wrongboundary') res.write(formRaw) res.end() }) t.teardown(server.close.bind(server)) - await new Promise((resolve) => { - server.listen(0, async () => { - const response = await fetch(`http://localhost:${server.address().port}`) - const form = await response.formData() - - const text = await form.get('key').text() - console.log(text) - t.equal(text, 'value') - resolve() + server.listen(0, async () => { + const res = await fetch(`http://localhost:${server.address().port}`) + res.formData().catch(err => { + t.equal(err.message, 'Unexpected end of form') }) }) }) From 5e9e5a8b55a4103e2332c047249d9825d3d1f41e Mon Sep 17 00:00:00 2001 From: Cameron Robey Date: Fri, 16 Sep 2022 14:17:27 +0100 Subject: [PATCH 10/10] Update tests to avoid promises and callbacks --- test/fetch/client-fetch.js | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/test/fetch/client-fetch.js b/test/fetch/client-fetch.js index e6c619130da..3d588248570 100644 --- a/test/fetch/client-fetch.js +++ b/test/fetch/client-fetch.js @@ -11,6 +11,7 @@ const { Client, setGlobalDispatcher, Agent } = require('../..') const nodeFetch = require('../../index-fetch') const { once } = require('events') const { gzipSync } = require('zlib') +const { promisify } = require('util') setGlobalDispatcher(new Agent({ keepAliveTimeout: 1, @@ -166,6 +167,7 @@ test('unsupported formData 1', (t) => { }) test('multipart formdata not base64', async (t) => { + t.plan(2) // Construct example form data, with text and blob fields const formData = new FormData() formData.append('field1', 'value1') @@ -183,19 +185,15 @@ test('multipart formdata not base64', async (t) => { }) t.teardown(server.close.bind(server)) - server.listen(0, () => { - fetch(`http://localhost:${server.address().port}`) - .then(res => res.formData()) - .then(form => { - const field1 = form.get('field1') - t.equal(field1, 'value1') - const field2 = form.get('field2') - return field2.text() - }) - .then(text => { - t.equal(text, 'example\ntext file') - }) - }) + const listen = promisify(server.listen.bind(server)) + await listen(0) + + const res = await fetch(`http://localhost:${server.address().port}`) + const form = await res.formData() + t.equal(form.get('field1'), 'value1') + + const text = await form.get('field2').text() + t.equal(text, 'example\ntext file') }) test('multipart formdata base64', (t) => { @@ -221,6 +219,7 @@ test('multipart formdata base64', (t) => { }) test('busboy emit error', async (t) => { + t.plan(1) const formData = new FormData() formData.append('field1', 'value1') @@ -234,12 +233,11 @@ test('busboy emit error', async (t) => { }) t.teardown(server.close.bind(server)) - server.listen(0, async () => { - const res = await fetch(`http://localhost:${server.address().port}`) - res.formData().catch(err => { - t.equal(err.message, 'Unexpected end of form') - }) - }) + const listen = promisify(server.listen.bind(server)) + await listen(0) + + const res = await fetch(`http://localhost:${server.address().port}`) + await t.rejects(res.formData(), 'Unexpected end of multipart data') }) test('urlencoded formData', (t) => {