feat: custom file (re)naming, sanitizing - use the new `options.… (#591)

* feat: custom file (re)naming, options.file

If `options.filename` is passed and is function, it is passed with `part`.
Must return full absolute filepath for the file.
Still works with `options.keepExtensions`.

By default we rename the files to random id (joined to the uploadDir), generated from `hexoid` package.

* fix: normalize uploaddir, add it to default_options
* fix: running tests locally now works
* chore: update deps, publish canary-20200402.1

Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>

package.json

@@ -1,6 +1,6 @@
 {
   "name": "formidable",
-  "version": "2.0.0-canary.20200226.1",
+  "version": "2.0.0-canary.20200402.1",
   "license": "MIT",
   "description": "A node.js module for parsing form data, especially file uploads.",
   "homepage": "https://github.com/node-formidable/formidable",
@@ -21,39 +21,39 @@
     "fmt:prepare": "prettier --write",
     "lint": "yarn run lint:prepare .",
     "lint:prepare": "eslint --cache --fix --quiet --format codeframe",
-    "pretest": "yarn del ./test/tmp",
-    "postpretest": "yarn make-dir ./test/tmp",
-    "reinstall": "yarn del ./node_modules ./yarn.lock",
+    "reinstall": "del-cli ./node_modules ./yarn.lock",
     "postreinstall": "yarn setup",
     "setup": "yarn",
-    "test": "yarn node test/run.js",
-    "pretest:ci": "yarn pretest",
+    "pretest": "del-cli ./test/tmp && make-dir ./test/tmp",
+    "test": "node test/run.js",
+    "pretest:ci": "yarn run pretest",
     "test:ci": "nyc node test/run.js",
     "test:jest": "jest --coverage"
   },
   "dependencies": {
     "dezalgo": "^1.0.3",
+    "hexoid": "^1.0.0",
     "once": "^1.4.0"
   },
   "devDependencies": {
     "@commitlint/cli": "^8.3.5",
     "@commitlint/config-conventional": "^8.3.4",
-    "@tunnckocore/prettier-config": "^1.3.5",
+    "@tunnckocore/prettier-config": "^1.3.8",
     "del-cli": "^3.0.0",
     "eslint": "^6.8.0",
-    "eslint-config-airbnb-base": "^14.0.0",
-    "eslint-config-prettier": "^6.10.0",
-    "eslint-plugin-import": "^2.20.1",
+    "eslint-config-airbnb-base": "^14.1.0",
+    "eslint-config-prettier": "^6.10.1",
+    "eslint-plugin-import": "^2.20.2",
     "eslint-plugin-prettier": "^3.1.2",
     "express": "^4.17.1",
-    "husky": "^4.2.2",
-    "jest": "^25.1.0",
+    "husky": "^4.2.3",
+    "jest": "^25.2.6",
     "koa": "^2.11.0",
-    "lint-staged": "^10.0.7",
+    "lint-staged": "^10.1.1",
     "make-dir-cli": "^2.0.0",
     "nyc": "^15.0.0",
     "prettier": "^2.0.2",
-    "prettier-plugin-pkgjson": "^0.2.5",
+    "prettier-plugin-pkgjson": "^0.2.8",
     "request": "^2.88.2",
     "supertest": "^4.0.2",
     "urun": "^0.0.8",

src/Formidable.js

@@ -6,19 +6,21 @@
 const os = require('os');
 const fs = require('fs');
 const path = require('path');
-const crypto = require('crypto');
+const hexoid = require('hexoid');
 const once = require('once');
 const dezalgo = require('dezalgo');
 const { EventEmitter } = require('events');
 const { StringDecoder } = require('string_decoder');
 
+const toHexoId = hexoid(25);
 const DEFAULT_OPTIONS = {
   maxFields: 1000,
   maxFieldsSize: 20 * 1024 * 1024,
   maxFileSize: 200 * 1024 * 1024,
   keepExtensions: false,
   encoding: 'utf-8',
   hash: false,
+  uploadDir: os.tmpdir(),
   multiples: false,
   enabledPlugins: ['octetstream', 'querystring', 'multipart', 'json'],
 };
@@ -34,19 +36,31 @@ function hasOwnProp(obj, key) {
 class IncomingForm extends EventEmitter {
   constructor(options = {}) {
     super();
-    this.error = null;
-    this.ended = false;
 
     this.options = { ...DEFAULT_OPTIONS, ...options };
-    this.uploadDir = this.uploadDir || os.tmpdir();
 
-    this.headers = null;
-    this.type = null;
-
-    this.bytesReceived = null;
-    this.bytesExpected = null;
+    const dir = this.options.uploadDir || this.options.uploaddir || os.tmpdir();
+
+    this.uploaddir = dir;
+    this.uploadDir = dir;
+
+    this.options.filename =
+      typeof this.options.filename === 'function'
+        ? this.options.filename.bind(this)
+        : this._uploadPath.bind(this);
+
+    // initialize with null
+    [
+      'error',
+      'headers',
+      'type',
+      'bytesExpected',
+      'bytesReceived',
+      '_parser',
+    ].forEach((key) => {
+      this[key] = null;
+    });
 
-    this._parser = null;
     this._flushing = 0;
     this._fieldsSize = 0;
     this._fileSize = 0;
@@ -276,7 +290,7 @@ class IncomingForm extends EventEmitter {
     this._flushing += 1;
 
     const file = new File({
-      path: this._uploadPath(part.filename),
+      path: this.options.filename(part, this),
       name: part.filename,
       type: part.mime,
       hash: this.options.hash,
@@ -422,18 +436,17 @@ class IncomingForm extends EventEmitter {
     return filename;
   }
 
-  _uploadPath(filename) {
-    const buf = crypto.randomBytes(16);
-    let name = `upload_${buf.toString('hex')}`;
+  _uploadPath(part) {
+    const name = `${this.uploadDir}${path.sep}${toHexoId()}`;
 
-    if (this.options.keepExtensions) {
-      let ext = path.extname(filename);
+    if (part && this.options.keepExtensions) {
+      let ext = path.extname(typeof part === 'string' ? part : part.filename);
       ext = ext.replace(/(\.[a-z0-9]+).*/i, '$1');
 
-      name += ext;
+      return `${name}${ext}`;
     }
 
-    return path.join(this.uploadDir, name);
+    return name;
   }
 
   _maybeEnd() {

test/unit/formidable.test.js

@@ -1,3 +1,4 @@
+/* eslint-disable max-statements */
 /* eslint-disable no-underscore-dangle */
 
 'use strict';
@@ -63,20 +64,37 @@ function makeHeader(filename) {
   test(`${name}#_uploadPath strips harmful characters from extension when keepExtensions`, () => {
     const form = getForm(name, { keepExtensions: true });
 
-    let ext = path.extname(form._uploadPath('fine.jpg?foo=bar'));
+    const getBasename = (part) => path.basename(form._uploadPath(part));
+
+    let basename = getBasename('fine.jpg?foo=bar');
+    expect(basename).toHaveLength(29);
+    let ext = path.extname(basename);
     expect(ext).toBe('.jpg');
 
-    ext = path.extname(form._uploadPath('fine?foo=bar'));
+    basename = getBasename('fine-no-ext?foo=qux');
+    expect(basename).toHaveLength(25);
+    ext = path.extname(basename);
     expect(ext).toBe('');
 
-    ext = path.extname(form._uploadPath('super.cr2+dsad'));
+    basename = getBasename({ filename: 'super.cr2+dsad' });
+    expect(basename).toHaveLength(29);
+    ext = path.extname(basename);
     expect(ext).toBe('.cr2');
 
-    ext = path.extname(form._uploadPath('super.bar'));
-    expect(ext).toBe('.bar');
+    basename = getBasename({ filename: 'super.gz' });
+    expect(basename).toHaveLength(28);
+    ext = path.extname(basename);
+    expect(ext).toBe('.gz');
 
-    ext = path.extname(form._uploadPath('file.aAa'));
+    basename = getBasename('file.aAa');
+    expect(basename).toHaveLength(29);
+    ext = path.extname(basename);
     expect(ext).toBe('.aAa');
+
+    basename = getBasename('file#!@#koh.QxZs?sa=1');
+    expect(basename).toHaveLength(30);
+    ext = path.extname(basename);
+    expect(ext).toBe('.QxZs');
   });
 
   test(`${name}#_Array parameters support`, () => {
@@ -93,4 +111,10 @@ function makeHeader(filename) {
     form.emit('field', 'a[]', 2);
     form.emit('end');
   });
+
+  // test(`${name}: use custom options.filename instead of form._uploadPath`, () => {
+  //   const form = getForm(name, {
+  //     filename: (_) => path.join(__dirname, 'sasa'),
+  //   });
+  // });
 });

yarn.lock

@@ -2586,6 +2586,11 @@ hasha@^5.0.0:
     is-stream "^2.0.0"
     type-fest "^0.8.0"
 
+hexoid@^1.0.0:
+  version "1.0.0"
+  resolved "https://registry.yarnpkg.com/hexoid/-/hexoid-1.0.0.tgz#ad10c6573fb907de23d9ec63a711267d9dc9bc18"
+  integrity sha512-QFLV0taWQOZtvIRIAdBChesmogZrtuXvVWsFHZTk2SU+anspqZ2vMnoLg7IE1+Uk16N19APic1BuF8bC8c2m5g==
+
 hosted-git-info@^2.1.4:
   version "2.8.6"
   resolved "https://registry.yarnpkg.com/hosted-git-info/-/hosted-git-info-2.8.6.tgz#3a6e6d0324c5371fc8c7ba7175e1e5d14578724d"
@@ -4561,10 +4566,10 @@ prettier-linter-helpers@^1.0.0:
   dependencies:
     fast-diff "^1.1.2"
 
-prettier-plugin-pkgjson@^0.2.5:
-  version "0.2.5"
-  resolved "https://registry.yarnpkg.com/prettier-plugin-pkgjson/-/prettier-plugin-pkgjson-0.2.5.tgz#f38f412dfcd238cb6d955fd23c39ae482a9b87ab"
-  integrity sha512-vOaW3MvRdvWrqoNWgYRgUgdgGbQIdjHKN2A5tl8acauX6JxxcuZKhlo4e5NWdTmm0q+gnn+rB6fF1hkk3QRu3Q==
+prettier-plugin-pkgjson@^0.2.8:
+  version "0.2.8"
+  resolved "https://registry.yarnpkg.com/prettier-plugin-pkgjson/-/prettier-plugin-pkgjson-0.2.8.tgz#cc644f846887332518d279dd6b8719e0781b742f"
+  integrity sha512-MBpPCjqQKxKc5SxhLkVeE2Q+3N7KBk+zJLkFzHVL6SMRAZlyiq/44w/NonCmO+24pI7s/LKoeFqcWY2+08vuEg==
   dependencies:
     sort-package-json "^1.40.0"