From bf7692b151827312a833e212ed3e9f28e7827981 Mon Sep 17 00:00:00 2001 From: William Dibbern Date: Tue, 3 Dec 2013 20:47:39 -0600 Subject: [PATCH] Switch from multipart to busboy Fixes #1227 - Removed deprecated `multipart` references. - Setup `busboy` to pass along file streams and do a naive parse of form values. - Updated logic in file storage and db import to handle file streams instead of the temporary files created by `multipart`. --- .gitignore | 1 + Gruntfile.js | 4 + core/server/api/db.js | 202 ++++++++++++------------- core/server/middleware/ghost-busboy.js | 66 ++++++++ core/server/middleware/index.js | 5 +- core/server/middleware/middleware.js | 5 +- core/server/storage/localfilesystem.js | 33 ++-- package.json | 1 + 8 files changed, 196 insertions(+), 121 deletions(-) create mode 100644 core/server/middleware/ghost-busboy.js diff --git a/.gitignore b/.gitignore index 0004a4a17f..1e10161d21 100644 --- a/.gitignore +++ b/.gitignore @@ -41,6 +41,7 @@ projectFilesBackup /core/server/data/export/exported* /docs /_site +/content/tmp/* /content/data/* /content/plugins/**/* /content/themes/**/* diff --git a/Gruntfile.js b/Gruntfile.js index 7a2cb02778..ad305fc118 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -239,6 +239,10 @@ var path = require('path'), ] }, + storage: { + src: ['core/test/unit/**/storage*_spec.js'] + }, + integration: { src: ['core/test/integration/**/model*_spec.js'] }, diff --git a/core/server/api/db.js b/core/server/api/db.js index 65096d7af1..03875e6ca8 100644 --- a/core/server/api/db.js +++ b/core/server/api/db.js @@ -1,14 +1,15 @@ -var dataExport = require('../data/export'), - dataImport = require('../data/import'), - api = require('../api'), - fs = require('fs-extra'), - path = require('path'), - when = require('when'), - nodefn = require('when/node/function'), - _ = require('underscore'), - schema = require('../data/schema'), - config = require('../config'), - debugPath = config.paths().webroot + '/ghost/debug/', +var dataExport = require('../data/export'), + dataImport = require('../data/import'), + apiNotifications = require('./notifications'), + apiSettings = require('./settings'), + fs = require('fs-extra'), + path = require('path'), + when = require('when'), + nodefn = require('when/node/function'), + _ = require('underscore'), + schema = require('../data/schema'), + config = require('../config'), + debugPath = config.paths().webroot + '/ghost/debug/', db; @@ -27,137 +28,136 @@ db = { res.download(exportedFilePath, 'GhostData.json'); }).otherwise(function (error) { // Notify of an error if it occurs - return api.notification.browse().then(function (notifications) { + return apiNotifications.browse().then(function (notifications) { var notification = { type: 'error', message: error.message || error, status: 'persistent', id: 'per-' + (notifications.length + 1) }; - return api.notifications.add(notification).then(function () { + + return apiNotifications.add(notification).then(function () { res.redirect(debugPath); }); }); }); }, 'import': function (req, res) { - var notification; + var notification, + databaseVersion; - if (!req.files.importfile || req.files.importfile.size === 0 || req.files.importfile.name.indexOf('json') === -1) { + if (!req.files.importfile || !req.files.importfile.path || req.files.importfile.name.indexOf('json') === -1) { /** * Notify of an error if it occurs * * - If there's no file (although if you don't select anything, the input is still submitted, so * !req.files.importfile will always be false) - * - If the size is 0 + * - If there is no path * - If the name doesn't have json in it */ - return api.notification.browse().then(function (notifications) { + return apiNotifications.browse().then(function (notifications) { notification = { type: 'error', message: "Must select a .json file to import", status: 'persistent', id: 'per-' + (notifications.length + 1) }; - return api.notifications.add(notification).then(function () { + + return apiNotifications.add(notification).then(function () { res.redirect(debugPath); }); }); } - // Get the current version for importing - api.settings.read({ key: 'databaseVersion' }) - .then(function (setting) { - return when(setting.value); - }, function () { - return when('001'); - }) - .then(function (databaseVersion) { - // Read the file contents - return nodefn.call(fs.readFile, req.files.importfile.path) - .then(function (fileContents) { - var importData, - error = "", - tableKeys = _.keys(schema); + apiSettings.read({ key: 'databaseVersion' }).then(function (setting) { + return when(setting.value); + }, function () { + return when('001'); + }).then(function (version) { + databaseVersion = version; + // Read the file contents + return nodefn.call(fs.readFile, req.files.importfile.path); + }).then(function (fileContents) { + var importData, + error = '', + tableKeys = _.keys(schema); - // Parse the json data - try { - importData = JSON.parse(fileContents); - } catch (e) { - return when.reject(new Error("Failed to parse the import file")); - } + // Parse the json data + try { + importData = JSON.parse(fileContents); + } catch (e) { + return when.reject(new Error("Failed to parse the import file")); + } - if (!importData.meta || !importData.meta.version) { - return when.reject(new Error("Import data does not specify version")); - } + if (!importData.meta || !importData.meta.version) { + return when.reject(new Error("Import data does not specify version")); + } - _.each(tableKeys, function (constkey) { - _.each(importData.data[constkey], function (elem) { - var prop; - for (prop in elem) { - if (elem.hasOwnProperty(prop)) { - if (schema[constkey].hasOwnProperty(prop)) { - if (elem.hasOwnProperty(prop)) { - if (!_.isNull(elem[prop])) { - if (elem[prop].length > schema[constkey][prop].maxlength) { - error += error !== "" ? "
" : ""; - error += "Property '" + prop + "' exceeds maximum length of " + schema[constkey][prop].maxlength + " (element:" + constkey + " / id:" + elem.id + ")"; - } - } else { - if (!schema[constkey][prop].nullable) { - error += error !== "" ? "
" : ""; - error += "Property '" + prop + "' is not nullable (element:" + constkey + " / id:" + elem.id + ")"; - } - } - } - } else { + _.each(tableKeys, function (constkey) { + _.each(importData.data[constkey], function (elem) { + var prop; + for (prop in elem) { + if (elem.hasOwnProperty(prop)) { + if (schema[constkey].hasOwnProperty(prop)) { + if (elem.hasOwnProperty(prop)) { + if (!_.isNull(elem[prop])) { + if (elem[prop].length > schema[constkey][prop].maxlength) { error += error !== "" ? "
" : ""; - error += "Property '" + prop + "' is not allowed (element:" + constkey + " / id:" + elem.id + ")"; + error += "Property '" + prop + "' exceeds maximum length of " + schema[constkey][prop].maxlength + " (element:" + constkey + " / id:" + elem.id + ")"; + } + } else { + if (!schema[constkey][prop].nullable) { + error += error !== "" ? "
" : ""; + error += "Property '" + prop + "' is not nullable (element:" + constkey + " / id:" + elem.id + ")"; } } } - }); - }); - - if (error !== "") { - return when.reject(new Error(error)); + } else { + error += error !== "" ? "
" : ""; + error += "Property '" + prop + "' is not allowed (element:" + constkey + " / id:" + elem.id + ")"; + } } - // Import for the current version - return dataImport(databaseVersion, importData); - }); - }) - .then(function importSuccess() { - return api.notification.browse().then(function (notifications) { - notification = { - type: 'success', - message: "Data imported. Log in with the user details you imported", - status: 'persistent', - id: 'per-' + (notifications.length + 1) - }; - - return api.notifications.add(notification).then(function () { - req.session.destroy(); - res.set({ - "X-Cache-Invalidate": "/*" - }); - res.redirect(config.paths().webroot + '/ghost/signin/'); - }); - }); - }, function importFailure(error) { - return api.notification.browse().then(function (notifications) { - // Notify of an error if it occurs - notification = { - type: 'error', - message: error.message || error, - status: 'persistent', - id: 'per-' + (notifications.length + 1) - }; - - return api.notifications.add(notification).then(function () { - res.redirect(debugPath); - }); + } }); }); + + if (error !== "") { + return when.reject(new Error(error)); + } + // Import for the current version + return dataImport(databaseVersion, importData); + }).then(function importSuccess() { + return apiNotifications.browse(); + }).then(function (notifications) { + notification = { + type: 'success', + message: "Data imported. Log in with the user details you imported", + status: 'persistent', + id: 'per-' + (notifications.length + 1) + }; + + return apiNotifications.add(notification).then(function () { + req.session.destroy(); + res.set({ + "X-Cache-Invalidate": "/*" + }); + res.redirect(config.paths().webroot + '/ghost/signin/'); + }); + }).otherwise(function importFailure(error) { + return apiNotifications.browse().then(function (notifications) { + // Notify of an error if it occurs + notification = { + type: 'error', + message: error.message || error, + status: 'persistent', + id: 'per-' + (notifications.length + 1) + }; + + return apiNotifications.add(notification).then(function () { + res.redirect(debugPath); + }); + }); + }); } }; diff --git a/core/server/middleware/ghost-busboy.js b/core/server/middleware/ghost-busboy.js new file mode 100644 index 0000000000..fc9246cfda --- /dev/null +++ b/core/server/middleware/ghost-busboy.js @@ -0,0 +1,66 @@ +var BusBoy = require('busboy'), + fs = require('fs-extra'), + path = require('path'), + os = require('os'); + +// ### ghostBusboy +// Process multipart file streams and copies them to a memory stream to be +// processed later. +function ghostBusBoy(req, res, next) { + var busboy, + tmpDir, + hasError = false; + + if (req.method && req.method.match(/get/i)) { + return next(); + } + + busboy = new BusBoy({ headers: req.headers }); + tmpDir = os.tmpdir(); + + req.files = req.files || {}; + req.body = req.body || {}; + + busboy.on('file', function (fieldname, file, filename, encoding, mimetype) { + var filePath; + + // If the filename is invalid, mark an error + if (!filename) { + hasError = true; + } + // If we've flagged any errors, do not process any streams + if (hasError) { + return file.emit('end'); + } + + filePath = path.join(tmpDir, filename || 'temp.tmp'); + + file.on('end', function () { + req.files[fieldname] = { + type: mimetype, + encoding: encoding, + name: filename, + path: filePath + }; + }); + + busboy.on('limit', function () { + hasError = true; + res.send(413, { errorCode: 413, message: 'File size limit breached.' }); + }); + + file.pipe(fs.createWriteStream(filePath)); + }); + + busboy.on('field', function (fieldname, val) { + req.body[fieldname] = val; + }); + + busboy.on('end', function () { + next(); + }); + + req.pipe(busboy); +} + +module.exports = ghostBusBoy; \ No newline at end of file diff --git a/core/server/middleware/index.js b/core/server/middleware/index.js index d58cbdc2f6..949514af92 100644 --- a/core/server/middleware/index.js +++ b/core/server/middleware/index.js @@ -237,9 +237,8 @@ module.exports = function (server, dbHash) { expressServer.use(express.json()); expressServer.use(express.urlencoded()); - expressServer.use(root + '/ghost/upload/', express.multipart()); - expressServer.use(root + '/ghost/upload/', express.multipart({uploadDir: __dirname + '/content/images'})); - expressServer.use(root + '/ghost/api/v0.1/db/', express.multipart()); + expressServer.use('/ghost/upload/', middleware.busboy); + expressServer.use('/ghost/api/v0.1/db/', middleware.busboy); // Session handling expressServer.use(express.cookieParser()); diff --git a/core/server/middleware/middleware.js b/core/server/middleware/middleware.js index d666fe3c18..39af2a2d97 100644 --- a/core/server/middleware/middleware.js +++ b/core/server/middleware/middleware.js @@ -4,6 +4,7 @@ var _ = require('underscore'), express = require('express'), + busboy = require('./ghost-busboy'), config = require('../config'), path = require('path'), api = require('../api'), @@ -139,7 +140,9 @@ var middleware = { return; } next(); - } + }, + + busboy: busboy }; module.exports = middleware; diff --git a/core/server/storage/localfilesystem.js b/core/server/storage/localfilesystem.js index 4819bb844a..cc28744592 100644 --- a/core/server/storage/localfilesystem.js +++ b/core/server/storage/localfilesystem.js @@ -20,24 +20,25 @@ localFileStore = _.extend(baseStore, { // - returns a promise which ultimately returns the full url to the uploaded image 'save': function (image) { var saved = when.defer(), - targetDir = this.getTargetDir(config.paths().imagesRelPath); + targetDir = this.getTargetDir(config.paths().imagesRelPath), + targetFilename; this.getUniqueFileName(this, image, targetDir).then(function (filename) { - nodefn.call(fs.mkdirs, targetDir).then(function () { - return nodefn.call(fs.copy, image.path, filename); - }).then(function () { - // we should remove the temporary image - return nodefn.call(fs.unlink, image.path).otherwise(errors.logError); - }).then(function () { - // The src for the image must be in URI format, not a file system path, which in Windows uses \ - // For local file system storage can use relative path so add a slash - var fullUrl = ('/' + filename).replace(new RegExp('\\' + path.sep, 'g'), '/'); - return saved.resolve(fullUrl); - }).otherwise(function (e) { - errors.logError(e); - return saved.reject(e); - }); - }).otherwise(errors.logError); + targetFilename = filename; + return nodefn.call(fs.mkdirs, targetDir); + }).then(function () { + return nodefn.call(fs.copy, image.path, targetFilename); + }).then(function () { + return nodefn.call(fs.unlink, image.path).otherwise(errors.logError); + }).then(function () { + // The src for the image must be in URI format, not a file system path, which in Windows uses \ + // For local file system storage can use relative path so add a slash + var fullUrl = ('/' + targetFilename).replace(new RegExp('\\' + path.sep, 'g'), '/'); + return saved.resolve(fullUrl); + }).otherwise(function (e) { + errors.logError(e); + return saved.reject(e); + }); return saved.promise; }, diff --git a/package.json b/package.json index e37e7b794a..f0d5ddf8ef 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "dependencies": { "bcryptjs": "0.7.10", "bookshelf": "0.6.1", + "busboy": "0.0.12", "colors": "0.6.2", "connect-slashes": "1.0.2", "downsize": "0.0.4",