Merge pull request #6659 from jaswilli/multer

Replace busboy upload middleware with multer

core/server/api/db.js

@@ -66,13 +66,16 @@ db = {
         options = options || {};
 
         function validate(options) {
+            options.name = options.originalname;
+            options.type = options.mimetype;
+
             // Check if a file was provided
-            if (!utils.checkFileExists(options, 'importfile')) {
+            if (!utils.checkFileExists(options)) {
                 return Promise.reject(new errors.ValidationError(i18n.t('errors.api.db.selectFileToImport')));
             }
 
             // Check if the file is valid
-            if (!utils.checkFileIsValid(options.importfile, importer.getTypes(), importer.getExtensions())) {
+            if (!utils.checkFileIsValid(options, importer.getTypes(), importer.getExtensions())) {
                 return Promise.reject(new errors.UnsupportedMediaTypeError(
                     i18n.t('errors.api.db.unsupportedFile') +
                         _.reduce(importer.getExtensions(), function (memo, ext) {
@@ -85,7 +88,7 @@ db = {
         }
 
         function importContent(options) {
-            return importer.importFromFile(options.importfile)
+            return importer.importFromFile(options)
                 .then(function () {
                     api.settings.updateSettingsCache();
                 })

core/server/api/index.js

@@ -192,7 +192,7 @@ http = function http(apiMethod) {
     return function apiHandler(req, res, next) {
         // We define 2 properties for using as arguments in API calls:
         var object = req.body,
-            options = _.extend({}, req.files, req.query, req.params, {
+            options = _.extend({}, req.file, req.query, req.params, {
                 context: {
                     user: (req.user && req.user.id) ? req.user.id : null
                 }

core/server/api/upload.js

@@ -1,6 +1,7 @@
 var config  = require('../config'),
     Promise = require('bluebird'),
     fs      = require('fs-extra'),
+    pUnlink = Promise.promisify(fs.unlink),
     storage = require('../storage'),
     errors  = require('../errors'),
     utils   = require('./utils'),
@@ -20,31 +21,31 @@ upload = {
      *
      * @public
      * @param {{context}} options
-     * @returns {Promise} Success
+     * @returns {Promise<String>} location of uploaded file
      */
-    add: function (options) {
+    add: Promise.method(function (options) {
-        var store = storage.getStorage(),
+        var store = storage.getStorage();
-            filepath;
+
+        // Public interface of the storage module's `save` method requires
+        // the file's name to be on the .name property.
+        options.name = options.originalname;
+        options.type = options.mimetype;
 
         // Check if a file was provided
-        if (!utils.checkFileExists(options, 'uploadimage')) {
+        if (!utils.checkFileExists(options)) {
-            return Promise.reject(new errors.NoPermissionError(i18n.t('errors.api.upload.pleaseSelectImage')));
+            throw new errors.NoPermissionError(i18n.t('errors.api.upload.pleaseSelectImage'));
         }
 
         // Check if the file is valid
-        if (!utils.checkFileIsValid(options.uploadimage, config.uploads.contentTypes, config.uploads.extensions)) {
+        if (!utils.checkFileIsValid(options, config.uploads.contentTypes, config.uploads.extensions)) {
-            return Promise.reject(new errors.UnsupportedMediaTypeError(i18n.t('errors.api.upload.pleaseSelectValidImage')));
+            throw new errors.UnsupportedMediaTypeError(i18n.t('errors.api.upload.pleaseSelectValidImage'));
         }
 
-        filepath = options.uploadimage.path;
+        return store.save(options).finally(function () {
-
-        return store.save(options.uploadimage).then(function (url) {
-            return url;
-        }).finally(function () {
             // Remove uploaded file from tmp location
-            return Promise.promisify(fs.unlink)(filepath);
+            return pUnlink(options.path);
         });
-    }
+    })
 };
 
 module.exports = upload;

core/server/api/utils.js

@@ -289,12 +289,12 @@ utils = {
 
         return Promise.resolve(object);
     },
-    checkFileExists: function (options, filename) {
+    checkFileExists: function (fileData) {
-        return !!(options[filename] && options[filename].type && options[filename].path);
+        return !!(fileData.mimetype && fileData.path);
     },
-    checkFileIsValid: function (file, types, extensions) {
+    checkFileIsValid: function (fileData, types, extensions) {
-        var type = file.type,
+        var type = fileData.mimetype,
-            ext = path.extname(file.name).toLowerCase();
+            ext = path.extname(fileData.name).toLowerCase();
 
         if (_.contains(types, type) && _.contains(extensions, ext)) {
             return true;

core/server/middleware/ghost-busboy.js

@@ -1,81 +0,0 @@
-var BusBoy  = require('busboy'),
-    fs      = require('fs-extra'),
-    path    = require('path'),
-    os      = require('os'),
-    i18n    = require('../i18n'),
-    crypto  = require('crypto');
-
-// ### ghostBusboy
-// Process multipart file streams
-function ghostBusBoy(req, res, next) {
-    var busboy,
-        stream,
-        tmpDir;
-
-    // busboy is only used for POST requests
-    if (req.method && !/post/i.test(req.method)) {
-        return next();
-    }
-
-    busboy = new BusBoy({headers: req.headers});
-    tmpDir = os.tmpdir();
-
-    req.files = req.files || {};
-    req.body = req.body || {};
-
-    busboy.on('file', function onFile(fieldname, file, filename, encoding, mimetype) {
-        var filePath,
-            tmpFileName,
-            md5 = crypto.createHash('md5');
-
-        // If the filename is invalid, skip the stream
-        if (!filename) {
-            return file.resume();
-        }
-
-        // Create an MD5 hash of original filename
-        md5.update(filename, 'utf8');
-
-        tmpFileName = (new Date()).getTime() + md5.digest('hex');
-
-        filePath = path.join(tmpDir, tmpFileName || 'temp.tmp');
-
-        file.on('end', function end() {
-            req.files[fieldname] = {
-                type: mimetype,
-                encoding: encoding,
-                name: filename,
-                path: filePath
-            };
-        });
-
-        file.on('error', function onError(error) {
-            console.log('Error', i18n.t('errors.middleware.ghostbusboy.fileUploadingError'), error);
-        });
-
-        stream = fs.createWriteStream(filePath);
-
-        stream.on('error', function onError(error) {
-            console.log('Error', i18n.t('errors.middleware.ghostbusboy.fileUploadingError'), error);
-        });
-
-        file.pipe(stream);
-    });
-
-    busboy.on('error', function onError(error) {
-        console.log('Error', i18n.t('errors.middleware.ghostbusboy.somethingWentWrong'), error);
-        res.status(500).send({code: 500, message: i18n.t('errors.middleware.ghostbusboy.couldNotParseUpload')});
-    });
-
-    busboy.on('field', function onField(fieldname, val) {
-        req.body[fieldname] = val;
-    });
-
-    busboy.on('finish', function onFinish() {
-        next();
-    });
-
-    req.pipe(busboy);
-}
-
-module.exports = ghostBusBoy;

core/server/middleware/index.js

@@ -10,9 +10,9 @@ var bodyParser      = require('body-parser'),
     passport        = require('passport'),
     utils           = require('../utils'),
     sitemapHandler  = require('../data/xml/sitemap/handler'),
-
+    multer          = require('multer'),
+    tmpdir          = require('os').tmpdir,
     authStrategies   = require('./auth-strategies'),
-    busboy           = require('./ghost-busboy'),
     auth             = require('./auth'),
     cacheControl     = require('./cache-control'),
     checkSSL         = require('./check-ssl'),
@@ -34,7 +34,7 @@ var bodyParser      = require('body-parser'),
     setupMiddleware;
 
 middleware = {
-    busboy: busboy,
+    upload: multer({dest: tmpdir()}),
     cacheControl: cacheControl,
     spamPrevention: spamPrevention,
     privateBlogging: privateBlogging,

core/server/routes/api.js

@@ -85,7 +85,7 @@ apiRoutes = function apiRoutes(middleware) {
 
     // ## DB
     router.get('/db', authenticatePrivate, api.http(api.db.exportContent));
-    router.post('/db', authenticatePrivate, middleware.busboy, api.http(api.db.importContent));
+    router.post('/db', authenticatePrivate, middleware.upload.single('importfile'), api.http(api.db.importContent));
     router.del('/db', authenticatePrivate, api.http(api.db.deleteAllContent));
 
     // ## Mail
@@ -111,7 +111,7 @@ apiRoutes = function apiRoutes(middleware) {
     router.post('/authentication/revoke', authenticatePrivate, api.http(api.authentication.revoke));
 
     // ## Uploads
-    router.post('/uploads', authenticatePrivate, middleware.busboy, api.http(api.uploads.add));
+    router.post('/uploads', authenticatePrivate, middleware.upload.single('uploadimage'), api.http(api.uploads.add));
 
     // API Router middleware
     router.use(middleware.api.errorHandler);

core/server/translations/en.json

@@ -70,11 +70,6 @@
                 "accessDenied": "Access denied.",
                 "pleaseSignIn": "Please Sign In"
             },
-            "ghostbusboy": {
-                "fileUploadingError": "Something went wrong uploading the file",
-                "somethingWentWrong": "Something went wrong parsing the form",
-                "couldNotParseUpload": "Could not parse upload completely."
-            },
             "oauth": {
                 "invalidClient": "Invalid client.",
                 "invalidRefreshToken": "Invalid refresh token.",

core/test/integration/api/api_db_spec.js

@@ -93,11 +93,11 @@ describe('DB API', function () {
     });
 
     it('import content is denied (editor, author & without authentication)', function (done) {
-        var file = {importfile: {
+        var file = {
-            name: 'myFile.json',
+            originalname: 'myFile.json',
             path: '/my/path/myFile.json',
-            type: 'application/json'
+            mimetype: 'application/json'
-        }};
+        };
 
         return dbAPI.importContent(_.extend(testUtils.context.editor, file)).then(function () {
             done(new Error('Import content is not denied for editor.'));
@@ -124,7 +124,7 @@ describe('DB API', function () {
             error.errorType.should.eql('ValidationError');
 
             var context = _.extend(testUtils.context.admin, {
-                importfile: {name: 'myFile.docx', path: '/my/path/myFile.docx', type: 'application/docx'}
+                originalname: 'myFile.docx', path: '/my/path/myFile.docx', mimetype: 'application/docx'
             });
 
             return dbAPI.importContent(context);

core/test/integration/api/api_upload_spec.js

@@ -8,8 +8,8 @@ var tmp         = require('tmp'),
     // Stuff we are testing
     UploadAPI   = require('../../../server/api/upload'),
     uploadimage = {
-        name: '',
+        originalname: '',
-        type: '',
+        mimetype: '',
         path: ''
     };
 
@@ -22,8 +22,8 @@ function setupAndTestUpload(filename, mimeType) {
             }
 
             uploadimage.path = path;
-            uploadimage.name = filename;
+            uploadimage.originalname = filename;
-            uploadimage.type = mimeType;
+            uploadimage.mimetype = mimeType;
 
             // create a temp directory (the directory that the API saves the file to)
             tmp.dir({keep: true, unsafeCleanup: true}, function (err, path, cleanupDir) {
@@ -37,7 +37,7 @@ function setupAndTestUpload(filename, mimeType) {
                     }
                 });
 
-                UploadAPI.add({uploadimage: uploadimage})
+                UploadAPI.add(uploadimage)
                 .then(resolve)
                 .catch(reject)
                 .finally(function () {
@@ -65,8 +65,8 @@ function testResult(filename, result) {
 describe('Upload API', function () {
     afterEach(function () {
         uploadimage  = {
-            name: '',
+            originalname: '',
-            type: 'application/octet-stream',
+            mimetype: 'application/octet-stream',
             path: ''
         };
 

core/test/unit/api_utils_spec.js

@@ -374,31 +374,31 @@ describe('API Utils', function () {
 
     describe('checkFileExists', function () {
         it('should return true if file exists in input', function () {
-            apiUtils.checkFileExists({test: {type: 'file', path: 'path'}}, 'test').should.be.true();
+            apiUtils.checkFileExists({mimetype: 'file', path: 'path'}).should.be.true();
         });
 
         it('should return false if file does not exist in input', function () {
-            apiUtils.checkFileExists({test: {type: 'file', path: 'path'}}, 'notthere').should.be.false();
+            apiUtils.checkFileExists({}).should.be.false();
         });
 
         it('should return false if file is incorrectly structured', function () {
-            apiUtils.checkFileExists({test: 'notafile'}, 'test').should.be.false();
+            apiUtils.checkFileExists({type: 'file'}).should.be.false();
         });
     });
 
     describe('checkFileIsValid', function () {
         it('returns true if file has valid extension and type', function () {
-            apiUtils.checkFileIsValid({name: 'test.txt', type: 'text'}, ['text'], ['.txt']).should.be.true();
+            apiUtils.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['text'], ['.txt']).should.be.true();
-            apiUtils.checkFileIsValid({name: 'test.jpg', type: 'jpeg'}, ['text', 'jpeg'], ['.txt', '.jpg']).should.be.true();
+            apiUtils.checkFileIsValid({name: 'test.jpg', mimetype: 'jpeg'}, ['text', 'jpeg'], ['.txt', '.jpg']).should.be.true();
         });
 
         it('returns false if file has invalid extension', function () {
-            apiUtils.checkFileIsValid({name: 'test.txt', type: 'text'}, ['text'], ['.tar']).should.be.false();
+            apiUtils.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['text'], ['.tar']).should.be.false();
-            apiUtils.checkFileIsValid({name: 'test', type: 'text'}, ['text'], ['.txt']).should.be.false();
+            apiUtils.checkFileIsValid({name: 'test', mimetype: 'text'}, ['text'], ['.txt']).should.be.false();
         });
 
         it('returns false if file has invalid type', function () {
-            apiUtils.checkFileIsValid({name: 'test.txt', type: 'text'}, ['archive'], ['.txt']).should.be.false();
+            apiUtils.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['archive'], ['.txt']).should.be.false();
         });
     });
 

package.json

@@ -30,7 +30,6 @@
     "bluebird": "3.3.3",
     "body-parser": "1.14.2",
     "bookshelf": "0.9.2",
-    "busboy": "0.2.12",
     "chalk": "1.1.1",
     "cheerio": "0.20.0",
     "compression": "1.6.1",
@@ -52,6 +51,7 @@
     "lodash": "3.10.1",
     "moment": "2.11.2",
     "morgan": "1.6.1",
+    "multer": "1.1.0",
     "node-uuid": "1.4.7",
     "nodemailer": "0.7.1",
     "oauth2orize": "1.2.2",