From f3b5bfa2d9609b9d73ea5d8887715133c2946617 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Tue, 21 Dec 2021 21:47:14 +0100 Subject: [PATCH 01/11] refactor: remove unused --- src/Formidable.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Formidable.js b/src/Formidable.js index 63523c2f..5c67b3f2 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -144,7 +144,6 @@ class IncomingForm extends EventEmitter { if (cb) { const callback = once(dezalgo(cb)); this.fields = {}; - let mockFields = ''; const files = {}; this.on('field', (name, value) => { From a25883e151be1912575e72b80f2892c065c737ea Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Tue, 21 Dec 2021 21:53:43 +0100 Subject: [PATCH 02/11] refactor: make options.maxFields more consitent with options.maxFiles --- README.md | 4 ++-- src/Formidable.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 9c841ff6..e5f77663 100644 --- a/README.md +++ b/README.md @@ -311,12 +311,12 @@ See it's defaults in [src/Formidable.js DEFAULT_OPTIONS](./src/Formidable.js) - `options.minFileSize` **{number}** - default `1` (1byte); the minium size of uploaded file. - `options.maxFiles` **{number}** - default `Infinity`; - limit the amount of uploaded files. + limit the amount of uploaded files, set Infinity for unlimited - `options.maxFileSize` **{number}** - default `200 * 1024 * 1024` (200mb); limit the size of each uploaded file. - `options.maxTotalFileSize` **{number}** - default `options.maxFileSize`; limit the size of the batch of uploaded files. -- `options.maxFields` **{number}** - default `1000`; limit the number of fields, set 0 for unlimited +- `options.maxFields` **{number}** - default `1000`; limit the number of fields, set Infinity for unlimited - `options.maxFieldsSize` **{number}** - default `20 * 1024 * 1024` (20mb); limit the amount of memory all fields together (except files) can allocate in bytes. diff --git a/src/Formidable.js b/src/Formidable.js index 5c67b3f2..c60df37b 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -584,7 +584,7 @@ class IncomingForm extends EventEmitter { } _setUpMaxFields() { - if (this.options.maxFields !== 0) { + if (this.options.maxFields !== Infinity) { let fieldsCount = 0; this.on('field', () => { fieldsCount += 1; From 9299ee9dec3ab837fb2b94032a8106fdd16b2292 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Tue, 21 Dec 2021 22:03:19 +0100 Subject: [PATCH 03/11] refactor: make pause a method and define this.req --- src/Formidable.js | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/Formidable.js b/src/Formidable.js index c60df37b..6d3bb15a 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -66,6 +66,7 @@ class IncomingForm extends EventEmitter { 'bytesExpected', 'bytesReceived', '_parser', + 'req', ].forEach((key) => { this[key] = null; }); @@ -110,21 +111,22 @@ class IncomingForm extends EventEmitter { return this; } - parse(req, cb) { - this.pause = () => { - try { - req.pause(); - } catch (err) { - // the stream was destroyed - if (!this.ended) { - // before it was completed, crash & burn - this._error(err); - } - return false; + pause () { + try { + this.req.pause(); + } catch (err) { + // the stream was destroyed + if (!this.ended) { + // before it was completed, crash & burn + this._error(err); } - return true; - }; + return false; + } + return true; + } + parse(req, cb) { + this.req = req; this.resume = () => { try { req.resume(); @@ -462,6 +464,7 @@ class IncomingForm extends EventEmitter { return; } + this.req = null; this.error = err; this.emit(eventName, err); @@ -626,7 +629,7 @@ class IncomingForm extends EventEmitter { if (!this.ended || this._flushing || this.error) { return; } - + this.req = null; this.emit('end'); } } From 3a8bbdc14d758e21d23f7cfd579216d6ecfb2cf4 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Tue, 21 Dec 2021 22:04:50 +0100 Subject: [PATCH 04/11] refactor: make resume a method --- src/Formidable.js | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/Formidable.js b/src/Formidable.js index 6d3bb15a..40e2eaa6 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -125,22 +125,23 @@ class IncomingForm extends EventEmitter { return true; } - parse(req, cb) { - this.req = req; - this.resume = () => { - try { - req.resume(); - } catch (err) { - // the stream was destroyed - if (!this.ended) { - // before it was completed, crash & burn - this._error(err); - } - return false; + resume () { + try { + this.req.resume(); + } catch (err) { + // the stream was destroyed + if (!this.ended) { + // before it was completed, crash & burn + this._error(err); } + return false; + } - return true; - }; + return true; + } + + parse(req, cb) { + this.req = req; // Setup callback first, so we don't miss anything from data events emitted immediately. if (cb) { From 5d70e88c05cd97f5bcbdd2ea8a3d7fe7c1893835 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Tue, 21 Dec 2021 22:07:41 +0100 Subject: [PATCH 05/11] refactor: remove placeholders --- src/Formidable.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/Formidable.js b/src/Formidable.js index 40e2eaa6..17045879 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -247,16 +247,6 @@ class IncomingForm extends EventEmitter { return this.bytesReceived; } - pause() { - // this does nothing, unless overwritten in IncomingForm.parse - return false; - } - - resume() { - // this does nothing, unless overwritten in IncomingForm.parse - return false; - } - onPart(part) { // this method can be overwritten by the user this._handlePart(part); From 530604bd159a61dd479249fb5b12452fb365c592 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Tue, 21 Dec 2021 22:09:23 +0100 Subject: [PATCH 06/11] refactor: remove if statement that is always true --- src/Formidable.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Formidable.js b/src/Formidable.js index 17045879..40e613c6 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -459,11 +459,9 @@ class IncomingForm extends EventEmitter { this.error = err; this.emit(eventName, err); - if (Array.isArray(this.openedFiles)) { - this.openedFiles.forEach((file) => { - file.destroy(); - }); - } + this.openedFiles.forEach((file) => { + file.destroy(); + }); } _parseContentLength() { From 4ae478a8a54c90b4351ec455fcc9717ff4382c7f Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Tue, 21 Dec 2021 22:18:21 +0100 Subject: [PATCH 07/11] refactor: remove old code as comments (use git) --- src/Formidable.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/Formidable.js b/src/Formidable.js index 40e613c6..08633cec 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -435,22 +435,9 @@ class IncomingForm extends EventEmitter { } this.emit('pluginsResults', results); - - // NOTE: probably not needed, because we check options.enabledPlugins in the constructor - // if (results.length === 0 /* && results.length !== this._plugins.length */) { - // this._error( - // new Error( - // `bad content-type header, unknown content-type: ${this.headers['content-type']}`, - // ), - // ); - // } } _error(err, eventName = 'error') { - // if (!err && this.error) { - // this.emit('error', this.error); - // return; - // } if (this.error || this.ended) { return; } @@ -612,9 +599,6 @@ class IncomingForm extends EventEmitter { } _maybeEnd() { - // console.log('ended', this.ended); - // console.log('_flushing', this._flushing); - // console.log('error', this.error); if (!this.ended || this._flushing || this.error) { return; } From d7110cfa6ba909f5947050f07f0436581b29bbd3 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Tue, 21 Dec 2021 22:21:33 +0100 Subject: [PATCH 08/11] refactor: make plugins consistent (they all do not return) --- src/plugins/octetstream.js | 2 -- src/plugins/querystring.js | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/plugins/octetstream.js b/src/plugins/octetstream.js index 5d29ad17..8c93ba82 100644 --- a/src/plugins/octetstream.js +++ b/src/plugins/octetstream.js @@ -14,8 +14,6 @@ export default function plugin(formidable, options) { if (/octet-stream/i.test(self.headers['content-type'])) { init.call(self, self, options); } - - return self; } // Note that it's a good practice (but it's up to you) to use the `this.options` instead diff --git a/src/plugins/querystring.js b/src/plugins/querystring.js index b252ecf1..fcb28552 100644 --- a/src/plugins/querystring.js +++ b/src/plugins/querystring.js @@ -15,8 +15,6 @@ export default function plugin(formidable, options) { if (/urlencoded/i.test(self.headers['content-type'])) { init.call(self, self, options); } - - return self; }; // Note that it's a good practice (but it's up to you) to use the `this.options` instead From 0edd3c23ad7dde01001f506374638c3d6438cfc5 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Tue, 21 Dec 2021 22:24:15 +0100 Subject: [PATCH 09/11] refactor: do not use pluginReturn as plugins do not return anything --- src/Formidable.js | 13 +++---------- test/unit/custom-plugins.test.js | 7 ------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/src/Formidable.js b/src/Formidable.js index 08633cec..1be18d35 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -410,11 +410,9 @@ class IncomingForm extends EventEmitter { // eslint-disable-next-line no-plusplus for (let idx = 0; idx < this._plugins.length; idx++) { const plugin = this._plugins[idx]; - - let pluginReturn = null; - + try { - pluginReturn = plugin(this, this.options) || this; + plugin(this, this.options) || this; } catch (err) { // directly throw from the `form.parse` method; // there is no other better way, except a handle through options @@ -427,14 +425,9 @@ class IncomingForm extends EventEmitter { throw error; } - Object.assign(this, pluginReturn); - // todo: use Set/Map and pass plugin name instead of the `idx` index - this.emit('plugin', idx, pluginReturn); - results.push(pluginReturn); + this.emit('plugin', idx); } - - this.emit('pluginsResults', results); } _error(err, eventName = 'error') { diff --git a/test/unit/custom-plugins.test.js b/test/unit/custom-plugins.test.js index 18f8621e..6ae00ef1 100644 --- a/test/unit/custom-plugins.test.js +++ b/test/unit/custom-plugins.test.js @@ -60,10 +60,6 @@ test('should call 3 custom and 1 builtin plugins, when .parse() is called', asyn ctx.__pluginsCount = ctx.__pluginsCount || 0; ctx.__pluginsCount += 1; }); - form.on('pluginsResults', (results) => { - expect(results.length).toBe(4); - ctx.__pluginsResults = true; - }); form.on('end', () => { ctx.__ends = 1; expect(ctx.__customPlugin1).toBe(111); @@ -117,9 +113,6 @@ test('.parse throw error when some plugin fail', async () => { ctx.__pluginsCount = ctx.__pluginsCount || 0; ctx.__pluginsCount += 1; }); - form.on('pluginsResults', () => { - throw new Error('should not be called'); - }); form.once('error', () => { throw new Error('error event should not be fired when plugin throw'); From b28f858de7c94f967f7e07b18171d384da68324a Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Tue, 21 Dec 2021 22:25:17 +0100 Subject: [PATCH 10/11] refactor: do not create unused variables --- src/Formidable.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Formidable.js b/src/Formidable.js index 1be18d35..88b4079e 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -404,8 +404,8 @@ class IncomingForm extends EventEmitter { return; } - const results = []; - const _dummyParser = new DummyParser(this, this.options); + + new DummyParser(this, this.options); // eslint-disable-next-line no-plusplus for (let idx = 0; idx < this._plugins.length; idx++) { From 207236fd8918081997c5d385fdbb043cf047a3db Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Tue, 21 Dec 2021 22:27:44 +0100 Subject: [PATCH 11/11] refactor: prefer forEach --- src/Formidable.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Formidable.js b/src/Formidable.js index 88b4079e..4a03d667 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -407,10 +407,7 @@ class IncomingForm extends EventEmitter { new DummyParser(this, this.options); - // eslint-disable-next-line no-plusplus - for (let idx = 0; idx < this._plugins.length; idx++) { - const plugin = this._plugins[idx]; - + this._plugins.forEach((plugin, idx) => { try { plugin(this, this.options) || this; } catch (err) { @@ -427,7 +424,7 @@ class IncomingForm extends EventEmitter { // todo: use Set/Map and pass plugin name instead of the `idx` index this.emit('plugin', idx); - } + }); } _error(err, eventName = 'error') {