From 7446b97ec958e3ef0fa20a14831c51b259d02097 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Fri, 16 Jun 2023 18:45:35 +0200 Subject: [PATCH 1/5] feat: promise --- src/Formidable.js | 71 +++++++++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/src/Formidable.js b/src/Formidable.js index 3fd29387..0e9a1536 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -178,40 +178,55 @@ class IncomingForm extends EventEmitter { return true; } + // returns a promise if no callback is provided async parse(req, cb) { this.req = req; + let promise; // Setup callback first, so we don't miss anything from data events emitted immediately. - if (cb) { - const callback = once(dezalgo(cb)); - this.fields = {}; - const files = {}; - - this.on('field', (name, value) => { - if (this.type === 'multipart' || this.type === 'urlencoded') { - if (!hasOwnProp(this.fields, name)) { - this.fields[name] = [value]; - } else { - this.fields[name].push(value); - } - } else { - this.fields[name] = value; - } + if (!cb) { + let resolveRef; + let rejectRef; + promise = new Promise((resolve, reject) => { + resolveRef = resolve; + rejectRef = reject; }); - this.on('file', (name, file) => { - if (!hasOwnProp(files, name)) { - files[name] = [file]; + cb = (err, fields, files) => { + if (err) { + rejectRef(err); } else { - files[name].push(file); + resolveRef([fields, files]); } - }); - this.on('error', (err) => { - callback(err, this.fields, files); - }); - this.on('end', () => { - callback(null, this.fields, files); - }); + } } + const callback = once(dezalgo(cb)); + this.fields = {}; + const files = {}; + + this.on('field', (name, value) => { + if (this.type === 'multipart' || this.type === 'urlencoded') { + if (!hasOwnProp(this.fields, name)) { + this.fields[name] = [value]; + } else { + this.fields[name].push(value); + } + } else { + this.fields[name] = value; + } + }); + this.on('file', (name, file) => { + if (!hasOwnProp(files, name)) { + files[name] = [file]; + } else { + files[name].push(file); + } + }); + this.on('error', (err) => { + callback(err, this.fields, files); + }); + this.on('end', () => { + callback(null, this.fields, files); + }); // Parse headers and setup the parser, ready to start listening for data. await this.writeHeaders(req.headers); @@ -240,7 +255,9 @@ class IncomingForm extends EventEmitter { this._parser.end(); } }); - + if (promise) { + return promise; + } return this; } From 2e00a530e9a46d0f2151e1619e56279ffcae7398 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Fri, 16 Jun 2023 18:46:13 +0200 Subject: [PATCH 2/5] doc: change example to use promise --- README.md | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 23f5218a..aa326e84 100644 --- a/README.md +++ b/README.md @@ -100,25 +100,26 @@ Parse an incoming file upload, with the import http from 'node:http'; import formidable, {errors as formidableErrors} from 'formidable'; -const server = http.createServer((req, res) => { +const server = http.createServer(async (req, res) => { if (req.url === '/api/upload' && req.method.toLowerCase() === 'post') { // parse a file upload const form = formidable({}); - - form.parse(req, (err, fields, files) => { - if (err) { + let fields; + let files; + try { + [fields, files] = await form.parse(req); + } catch (err) { // example to check for a very specific error if (err.code === formidableErrors.maxFieldsExceeded) { } + console.error(err); res.writeHead(err.httpCode || 400, { 'Content-Type': 'text/plain' }); res.end(String(err)); return; - } - res.writeHead(200, { 'Content-Type': 'application/json' }); - res.end(JSON.stringify({ fields, files }, null, 2)); - }); - + } + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ fields, files }, null, 2)); return; } From 17c74a33c46dda77435720d90c0c53f7d125161b Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Sat, 17 Jun 2023 09:52:56 +0200 Subject: [PATCH 3/5] test: add test for promise --- test-node/standalone/promise.test.js | 115 +++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 test-node/standalone/promise.test.js diff --git a/test-node/standalone/promise.test.js b/test-node/standalone/promise.test.js new file mode 100644 index 00000000..7026dd99 --- /dev/null +++ b/test-node/standalone/promise.test.js @@ -0,0 +1,115 @@ +import {strictEqual, ok} from 'node:assert'; +import { createServer, request } from 'node:http'; +import formidable, {errors} from '../../src/index.js'; +import test from 'node:test'; + +const PORT = 13539; + +const isPromise = (x) => { + return x && typeof x === `object` && typeof x.then === `function`; +}; + +test('parse returns promise if no callback is provided', (t,done) => { + const server = createServer((req, res) => { + const form = formidable(); + + const promise = form.parse(req); + strictEqual(isPromise(promise), true); + promise.then(([fields, files]) => { + ok(typeof fields === 'object'); + ok(typeof files === 'object'); + res.writeHead(200); + res.end("ok") + }).catch(e => { + done(e) + }) + }); + + server.listen(PORT, () => { + const chosenPort = server.address().port; + const body = `----13068458571765726332503797717\r +Content-Disposition: form-data; name="title"\r +\r +a\r +----13068458571765726332503797717\r +Content-Disposition: form-data; name="multipleFiles"; filename="x.txt"\r +Content-Type: application/x-javascript\r +\r +\r +\r +a\r +b\r +c\r +d\r +\r +----13068458571765726332503797717--\r +`; + fetch(String(new URL(`http:localhost:${chosenPort}/`)), { + method: 'POST', + + headers: { + 'Content-Length': body.length, + Host: `localhost:${chosenPort}`, + 'Content-Type': 'multipart/form-data; boundary=--13068458571765726332503797717', + }, + body + }).then(res => { + strictEqual(res.status, 200); + server.close(); + done(); + }); + + }); +}); + +test('parse rejects with promise if it fails', (t,done) => { + const server = createServer((req, res) => { + const form = formidable({minFileSize: 10 ** 6}); // create condition to fail + + const promise = form.parse(req); + strictEqual(isPromise(promise), true); + promise.then(() => { + done('should have failed') + }).catch(e => { + res.writeHead(e.httpCode); + strictEqual(e.code, errors.smallerThanMinFileSize); + res.end(String(e)) + }) + }); + + server.listen(PORT, () => { + const chosenPort = server.address().port; + const body = `----13068458571765726332503797717\r +Content-Disposition: form-data; name="title"\r +\r +a\r +----13068458571765726332503797717\r +Content-Disposition: form-data; name="multipleFiles"; filename="x.txt"\r +Content-Type: application/x-javascript\r +\r +\r +\r +a\r +b\r +c\r +d\r +\r +----13068458571765726332503797717--\r +`; + fetch(String(new URL(`http:localhost:${chosenPort}/`)), { + method: 'POST', + + headers: { + 'Content-Length': body.length, + Host: `localhost:${chosenPort}`, + 'Content-Type': 'multipart/form-data; boundary=--13068458571765726332503797717', + }, + body + }).then(res => { + strictEqual(res.status, 400); + server.close(); + done(); + }); + + }); +}); From 9a5bb828c4074cb6d5138ba796cdc009e7f397c4 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Sat, 17 Jun 2023 10:03:31 +0200 Subject: [PATCH 4/5] chore: version and changelog --- CHANGELOG.md | 8 +++++++- package.json | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23a84fd5..beb573a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,14 @@ # Changelog +### 3.4.0 + + * feature: ([#940](https://github.com/node-formidable/formidable/pull/940)) form.parse returns a promise if no callback is provided + * it resolves with and array `[fields, files]` + + ### 3.3.2 - * feature: ([#855](https://github.com/node-formidable/formidable/pull/855))add options.createDirsFromUploads, see README for usage + * feature: ([#855](https://github.com/node-formidable/formidable/pull/855)) add options.createDirsFromUploads, see README for usage * form.parse is an async function (ignore the promise) * benchmarks: add e2e becnhmark with as many request as possible per second * npm run to display all the commands diff --git a/package.json b/package.json index 2c1affd8..001a8c56 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "formidable", - "version": "3.3.2", + "version": "3.4.0", "license": "MIT", "description": "A node.js module for parsing form data, especially file uploads.", "homepage": "https://github.com/node-formidable/formidable", From 9429b8faa18e8a06c397dbae1026193e6f5f1e77 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Sat, 17 Jun 2023 10:04:49 +0200 Subject: [PATCH 5/5] doc: add promise --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index aa326e84..94c74e28 100644 --- a/README.md +++ b/README.md @@ -383,10 +383,9 @@ const options = { ``` -### .parse(request, callback) +### .parse(request, ?callback) -Parses an incoming Node.js `request` containing form data. If `callback` is -provided, all fields and files are collected and passed to the callback. +Parses an incoming Node.js `request` containing form data. If `callback` is not provided a promise is returned. ```js const form = formidable({ uploadDir: __dirname }); @@ -395,6 +394,9 @@ form.parse(req, (err, fields, files) => { console.log('fields:', fields); console.log('files:', files); }); + +// with Promise +const [fields, files] = await form.parse(req); ``` You may overwrite this method if you are interested in directly accessing the