Safe default filename

README.md

@@ -354,6 +354,10 @@ See it's defaults in [src/Formidable.js DEFAULT_OPTIONS](./src/Formidable.js)
   files for inputs which submit multiple files using the HTML5 `multiple`
   attribute. Also, the `fields` argument will contain arrays of values for
   fields that have names ending with '[]'.
+- `options.filename` **{function}** - default `undefined` Use it to control
+  newName. Must return a string. Will be joined with options.uploadDir.
+
+#### `options.filename`  **{function}** function (name, ext, part, form) -> string
 
 _**Note:** If this size of combined fields, or size of some file is exceeded, an
 `'error'` event is fired._
@@ -567,7 +571,7 @@ form.onPart = function (part) {
   // let formidable handle only non-file parts
   if (part.filename === '' || !part.mime) {
     // used internally, please do not override!
-    form.handlePart(part);
+    form._handlePart(part);
   }
 };
 ```
@@ -634,8 +638,9 @@ file system.
 ```js
 form.on('fileBegin', (formName, file) => {
     // accessible here 
-    // formName the name in the form (<input name="thisname" type="file">)
+    // formName the name in the form (<input name="thisname" type="file">) or http filename for octetstream
     // file.name http filename or null if there was a parsing error
+    // file.newName generated hexoid or what options.filename returned
     // file.path default pathnme as per options.uploadDir and options.filename
     // file.path = CUSTOM_PATH // to change the final path
 });

examples/log-file-content-to-console.js

@@ -8,7 +8,7 @@ const server = http.createServer((req, res) => {
   if (req.url === '/api/upload' && req.method.toLowerCase() === 'post') {
     // parse a file upload
     const form = formidable({
-      fileWriteStreamHandler: () => {
+      fileWriteStreamHandler: (/* file */) => {
         const writable = Writable();
         // eslint-disable-next-line no-underscore-dangle
         writable._write = (chunk, enc, next) => {

examples/store-files-on-s3.js

@@ -15,12 +15,12 @@ const s3Client = new AWS.S3({
   },
 });
 
-const uploadStream = (filename) => {
+const uploadStream = (file) => {
   const pass = PassThrough();
   s3Client.upload(
     {
       Bucket: 'demo-bucket',
-      Key: filename,
+      Key: file.newName,
       Body: pass,
     },
     (err, data) => {

examples/with-http.js

@@ -6,15 +6,22 @@ const formidable = require('../src/index');
 const server = http.createServer((req, res) => {
   if (req.url === '/api/upload' && req.method.toLowerCase() === 'post') {
     // parse a file upload
-    const form = formidable({ multiples: true });
-
-    form.on('fileBegin', (formName, file) => {
-      if (file.name === null) {
-        // todo change lint rules because that is what we recommend
-        // eslint-disable-next-line
-        file.name = 'invalid-characters';
-      }
+    const form = formidable({
+      multiples: true,
+      uploadDir: `uploads`,
+      keepExtensions: true,
+      filename(/*name, ext, part, form*/) {
+        /* name basename of the http filename
+          ext with the dot ".txt" only if keepExtension is true
+         */
+        // slugify to avoid invalid filenames
+        // substr to define a maximum length
+        // return `${slugify(name).${slugify(ext, separator: '')}`.substr(0, 100);
+        return 'yo.txt'; // or completly different name
+        // return 'z/yo.txt'; // subdirectory
+      },
     });
+
     form.parse(req, (err, fields, files) => {
       if (err) {
         console.error(err);

src/Formidable.js

@@ -27,6 +27,7 @@ const DEFAULT_OPTIONS = {
   multiples: false,
   enabledPlugins: ['octetstream', 'querystring', 'multipart', 'json'],
   fileWriteStreamHandler: null,
+  defaultInvalidName: 'invalid-name',
 };
 
 const PersistentFile = require('./PersistentFile');
@@ -47,7 +48,9 @@ class IncomingForm extends EventEmitter {
 
     this.options = { ...DEFAULT_OPTIONS, ...options };
 
-    const dir = this.options.uploadDir || this.options.uploaddir || os.tmpdir();
+    const dir = path.resolve(
+      this.options.uploadDir || this.options.uploaddir || os.tmpdir(),
+    );
 
     this.uploaddir = dir;
     this.uploadDir = dir;
@@ -316,8 +319,11 @@ class IncomingForm extends EventEmitter {
 
     this._flushing += 1;
 
+    const newName = this._getNewName(part);
+    const finalPath = this._joinDirectoryName(newName);
     const file = this._newFile({
-      path: this._rename(part),
+      newName,
+      path: finalPath,
       filename: part.filename,
       mime: part.mime,
     });
@@ -477,18 +483,21 @@ class IncomingForm extends EventEmitter {
     return new MultipartParser(this.options);
   }
 
-  _newFile({ path: filePath, filename: name, mime: type }) {
+  _newFile({ path: filePath, filename, mime, newName }) {
     return this.options.fileWriteStreamHandler
       ? new VolatileFile({
-          name,
-          type,
+          newName,
+          path: filePath, // avoid shadow
+          filename,
+          mime,
           createFileWriteStream: this.options.fileWriteStreamHandler,
           hash: this.options.hash,
         })
       : new PersistentFile({
+          newName,
           path: filePath,
-          name,
-          type,
+          filename,
+          mime,
           hash: this.options.hash,
         });
   }
@@ -527,8 +536,8 @@ class IncomingForm extends EventEmitter {
     return basename.slice(firstDot, lastDot) + extname;
   }
 
-  _uploadPath(part, fp) {
-    const name = fp || `${this.uploadDir}${path.sep}${toHexoId()}`;
+  _uploadPath(part) {
+    const name = toHexoId();
 
     if (part && this.options.keepExtensions) {
       const filename = typeof part === 'string' ? part : part.filename;
@@ -538,17 +547,34 @@ class IncomingForm extends EventEmitter {
     return name;
   }
 
-  _setUpRename() {
-    const hasRename = typeof this.options.filename === 'function';
+  _joinDirectoryName(name) {
+    const newPath = path.join(this.uploadDir, name);
+
+    // prevent directory traversal attacks
+    if (!newPath.startsWith(this.uploadDir)) {
+      return path.join(this.uploadDir, this.options.defaultInvalidName);
+    }
 
-    if (this.options.keepExtensions === true && hasRename) {
-      this._rename = (part) => {
-        const resultFilepath = this.options.filename.call(this, part, this);
+    return newPath;
+  }
 
-        return this._uploadPath(part, resultFilepath);
+  _setUpRename() {
+    const hasRename = typeof this.options.filename === 'function';
+    if (hasRename) {
+      this._getNewName = (part) => {
+        let ext = '';
+        let name = this.options.defaultInvalidName;
+        if (part.filename) {
+          // can be null
+          ({ ext, name } = path.parse(part.filename));
+          if (!this.options.keepExtensions) {
+            ext = '';
+          }
+        }
+        return this.options.filename.call(this, name, ext, part, this);
       };
     } else {
-      this._rename = (part) => this._uploadPath(part);
+      this._getNewName = (part) => this._uploadPath(part);
     }
   }
 

src/FormidableError.js

@@ -1,6 +1,5 @@
 /* eslint-disable no-plusplus */
 
-
 const missingPlugin = 1000;
 const pluginFunction = 1001;
 const aborted = 1002;

src/PersistentFile.js

@@ -10,20 +10,11 @@ class PersistentFile extends EventEmitter {
   constructor(properties) {
     super();
 
-    this.size = 0;
-    this.path = null;
-    this.name = null;
-    this.type = null;
-    this.hash = null;
-    this.lastModifiedDate = null;
+    Object.assign(this, properties);
 
+    this.size = 0;
     this._writeStream = null;
 
-    // eslint-disable-next-line guard-for-in, no-restricted-syntax
-    for (const key in properties) {
-      this[key] = properties[key];
-    }
-
     if (typeof this.hash === 'string') {
       this.hash = crypto.createHash(properties.hash);
     } else {
@@ -42,12 +33,11 @@ class PersistentFile extends EventEmitter {
     const json = {
       size: this.size,
       path: this.path,
-      name: this.name,
-      type: this.type,
+      newName: this.newName,
+      mime: this.mime,
       mtime: this.lastModifiedDate,
       length: this.length,
       filename: this.filename,
-      mime: this.mime,
     };
     if (this.hash && this.hash !== '') {
       json.hash = this.hash;
@@ -56,7 +46,7 @@ class PersistentFile extends EventEmitter {
   }
 
   toString() {
-    return `PersistentFile: ${this.name}, Path: ${this.path}`;
+    return `PersistentFile: ${this.filename}, Path: ${this.path}`;
   }
 
   write(buffer, cb) {

src/VolatileFile.js

@@ -9,18 +9,11 @@ class VolatileFile extends EventEmitter {
   constructor(properties) {
     super();
 
-    this.size = 0;
-    this.name = null;
-    this.type = null;
-    this.hash = null;
+    Object.assign(this, properties);
 
+    this.size = 0;
     this._writeStream = null;
 
-    // eslint-disable-next-line guard-for-in, no-restricted-syntax
-    for (const key in properties) {
-      this[key] = properties[key];
-    }
-
     if (typeof this.hash === 'string') {
       this.hash = crypto.createHash(properties.hash);
     } else {
@@ -29,7 +22,7 @@ class VolatileFile extends EventEmitter {
   }
 
   open() {
-    this._writeStream = this.createFileWriteStream(this.name);
+    this._writeStream = this.createFileWriteStream(this);
     this._writeStream.on('error', (err) => {
       this.emit('error', err);
     });
@@ -42,8 +35,7 @@ class VolatileFile extends EventEmitter {
   toJSON() {
     const json = {
       size: this.size,
-      name: this.name,
-      type: this.type,
+      newName: this.newName,
       length: this.length,
       filename: this.filename,
       mime: this.mime,
@@ -55,7 +47,7 @@ class VolatileFile extends EventEmitter {
   }
 
   toString() {
-    return `VolatileFile: ${this.name}`;
+    return `VolatileFile: ${this.filename}`;
   }
 
   write(buffer, cb) {