diff --git a/core/server/api/index.js b/core/server/api/index.js index 079470e352..86f001e823 100644 --- a/core/server/api/index.js +++ b/core/server/api/index.js @@ -266,7 +266,7 @@ addHeaders = function addHeaders(apiMethod, req, res, result) { 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, + let object = req.body, options = _.extend({}, req.file, {ip: req.ip}, req.query, req.params, { context: { // @TODO: forward the client and user obj (options.context.user.id) @@ -276,6 +276,10 @@ http = function http(apiMethod) { } }); + if (req.files) { + options.files = req.files; + } + // If this is a GET, or a DELETE, req.body should be null, so we only have options (route and query params) // If this is a PUT, POST, or PATCH, req.body is an object if (_.isEmpty(object)) { diff --git a/core/server/api/upload.js b/core/server/api/upload.js index 744e78bf0a..5ae1c05577 100644 --- a/core/server/api/upload.js +++ b/core/server/api/upload.js @@ -14,12 +14,27 @@ upload = { /** * ### Add Image * + * We only allow multiple uploads internally - see images middlewares. + * * @public * @param {{context}} options * @returns {Promise} location of uploaded file */ add: Promise.method(function (options) { - var store = storage.getStorage(); + const store = storage.getStorage(); + + if (options.files) { + return Promise.map(options.files, (file) => { + return store + .save(file) + .finally(function () { + // Remove uploaded file from tmp location + return fs.unlink(file.path); + }); + }).then((paths) => { + return paths[0]; + }); + } return store.save(options).finally(function () { // Remove uploaded file from tmp location diff --git a/core/server/config/defaults.json b/core/server/config/defaults.json index fb7305ecb8..0e736f7e00 100644 --- a/core/server/config/defaults.json +++ b/core/server/config/defaults.json @@ -78,5 +78,8 @@ "robotstxt": { "maxAge": 3600000 } + }, + "imageOptimization": { + "resize": true } } diff --git a/core/server/lib/image/index.js b/core/server/lib/image/index.js index 8e03194470..34808a6998 100644 --- a/core/server/lib/image/index.js +++ b/core/server/lib/image/index.js @@ -13,5 +13,9 @@ module.exports = { get imageSizeCache() { return require('./cached-image-size-from-url'); + }, + + get manipulator() { + return require('./manipulator'); } }; diff --git a/core/server/lib/image/manipulator.js b/core/server/lib/image/manipulator.js new file mode 100644 index 0000000000..5a3549af64 --- /dev/null +++ b/core/server/lib/image/manipulator.js @@ -0,0 +1,50 @@ +const Promise = require('bluebird'); +const common = require('../common'); + +/** + * @NOTE: Sharp cannot operate on the same image path, that's why we have to use in & out paths. + * + * We currently can't enable compression or having more config options, because of + * https://github.com/lovell/sharp/issues/1360. + */ +const process = (options = {}) => { + let sharp, img; + + try { + sharp = require('sharp'); + + // @NOTE: workaround for Windows as libvips keeps a reference to the input file + // which makes it impossible to fs.unlink() it on cleanup stage + sharp.cache(false); + img = sharp(options.in); + } catch (err) { + return Promise.reject(new common.errors.InternalServerError({ + message: 'Sharp wasn\'t installed', + code: 'SHARP_INSTALLATION', + err: err + })); + } + + return img.metadata() + .then((metadata) => { + if (metadata.width > options.width) { + img.resize(options.width); + } + + // CASE: if you call `rotate` it will automatically remove the orientation (and all other meta data) and rotates + // based on the orientation. It does not rotate if no orientation is set. + img.rotate(); + }) + .then(() => { + return img.toFile(options.out); + }) + .catch((err) => { + throw new common.errors.InternalServerError({ + message: 'Unable to manipulate image.', + err: err, + code: 'IMAGE_PROCESSING' + }); + }); +}; + +module.exports.process = process; diff --git a/core/server/web/api/routes.js b/core/server/web/api/routes.js index 26ca88b74f..45b4a3c99b 100644 --- a/core/server/web/api/routes.js +++ b/core/server/web/api/routes.js @@ -14,6 +14,7 @@ const express = require('express'), tmpdir = require('os').tmpdir, upload = require('multer')({dest: tmpdir()}), validation = require('../middleware/validation'), + image = require('../middleware/image'), // Temporary // @TODO find a more appy way to do this! @@ -182,6 +183,7 @@ module.exports = function apiRoutes() { mw.authenticatePrivate, upload.single('uploadimage'), validation.upload({type: 'images'}), + image.normalize, api.http(api.uploads.add) ); diff --git a/core/server/web/middleware/image/index.js b/core/server/web/middleware/image/index.js new file mode 100644 index 0000000000..f5bb85b7f3 --- /dev/null +++ b/core/server/web/middleware/image/index.js @@ -0,0 +1,5 @@ +module.exports = { + get normalize() { + return require('./normalize'); + } +}; diff --git a/core/server/web/middleware/image/normalize.js b/core/server/web/middleware/image/normalize.js new file mode 100644 index 0000000000..71b5873b0e --- /dev/null +++ b/core/server/web/middleware/image/normalize.js @@ -0,0 +1,43 @@ +const _ = require('lodash'); +const path = require('path'); +const config = require('../../../config'); +const common = require('../../../lib/common'); +const image = require('../../../lib/image'); + +module.exports = function normalize(req, res, next) { + const imageOptimizationOptions = config.get('imageOptimization'); + + if (!imageOptimizationOptions.resize) { + return next(); + } + + const out = `${req.file.path}_processed`; + const originalPath = req.file.path; + + const options = Object.assign({ + in: originalPath, + out, + ext: req.file.ext, + width: 2000 + }, imageOptimizationOptions); + + image.manipulator.process(options) + .then(() => { + req.files = []; + + // CASE: push the processed/optimised image + req.files.push(Object.assign(req.file, {path: out})); + + // CASE: push original image, we keep a copy of it + const parsedFileName = path.parse(req.file.name); + const newName = `${parsedFileName.name}_o${parsedFileName.ext}`; + req.files.push(Object.assign(_.cloneDeep(req.file), {path: originalPath, name: newName})); + + next(); + }) + .catch((err) => { + err.context = `${req.file.name} / ${req.file.type}`; + common.logging.error(err); + next(); + }); +}; diff --git a/core/server/web/middleware/validation/upload.js b/core/server/web/middleware/validation/upload.js index 6a1aea974c..82faa07e71 100644 --- a/core/server/web/middleware/validation/upload.js +++ b/core/server/web/middleware/validation/upload.js @@ -1,6 +1,7 @@ -const common = require('../../../lib/common'), - config = require('../../../config'), - localUtils = require('../../utils'); +const path = require('path'); +const common = require('../../../lib/common'); +const config = require('../../../config'); +const localUtils = require('../../utils'); module.exports = function upload(options) { var type = options.type; @@ -21,6 +22,8 @@ module.exports = function upload(options) { })); } + req.file.ext = path.extname(req.file.name).toLowerCase(); + // Check if the file is valid if (!localUtils.checkFileIsValid(req.file, contentTypes, extensions)) { return next(new common.errors.UnsupportedMediaTypeError({ diff --git a/core/server/web/utils.js b/core/server/web/utils.js index 087bec8f51..c2d8d19b93 100644 --- a/core/server/web/utils.js +++ b/core/server/web/utils.js @@ -1,5 +1,4 @@ const url = require('url'), - path = require('path'), _ = require('lodash'); let _private = {}; @@ -37,10 +36,9 @@ module.exports.checkFileExists = function checkFileExists(fileData) { }; module.exports.checkFileIsValid = function checkFileIsValid(fileData, types, extensions) { - var type = fileData.mimetype, - ext = path.extname(fileData.name).toLowerCase(); + const type = fileData.mimetype; - if (_.includes(types, type) && _.includes(extensions, ext)) { + if (_.includes(types, type) && _.includes(extensions, fileData.ext)) { return true; } return false; diff --git a/core/test/unit/lib/image/manipulator_spec.js b/core/test/unit/lib/image/manipulator_spec.js new file mode 100644 index 0000000000..c9fe6be047 --- /dev/null +++ b/core/test/unit/lib/image/manipulator_spec.js @@ -0,0 +1,97 @@ +const should = require('should'); +const sinon = require('sinon'); +const common = require('../../../../server/lib/common'); +const manipulator = require('../../../../server/lib/image/manipulator'); +const testUtils = require('../../../utils'); +const sandbox = sinon.sandbox.create(); + +describe('lib/image: manipulator', function () { + afterEach(function () { + sandbox.restore(); + testUtils.unmockNotExistingModule(); + }); + + describe('cases', function () { + let sharp, sharpInstance; + + beforeEach(function () { + sharpInstance = { + metadata: sandbox.stub(), + resize: sandbox.stub(), + rotate: sandbox.stub(), + toFile: sandbox.stub() + }; + + sharp = sandbox.stub().callsFake(() => { + return sharpInstance; + }); + + sharp.cache = sandbox.stub(); + testUtils.mockNotExistingModule('sharp', sharp); + }); + + it('resize image', function () { + sharpInstance.metadata.resolves({width: 2000, height: 2000}); + sharpInstance.toFile.resolves(); + + return manipulator.process({width: 1000}) + .then(() => { + sharp.cache.calledOnce.should.be.true(); + sharpInstance.metadata.calledOnce.should.be.true(); + sharpInstance.toFile.calledOnce.should.be.true(); + sharpInstance.resize.calledOnce.should.be.true(); + sharpInstance.rotate.calledOnce.should.be.true(); + }); + }); + + it('skip resizing if image is too small', function () { + sharpInstance.metadata.resolves({width: 50, height: 50}); + sharpInstance.toFile.resolves(); + + return manipulator.process({width: 1000}) + .then(() => { + sharp.cache.calledOnce.should.be.true(); + sharpInstance.metadata.calledOnce.should.be.true(); + sharpInstance.toFile.calledOnce.should.be.true(); + sharpInstance.resize.calledOnce.should.be.false(); + sharpInstance.rotate.calledOnce.should.be.true(); + }); + }); + + it('sharp throws error during processing', function () { + sharpInstance.metadata.resolves({width: 500, height: 500}); + sharpInstance.toFile.rejects(new Error('whoops')); + + return manipulator.process({width: 2000}) + .then(() => { + '1'.should.eql(1, 'Expected to fail'); + }) + .catch((err) => { + (err instanceof common.errors.InternalServerError).should.be.true; + err.code.should.eql('IMAGE_PROCESSING'); + sharp.cache.calledOnce.should.be.true; + sharpInstance.metadata.calledOnce.should.be.true(); + sharpInstance.toFile.calledOnce.should.be.true(); + sharpInstance.resize.calledOnce.should.be.false(); + sharpInstance.rotate.calledOnce.should.be.true(); + }); + }); + }); + + describe('installation', function () { + beforeEach(function () { + testUtils.mockNotExistingModule('sharp', new Error(), true); + }); + + it('sharp was not installed', function () { + return manipulator.process() + .then(() => { + '1'.should.eql(1, 'Expected to fail'); + }) + .catch((err) => { + (err instanceof common.errors.InternalServerError).should.be.true(); + err.code.should.eql('SHARP_INSTALLATION'); + }); + }); + }); +}); diff --git a/core/test/unit/web/middleware/image/normalize_spec.js b/core/test/unit/web/middleware/image/normalize_spec.js new file mode 100644 index 0000000000..195d45cae1 --- /dev/null +++ b/core/test/unit/web/middleware/image/normalize_spec.js @@ -0,0 +1,71 @@ +const should = require('should'); +const sinon = require('sinon'); +const configUtils = require('../../../../utils/configUtils'); +const image = require('../../../../../server/lib/image'); +const common = require('../../../../../server/lib/common'); +const normalize = require('../../../../../server/web/middleware/image/normalize'); + +const sandbox = sinon.sandbox.create(); + +describe('normalize', function () { + let res, req; + + beforeEach(function () { + req = { + file: { + name: 'test', + path: '/test/path' + } + }; + + sandbox.stub(image.manipulator, 'process'); + sandbox.stub(common.logging, 'error'); + }); + + afterEach(function () { + sandbox.restore(); + configUtils.restore(); + }); + + it('should do manipulation by default', function (done) { + image.manipulator.process.resolves(); + + normalize(req, res, () => { + image.manipulator.process.calledOnce.should.be.true(); + done(); + }); + }); + + it('should add files array to request object with original and processed files', function (done) { + image.manipulator.process.resolves(); + + normalize(req, res, () => { + req.files.length.should.be.equal(2); + done(); + }); + }); + + it('should not do manipulation without resize flag set', function (done) { + configUtils.set({ + imageOptimization: { + resize: false, + } + }); + + normalize(req, res, () => { + image.manipulator.process.called.should.be.false(); + done(); + }); + }); + + it('should call manipulation when resize flag is explicitly set', function (done) { + image.manipulator.process.rejects(); + + normalize(req, res, ()=> { + common.logging.error.calledOnce.should.be.true(); + req.file.should.not.be.equal(undefined); + should.not.exist(req.files); + done(); + }); + }); +}); diff --git a/core/test/unit/web/utils_spec.js b/core/test/unit/web/utils_spec.js index ebb60ee7b6..5364804861 100644 --- a/core/test/unit/web/utils_spec.js +++ b/core/test/unit/web/utils_spec.js @@ -18,10 +18,16 @@ describe('web utils', function () { describe('checkFileIsValid', function () { it('returns true if file has valid extension and type', function () { - webUtils.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['text'], ['.txt']).should.be.true(); + webUtils.checkFileIsValid({ + name: 'test.txt', + mimetype: 'text', + ext: '.txt' + }, ['text'], ['.txt']).should.be.true(); + webUtils.checkFileIsValid({ name: 'test.jpg', - mimetype: 'jpeg' + mimetype: 'jpeg', + ext: '.jpg' }, ['text', 'jpeg'], ['.txt', '.jpg']).should.be.true(); }); diff --git a/core/test/utils/index.js b/core/test/utils/index.js index 3065f8351b..a39f8849f0 100644 --- a/core/test/utils/index.js +++ b/core/test/utils/index.js @@ -844,9 +844,13 @@ teardown = function teardown() { * we start with a small function set to mock non existent modules */ originalRequireFn = Module.prototype.require; -mockNotExistingModule = function mockNotExistingModule(modulePath, module) { +mockNotExistingModule = function mockNotExistingModule(modulePath, module, error = false) { Module.prototype.require = function (path) { if (path.match(modulePath)) { + if (error) { + throw module; + } + return module; } diff --git a/package.json b/package.json index 6528aab011..43943154d0 100644 --- a/package.json +++ b/package.json @@ -98,6 +98,7 @@ "xml": "1.0.1" }, "optionalDependencies": { + "sharp": "0.20.7", "sqlite3": "4.0.1" }, "devDependencies": { diff --git a/yarn.lock b/yarn.lock index 6bd24ba2d4..a4df63c92f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -931,7 +931,13 @@ color-convert@^1.3.0, color-convert@^1.9.0: dependencies: color-name "^1.1.1" -color-name@^1.0.0, color-name@^1.1.1: +color-convert@^1.9.1: + version "1.9.3" + resolved "https://registry.yarnpkg.com/color-convert/-/color-convert-1.9.3.tgz#bb71850690e1f136567de629d2d5471deda4c1e8" + dependencies: + color-name "1.1.3" + +color-name@1.1.3, color-name@^1.0.0, color-name@^1.1.1: version "1.1.3" resolved "https://registry.yarnpkg.com/color-name/-/color-name-1.1.3.tgz#a7d0558bd89c42f795dd42328f740831ca53bc25" @@ -941,6 +947,13 @@ color-string@^0.3.0: dependencies: color-name "^1.0.0" +color-string@^1.5.2: + version "1.5.3" + resolved "https://registry.yarnpkg.com/color-string/-/color-string-1.5.3.tgz#c9bbc5f01b58b5492f3d6857459cb6590ce204cc" + dependencies: + color-name "^1.0.0" + simple-swizzle "^0.2.2" + color@^0.11.0: version "0.11.4" resolved "https://registry.yarnpkg.com/color/-/color-0.11.4.tgz#6d7b5c74fb65e841cd48792ad1ed5e07b904d764" @@ -949,6 +962,13 @@ color@^0.11.0: color-convert "^1.3.0" color-string "^0.3.0" +color@^3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/color/-/color-3.0.0.tgz#d920b4328d534a3ac8295d68f7bd4ba6c427be9a" + dependencies: + color-convert "^1.9.1" + color-string "^1.5.2" + colormin@^1.0.5: version "1.1.2" resolved "https://registry.yarnpkg.com/colormin/-/colormin-1.1.2.tgz#ea2f7420a72b96881a38aae59ec124a6f7298133" @@ -2021,6 +2041,10 @@ fs-constants@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/fs-constants/-/fs-constants-1.0.0.tgz#6be0de9be998ce16af8afc24497b9ee9b7ccd9ad" +fs-copy-file-sync@^1.1.1: + version "1.1.1" + resolved "https://registry.yarnpkg.com/fs-copy-file-sync/-/fs-copy-file-sync-1.1.1.tgz#11bf32c096c10d126e5f6b36d06eece776062918" + fs-extra@3.0.1, fs-extra@^3.0.1: version "3.0.1" resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-3.0.1.tgz#3794f378c58b342ea7dbbb23095109c4b3b62291" @@ -2929,6 +2953,10 @@ is-arrayish@^0.2.1: version "0.2.1" resolved "https://registry.yarnpkg.com/is-arrayish/-/is-arrayish-0.2.1.tgz#77c99840527aa8ecb1a8ba697b80645a7a926a9d" +is-arrayish@^0.3.1: + version "0.3.2" + resolved "https://registry.yarnpkg.com/is-arrayish/-/is-arrayish-0.3.2.tgz#4574a2ae56f7ab206896fb431eaeed066fdf8f03" + is-buffer@^1.1.5: version "1.1.6" resolved "https://registry.yarnpkg.com/is-buffer/-/is-buffer-1.1.6.tgz#efaa2ea9daa0d7ab2ea13a97b2b8ad51fefbe8be" @@ -4228,7 +4256,7 @@ npm-run-path@^2.0.0: dependencies: path-key "^2.0.0" -"npmlog@0 || 1 || 2 || 3 || 4", npmlog@^4.0.1, npmlog@^4.0.2: +"npmlog@0 || 1 || 2 || 3 || 4", npmlog@^4.0.1, npmlog@^4.0.2, npmlog@^4.1.2: version "4.1.2" resolved "https://registry.yarnpkg.com/npmlog/-/npmlog-4.1.2.tgz#08a7f2a8bf734604779a9efa4ad5cc717abb954b" dependencies: @@ -4802,6 +4830,26 @@ prebuild-install@^2.3.0: tunnel-agent "^0.6.0" which-pm-runs "^1.0.0" +prebuild-install@^4.0.0: + version "4.0.0" + resolved "https://registry.yarnpkg.com/prebuild-install/-/prebuild-install-4.0.0.tgz#206ce8106ce5efa4b6cf062fc8a0a7d93c17f3a8" + dependencies: + detect-libc "^1.0.3" + expand-template "^1.0.2" + github-from-package "0.0.0" + minimist "^1.2.0" + mkdirp "^0.5.1" + node-abi "^2.2.0" + noop-logger "^0.1.1" + npmlog "^4.0.1" + os-homedir "^1.0.1" + pump "^2.0.1" + rc "^1.1.6" + simple-get "^2.7.0" + tar-fs "^1.13.0" + tunnel-agent "^0.6.0" + which-pm-runs "^1.0.0" + prelude-ls@~1.1.2: version "1.1.2" resolved "https://registry.yarnpkg.com/prelude-ls/-/prelude-ls-1.1.2.tgz#21932a549f5e52ffd9a827f570e04be62a97da54" @@ -5286,6 +5334,10 @@ secure-keys@^1.0.0: version "5.5.0" resolved "https://registry.yarnpkg.com/semver/-/semver-5.5.0.tgz#dc4bbc7a6ca9d916dee5d43516f0092b58f7b8ab" +semver@^5.5.1: + version "5.5.1" + resolved "https://registry.yarnpkg.com/semver/-/semver-5.5.1.tgz#7dfdd8814bdb7cabc7be0fb1d734cfb66c940477" + semver@~5.3.0: version "5.3.0" resolved "https://registry.yarnpkg.com/semver/-/semver-5.3.0.tgz#9b2ce5d3de02d17c6012ad326aa6b4d0cf54f94f" @@ -5351,6 +5403,21 @@ setprototypeof@1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/setprototypeof/-/setprototypeof-1.1.0.tgz#d0bd85536887b6fe7c0d818cb962d9d91c54e656" +sharp@0.20.7: + version "0.20.7" + resolved "https://registry.yarnpkg.com/sharp/-/sharp-0.20.7.tgz#d6e1abbe91453e2200090043f91a5cd345ee95cf" + dependencies: + color "^3.0.0" + detect-libc "^1.0.3" + fs-copy-file-sync "^1.1.1" + nan "^2.10.0" + npmlog "^4.1.2" + prebuild-install "^4.0.0" + semver "^5.5.1" + simple-get "^2.8.1" + tar "^4.4.6" + tunnel-agent "^0.6.0" + shebang-command@^1.2.0: version "1.2.0" resolved "https://registry.yarnpkg.com/shebang-command/-/shebang-command-1.2.0.tgz#44aac65b695b03398968c39f363fee5deafdf1ea" @@ -5421,7 +5488,7 @@ simple-dom@0.3.2: version "0.3.2" resolved "https://registry.yarnpkg.com/simple-dom/-/simple-dom-0.3.2.tgz#0663d10f1556f1500551d518f56e3aba0871371d" -simple-get@^2.7.0: +simple-get@^2.7.0, simple-get@^2.8.1: version "2.8.1" resolved "https://registry.yarnpkg.com/simple-get/-/simple-get-2.8.1.tgz#0e22e91d4575d87620620bc91308d57a77f44b5d" dependencies: @@ -5433,6 +5500,12 @@ simple-html-tokenizer@0.5.5: version "0.5.5" resolved "https://registry.yarnpkg.com/simple-html-tokenizer/-/simple-html-tokenizer-0.5.5.tgz#110e63f2fe095e1f21f2f07e8c259a5ab6d6bebb" +simple-swizzle@^0.2.2: + version "0.2.2" + resolved "https://registry.yarnpkg.com/simple-swizzle/-/simple-swizzle-0.2.2.tgz#a4da6b635ffcccca33f70d17cb92592de95e557a" + dependencies: + is-arrayish "^0.3.1" + "simplesmtp@~0.2 || ~0.3.30", simplesmtp@~0.3.30: version "0.3.35" resolved "https://registry.yarnpkg.com/simplesmtp/-/simplesmtp-0.3.35.tgz#017b1eb8b26317ac36d2a2a8a932631880736a03" @@ -5853,6 +5926,18 @@ tar@^4: safe-buffer "^5.1.2" yallist "^3.0.2" +tar@^4.4.6: + version "4.4.6" + resolved "https://registry.yarnpkg.com/tar/-/tar-4.4.6.tgz#63110f09c00b4e60ac8bcfe1bf3c8660235fbc9b" + dependencies: + chownr "^1.0.1" + fs-minipass "^1.2.5" + minipass "^2.3.3" + minizlib "^1.1.0" + mkdirp "^0.5.0" + safe-buffer "^5.1.2" + yallist "^3.0.2" + tarn@^1.1.4: version "1.1.4" resolved "https://registry.yarnpkg.com/tarn/-/tarn-1.1.4.tgz#aeeb85964b1afa0bbf381359c1167df237c27b6a"