feat: Url search params and remove multiple (#730)

Author: GrosSacASac

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ### Unreleased 3.0
 
+ * feat: remove options.multiples ([730](https://github.com/node-formidable/formidable/pull/730))
+ * use modern URLSearchParams https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams internally
+ * files and fields values are always arrays
+ * fields with [] in the name do not receive special treatment
+ * remove unused qs and querystring dependency
  * feat: Use ES modules ([727](https://github.com/node-formidable/formidable/pull/727))
  * options.enabledPlugins must contain the plugin themselves instead of the plugins names 
 

examples/with-http.js

@@ -49,6 +49,13 @@ const server = http.createServer((req, res) => {
       <div>File: <input type="file" name="multipleFiles" multiple="multiple" /></div>
       <input type="submit" value="Upload" />
     </form>
+
+    <form action="/api/upload" enctype="multipart/form-data" method="post">
+      <div>Text field title: <input type="text" name="title" /></div>
+      <div>Text field with same name: <input type="text" name="title" /></div>
+      <div>Other field <input type="text" name="other" /></div>
+      <input type="submit" value="submit simple" />
+    </form>
   `);
 });
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "formidable",
-  "version": "3.0.0-canary.20210427",
+  "version": "3.0.0-canary.20210428",
   "license": "MIT",
   "description": "A node.js module for parsing form data, especially file uploads.",
   "homepage": "https://github.com/node-formidable/formidable",
@@ -30,13 +30,12 @@
     "pretest": "del-cli ./test/tmp && make-dir ./test/tmp",
     "test": "node  --experimental-vm-modules ./node_modules/jest/bin/jest.js --coverage",
     "pretest:ci": "yarn run pretest",
-    "test:ci": "node --experimental-vm-modules node_modules/.bin/nyc jest --coverage" 
+    "test:ci": "node --experimental-vm-modules node_modules/.bin/nyc jest --coverage"
   },
   "dependencies": {
     "dezalgo": "1.0.3",
     "hexoid": "1.0.0",
-    "once": "1.4.0",
-    "qs": "6.9.3"
+    "once": "1.4.0"
   },
   "devDependencies": {
     "@commitlint/cli": "8.3.5",

src/Formidable.js

@@ -1,16 +1,13 @@
 /* eslint-disable class-methods-use-this */
 /* eslint-disable no-underscore-dangle */
 
-'use strict';
-
 import os from 'os';
 import path from 'path';
 import hexoid from 'hexoid';
 import once from 'once';
 import dezalgo from 'dezalgo';
 import { EventEmitter } from 'events';
 import { StringDecoder } from 'string_decoder';
-import { stringify, parse as __parse } from 'qs';
 import {
   octetstream,
   querystring,
@@ -29,7 +26,6 @@ const DEFAULT_OPTIONS = {
   encoding: 'utf-8',
   hashAlgorithm: false,
   uploadDir: os.tmpdir(),
-  multiples: false,
   enabledPlugins: [
     octetstream,
     querystring,
@@ -156,39 +152,30 @@ class IncomingForm extends EventEmitter {
 
       this.on('field', (name, value) => {
         if (
-          this.options.multiples &&
-          (this.type === 'multipart' || this.type === 'urlencoded')
+          this.type === 'multipart' || this.type === 'urlencoded'
         ) {
-          const mObj = { [name]: value };
-          mockFields = mockFields
-            ? `${mockFields}&${stringify(mObj)}`
-            : `${stringify(mObj)}`;
+          if (!hasOwnProp(fields, name)) {
+            fields[name] = [value];
+          } else {
+            fields[name].push(value);
+          }
+          
         } else {
           fields[name] = value;
         }
       });
       this.on('file', (name, file) => {
-        // TODO: too much nesting
-        if (this.options.multiples) {
-          if (hasOwnProp(files, name)) {
-            if (!Array.isArray(files[name])) {
-              files[name] = [files[name]];
-            }
-            files[name].push(file);
+          if (!hasOwnProp(files, name)) {
+              files[name] = [file];
           } else {
-            files[name] = file;
+              files[name].push(file);
           }
-        } else {
-          files[name] = file;
-        }
+        
       });
       this.on('error', (err) => {
         callback(err, fields, files);
       });
       this.on('end', () => {
-        if (this.options.multiples) {
-          Object.assign(fields, __parse(mockFields));
-        }
         callback(null, fields, files);
       });
     }

src/parsers/Querystring.js

@@ -1,7 +1,7 @@
 /* eslint-disable no-underscore-dangle */
 
 import { Transform } from 'stream';
-import { parse } from 'querystring';
+
 
 // This is a buffering parser, not quite as nice as the multipart one.
 // If I find time I'll rewrite this to be fully streaming as well
@@ -20,12 +20,11 @@ class QuerystringParser extends Transform {
   }
 
   _flush(callback) {
-    const fields = parse(this.buffer, '&', '=');
-    // eslint-disable-next-line no-restricted-syntax, guard-for-in
-    for (const key in fields) {
+    const fields = new URLSearchParams(this.buffer);
+    for (const [key, value] of fields) {
       this.push({
         key,
-        value: fields[key],
+        value,
       });
     }
     this.buffer = '';

test/integration/file-write-stream-handler-option.test.js

@@ -46,7 +46,7 @@ test('file write stream handler', (done) => {
 
     form.parse(req, (err, fields, files) => {
       strictEqual(Object.keys(files).length, 1);
-      const { file } = files;
+      const file = files.file[0];
 
       strictEqual(file.size, 301);
       strictEqual(typeof file.filepath, 'string');

test/integration/octet-stream.test.js

@@ -23,7 +23,7 @@ test('octet stream', (done) => {
 
     form.parse(req, (err, fields, files) => {
       strictEqual(Object.keys(files).length, 1);
-      const { file } = files;
+      const file = files.file[0];
 
       strictEqual(file.size, 301);
 

test/integration/store-files-option.test.js

@@ -35,7 +35,7 @@ test('store files option', (done) => {
 
     form.parse(req, (err, fields, files) => {
       strictEqual(Object.keys(files).length, 1);
-      const { file } = files;
+      const file = files.file[0];
 
       strictEqual(file.size, 301);
       strictEqual(typeof file.filepath, 'string');

test/standalone/issue-46.test.js

@@ -47,7 +47,7 @@ test('issue 46', (done) => {
       const obj = JSON.parse(body);
 
       ok(obj.fields.foo, 'should have fields.foo === barry');
-      strictEqual(obj.fields.foo, 'barry');
+      strictEqual(obj.fields.foo[0], 'barry');
 
       server.close();
       done();

test/unit/formidable.test.js

@@ -109,8 +109,8 @@ function makeHeader(originalFilename) {
       expect(fields.a[0]).toBe('1');
       expect(fields.a[1]).toBe('2');
     });
-    form.emit('field', 'a[]', '1');
-    form.emit('field', 'a[]', '2');
+    form.emit('field', 'a', '1');
+    form.emit('field', 'a', '2');
     form.emit('end');
   });
 
@@ -123,14 +123,14 @@ function makeHeader(originalFilename) {
       'content-type': 'multipart/form-data; boundary=----TLVx',
     };
     form.parse(req, (error, fields) => {
-      expect(Array.isArray(fields.a)).toBe(true);
-      expect(fields.a[0][0]).toBe('a');
-      expect(fields.a[0][1]).toBe('b');
-      expect(fields.a[1][0]).toBe('c');
+      expect(Array.isArray(fields[`a[0]`])).toBe(true);
+      expect(fields[`a[0]`][0]).toBe('a');
+      expect(fields[`a[0]`][1]).toBe('b');
+      expect(fields[`a[1]`][0]).toBe('c');
     });
-    form.emit('field', 'a[0][]', 'a');
-    form.emit('field', 'a[0][]', 'b');
-    form.emit('field', 'a[1][]', 'c');
+    form.emit('field', 'a[0]', 'a');
+    form.emit('field', 'a[0]', 'b');
+    form.emit('field', 'a[1]', 'c');
     form.emit('end');
   });
 
@@ -143,15 +143,15 @@ function makeHeader(originalFilename) {
       'content-type': 'multipart/form-data; boundary=----TLVx',
     };
     form.parse(req, (error, fields) => {
-      expect(fields.a.x).toBe('1');
-      expect(fields.a.y).toBe('2');
+      expect(fields[`a[x]`][0]).toBe('1');
+      expect(fields[`a[y]`][0]).toBe('2');
     });
     form.emit('field', 'a[x]', '1');
     form.emit('field', 'a[y]', '2');
     form.emit('end');
   });
 
-  test(`${name}#_Nested object parameters support`, () => {
+  xtest(`${name}#_Nested object parameters support`, () => {
     const form = getForm(name, { multiples: true });
 
     const req = new http.ClientRequest();