From c41ee354b194b2ea3d1c4853722548eeac7cdcf4 Mon Sep 17 00:00:00 2001 From: Jason Williams Date: Tue, 29 Mar 2016 22:31:31 -0500 Subject: [PATCH] Replace busboy upload middleware with multer - deps: multer@1.1.0 --- core/server/api/db.js | 9 ++- core/server/api/index.js | 2 +- core/server/api/upload.js | 31 ++++---- core/server/api/utils.js | 10 +-- core/server/middleware/ghost-busboy.js | 81 -------------------- core/server/middleware/index.js | 6 +- core/server/routes/api.js | 4 +- core/server/translations/en.json | 5 -- core/test/integration/api/api_db_spec.js | 10 +-- core/test/integration/api/api_upload_spec.js | 14 ++-- core/test/unit/api_utils_spec.js | 16 ++-- package.json | 2 +- 12 files changed, 54 insertions(+), 136 deletions(-) delete mode 100644 core/server/middleware/ghost-busboy.js diff --git a/core/server/api/db.js b/core/server/api/db.js index 6b43fb8b42..33e36eff17 100644 --- a/core/server/api/db.js +++ b/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(); }) diff --git a/core/server/api/index.js b/core/server/api/index.js index e277ccdd65..e91bc71b75 100644 --- a/core/server/api/index.js +++ b/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 } diff --git a/core/server/api/upload.js b/core/server/api/upload.js index 35047d070d..86adb1d3e3 100644 --- a/core/server/api/upload.js +++ b/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} location of uploaded file */ - add: function (options) { - var store = storage.getStorage(), - filepath; + add: Promise.method(function (options) { + var store = storage.getStorage(); + + // 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')) { - return Promise.reject(new errors.NoPermissionError(i18n.t('errors.api.upload.pleaseSelectImage'))); + if (!utils.checkFileExists(options)) { + 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)) { - return Promise.reject(new errors.UnsupportedMediaTypeError(i18n.t('errors.api.upload.pleaseSelectValidImage'))); + if (!utils.checkFileIsValid(options, config.uploads.contentTypes, config.uploads.extensions)) { + throw new errors.UnsupportedMediaTypeError(i18n.t('errors.api.upload.pleaseSelectValidImage')); } - filepath = options.uploadimage.path; - - return store.save(options.uploadimage).then(function (url) { - return url; - }).finally(function () { + return store.save(options).finally(function () { // Remove uploaded file from tmp location - return Promise.promisify(fs.unlink)(filepath); + return pUnlink(options.path); }); - } + }) }; module.exports = upload; diff --git a/core/server/api/utils.js b/core/server/api/utils.js index 9699fbd590..9401527314 100644 --- a/core/server/api/utils.js +++ b/core/server/api/utils.js @@ -289,12 +289,12 @@ utils = { return Promise.resolve(object); }, - checkFileExists: function (options, filename) { - return !!(options[filename] && options[filename].type && options[filename].path); + checkFileExists: function (fileData) { + return !!(fileData.mimetype && fileData.path); }, - checkFileIsValid: function (file, types, extensions) { - var type = file.type, - ext = path.extname(file.name).toLowerCase(); + checkFileIsValid: function (fileData, types, extensions) { + var type = fileData.mimetype, + ext = path.extname(fileData.name).toLowerCase(); if (_.contains(types, type) && _.contains(extensions, ext)) { return true; diff --git a/core/server/middleware/ghost-busboy.js b/core/server/middleware/ghost-busboy.js deleted file mode 100644 index e1ed28c0df..0000000000 --- a/core/server/middleware/ghost-busboy.js +++ /dev/null @@ -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; diff --git a/core/server/middleware/index.js b/core/server/middleware/index.js index b73a72f65f..191b779486 100644 --- a/core/server/middleware/index.js +++ b/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'), @@ -33,7 +33,7 @@ var bodyParser = require('body-parser'), setupMiddleware; middleware = { - busboy: busboy, + upload: multer({dest: tmpdir()}), cacheControl: cacheControl, spamPrevention: spamPrevention, privateBlogging: privateBlogging, diff --git a/core/server/routes/api.js b/core/server/routes/api.js index ffb307ac10..29c2c562ba 100644 --- a/core/server/routes/api.js +++ b/core/server/routes/api.js @@ -80,7 +80,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 @@ -106,7 +106,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); diff --git a/core/server/translations/en.json b/core/server/translations/en.json index ceacd4070c..f60e451619 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -72,11 +72,6 @@ "pleaseSignIn": "Please Sign In", "attemptedToAccessAdmin": "You have attempted to access your Ghost admin panel from a url that does not appear in config.js." }, - "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.", diff --git a/core/test/integration/api/api_db_spec.js b/core/test/integration/api/api_db_spec.js index 5faa92b8ab..044a8334d6 100644 --- a/core/test/integration/api/api_db_spec.js +++ b/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: { - name: 'myFile.json', + var file = { + 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); diff --git a/core/test/integration/api/api_upload_spec.js b/core/test/integration/api/api_upload_spec.js index 67377c69f7..013c528fa1 100644 --- a/core/test/integration/api/api_upload_spec.js +++ b/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: '', - type: '', + originalname: '', + mimetype: '', path: '' }; @@ -22,8 +22,8 @@ function setupAndTestUpload(filename, mimeType) { } uploadimage.path = path; - uploadimage.name = filename; - uploadimage.type = mimeType; + uploadimage.originalname = filename; + 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: '', - type: 'application/octet-stream', + originalname: '', + mimetype: 'application/octet-stream', path: '' }; diff --git a/core/test/unit/api_utils_spec.js b/core/test/unit/api_utils_spec.js index 762a6f1bd6..d79186b0d4 100644 --- a/core/test/unit/api_utils_spec.js +++ b/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.jpg', type: 'jpeg'}, ['text', 'jpeg'], ['.txt', '.jpg']).should.be.true(); + apiUtils.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['text'], ['.txt']).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', type: 'text'}, ['text'], ['.txt']).should.be.false(); + apiUtils.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['text'], ['.tar']).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(); }); }); diff --git a/package.json b/package.json index c973f7e0ca..02739a6c9c 100644 --- a/package.json +++ b/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", @@ -51,6 +50,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",