Merge pull request #27 from pronein/feature/allowExtensionPreservationWithSafeFileNames

richardgirges · web-flow · commit 8f599b481a2f · 2017-04-29T21:51:34.000-07:00
Feature - Allow extension preservation with safe file names
diff --git a/README.md b/README.md
@@ -121,6 +121,7 @@ Pass in non-Busboy options directly to the middleware. These are express-fileupl
 Option | Acceptable&nbsp;Values | Details
 --- | --- | ---
 safeFileNames | <ul><li><code>false</code>&nbsp;**(default)**</li><li><code>true</code></li><li>regex</li></ul> | Strips characters from the upload's filename. You can use custom regex to determine what to strip. If set to `true`, non-alphanumeric characters _except_ dashes and underscores will be stripped. This option is off by default.<br /><br />**Example #1 (strip slashes from file names):** `app.use(fileUpload({ safeFileNames: /\\/g }))`<br />**Example #2:** `app.use(fileUpload({ safeFileNames: true }))`
+preserveExtension | <ul><li><code>false</code>&nbsp;**(default)**</li><li><code>true</code></li><li><code>*Number*</code></li></ul> | Preserves filename extension when using <code>safeFileNames</code> option. If set to <code>true</code>, will default to an extension length of 3. If set to <code>*Number*</code>, this will be the max allowable extension length. If an extension is smaller than the extension length, it remains untouched. If the extension is longer, it is shifted.<br /><br />**Example #1 (true):**<br /><code>app.use(fileUpload({ safeFileNames: true, preserveExtension: true }));</code><br />*myFileName.ext* --> *myFileName.ext*<br /><br />**Example #2 (max extension length 2, extension shifted):**<br /><code>app.use(fileUpload({ safeFileNames: true, preserveExtension: 2 }));</code><br />*myFileName.ext* --> *myFileNamee.xt*
 
 # Help Wanted
 Pull Requests are welcomed!
diff --git a/lib/index.js b/lib/index.js
@@ -15,7 +15,7 @@ module.exports = function(options) {
 
   return function(req, res, next) {
     if (!hasBody(req) || !hasAcceptableMethod(req) || !hasAcceptableMime(req))
-        return next();
+      return next();
 
     processMultipart(options, req, res, next);
   };
@@ -85,14 +85,40 @@ function processMultipart(options, req, res, next) {
       // see: https://github.com/richardgirges/express-fileupload/issues/14
       // firefox uploads empty file in case of cache miss when f5ing page.
       // resulting in unexpected behavior. if there is no file data, the file is invalid.
-      if(!buf.length)
+      if (!buf.length)
         return;
 
       if (options.safeFileNames) {
+        let maxExtensionLength = 3;
+        let extension = '';
+
         if (typeof options.safeFileNames === 'object')
           safeFileNameRegex = options.safeFileNames;
 
-        filename = filename.replace(safeFileNameRegex, '');
+        maxExtensionLength = parseInt(options.preserveExtension);
+        if (options.preserveExtension || maxExtensionLength === 0) {
+          if (isNaN(maxExtensionLength))
+            maxExtensionLength = 3;
+          else
+            maxExtensionLength = Math.abs(maxExtensionLength);
+
+          let filenameParts = filename.split('.');
+          let filenamePartsLen = filenameParts.length;
+          if (filenamePartsLen > 1) {
+            extension = filenameParts.pop();
+
+            if (extension.length > maxExtensionLength && maxExtensionLength > 0) {
+              filenameParts[filenameParts.length - 1] +=
+                '.' + extension.substr(0, extension.length - maxExtensionLength);
+              extension = extension.substr(-maxExtensionLength);
+            }
+
+            extension = maxExtensionLength ? '.' + extension.replace(safeFileNameRegex, '') : '';
+            filename = filenameParts.join('.');
+          }
+        }
+
+        filename = filename.replace(safeFileNameRegex, '').concat(extension);
       }
 
       let newFile = {
@@ -123,9 +149,9 @@ function processMultipart(options, req, res, next) {
       } else {
         // Array fields
         if (req.files[fieldname] instanceof Array)
-            req.files[fieldname].push(newFile);
+          req.files[fieldname].push(newFile);
         else
-            req.files[fieldname] = [req.files[fieldname], newFile];
+          req.files[fieldname] = [req.files[fieldname], newFile];
       }
     });
   });
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "express-fileupload",
-  "version": "0.1.2",
+  "version": "0.1.3",
   "author": "Richard Girges <richardgirges@gmail.com>",
   "description": "Simple express file upload middleware that wraps around Busboy",
   "main": "./lib/index",
diff --git a/test/files/basket.ball.bp b/test/files/basket.ball.bp
diff --git a/test/files/my$Invalid#fileName.png123 b/test/files/my$Invalid#fileName.png123
diff --git a/test/multipartFields.spec.js b/test/multipartFields.spec.js
@@ -2,7 +2,7 @@
 
 const request = require('supertest');
 const server = require('./server');
-const app = server.app;
+const app = server.setup();
 
 let mockUser = {
   firstName: 'Joe',
diff --git a/test/multipartUploads.spec.js b/test/multipartUploads.spec.js
@@ -4,7 +4,7 @@ const fs = require('fs');
 const path = require('path');
 const request = require('supertest');
 const server = require('./server');
-const app = server.app;
+const app = server.setup();
 const clearUploadsDir = server.clearUploadsDir;
 const fileDir = server.fileDir;
 const uploadDir = server.uploadDir;
diff --git a/test/options.spec.js b/test/options.spec.js
@@ -0,0 +1,187 @@
+const fs = require('fs');
+const path = require('path');
+const request = require('supertest');
+const server = require('./server');
+const clearUploadsDir = server.clearUploadsDir;
+const fileDir = server.fileDir;
+const uploadDir = server.uploadDir;
+
+describe('File Upload Options Tests', function() {
+  afterEach(function(done) {
+    clearUploadsDir();
+    done();
+  });
+
+  /**
+   * Upload the file for testing and verify the expected filename.
+   * @param {object} options The expressFileUpload options.
+   * @param {string} actualFileNameToUpload The name of the file to upload.
+   * @param {string} expectedFileNameOnFileSystem The name of the file after upload.
+   * @param {function} done The mocha continuation function.
+   */
+  function executeFileUploadTestWalk(options,
+                                     actualFileNameToUpload,
+                                     expectedFileNameOnFileSystem,
+                                     done) {
+    request(server.setup(options))
+      .post('/upload/single')
+      .attach('testFile', path.join(fileDir, actualFileNameToUpload))
+      .expect(200)
+      .end(function(err) {
+        if (err)
+          return done(err);
+
+        const uploadedFilePath = path.join(uploadDir, expectedFileNameOnFileSystem);
+
+        fs.stat(uploadedFilePath, done);
+      });
+  }
+
+  describe('Testing [safeFileNames] option to ensure:', function() {
+    it('Does nothing to your filename when disabled.',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: false};
+        const actualFileName = 'my$Invalid#fileName.png123';
+        const expectedFileName = 'my$Invalid#fileName.png123';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Is disabled by default.',
+      function(done) {
+        const fileUploadOptions = null;
+        const actualFileName = 'my$Invalid#fileName.png123';
+        const expectedFileName = 'my$Invalid#fileName.png123';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Strips away all non-alphanumeric characters (excluding hyphens/underscores) when enabled.',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: true};
+        const actualFileName = 'my$Invalid#fileName.png123';
+        const expectedFileName = 'myInvalidfileNamepng123';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Accepts a regex for stripping (decidedly) "invalid" characters from filename.',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: /[\$#]/g};
+        const actualFileName = 'my$Invalid#fileName.png123';
+        const expectedFileName = 'myInvalidfileName.png123';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+  });
+
+  describe('Testing [preserveExtension] option to ensure:', function() {
+    it('Does not preserve the extension of your filename when disabled.',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: true, preserveExtension: false};
+        const actualFileName = 'my$Invalid#fileName.png123';
+        const expectedFileName = 'myInvalidfileNamepng123';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Is disabled by default.',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: true};
+        const actualFileName = 'my$Invalid#fileName.png123';
+        const expectedFileName = 'myInvalidfileNamepng123';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Shortens your extension to the default(3) when enabled, if the extension found is larger.',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: true, preserveExtension: true};
+        const actualFileName = 'my$Invalid#fileName.png123';
+        const expectedFileName = 'myInvalidfileNamepng.123';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Leaves your extension alone when enabled, if the extension found is <= default(3) length',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: true, preserveExtension: true};
+        const actualFileName = 'car.png';
+        const expectedFileName = 'car.png';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Can be configured for an extension length > default(3).',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: true, preserveExtension: 7};
+        const actualFileName = 'my$Invalid#fileName.png123';
+        const expectedFileName = 'myInvalidfileName.png123';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Can be configured for an extension length < default(3).',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: true, preserveExtension: 2};
+        const actualFileName = 'my$Invalid#fileName.png123';
+        const expectedFileName = 'myInvalidfileNamepng1.23';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Will use the absolute value of your extension length when negative.',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: true, preserveExtension: -5};
+        const actualFileName = 'my$Invalid#fileName.png123';
+        const expectedFileName = 'myInvalidfileNamep.ng123';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Will leave no extension when the extension length == 0.',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: true, preserveExtension: 0};
+        const actualFileName = 'car.png';
+        const expectedFileName = 'car';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Will accept numbers as strings, if they can be resolved with parseInt.',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: true, preserveExtension: '3'};
+        const actualFileName = 'my$Invalid#fileName.png123';
+        const expectedFileName = 'myInvalidfileNamepng.123';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Will be evaluated for truthy-ness if it cannot be parsed as an int.',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: true, preserveExtension: 'not-a-#-but-truthy'};
+        const actualFileName = 'my$Invalid#fileName.png123';
+        const expectedFileName = 'myInvalidfileNamepng.123';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Will ignore any decimal amount when evaluating for extension length.',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: true, preserveExtension: 4.98};
+        const actualFileName = 'my$Invalid#fileName.png123';
+        const expectedFileName = 'myInvalidfileNamepn.g123';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+
+    it('Only considers the last dotted part as the extension.',
+      function(done) {
+        const fileUploadOptions = {safeFileNames: true, preserveExtension: true};
+        const actualFileName = 'basket.ball.bp';
+        const expectedFileName = 'basketball.bp';
+
+        executeFileUploadTestWalk(fileUploadOptions, actualFileName, expectedFileName, done);
+      });
+  });
+});
diff --git a/test/server.js b/test/server.js

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "express-fileupload",`
`3`		`- "version": "0.1.2",`
	`3`	`+ "version": "0.1.3",`
`4`	`4`	`"author": "Richard Girges <[email protected]>",`
`5`	`5`	`"description": "Simple express file upload middleware that wraps around Busboy",`
`6`	`6`	`"main": "./lib/index",`