diff --git a/core/server/lib/image/manipulator.js b/core/server/lib/image/manipulator.js index 9051272d7b..f0ecf92c7b 100644 --- a/core/server/lib/image/manipulator.js +++ b/core/server/lib/image/manipulator.js @@ -8,60 +8,20 @@ const fs = require('fs-extra'); * 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, originalData, originalSize; - - try { - sharp = require('sharp'); - } catch (err) { - return Promise.reject(new common.errors.InternalServerError({ - message: 'Sharp wasn\'t installed', - code: 'SHARP_INSTALLATION', - err: err - })); - } - - // @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); +const unsafeProcess = (options = {}) => { return fs.readFile(options.in) .then((data) => { - originalData = data; - - // @NOTE: have to use constructor with Buffer for sharp to be able to expose size property - img = sharp(data); - }) - .then(() => img.metadata()) - .then((metadata) => { - originalSize = metadata.size; - - 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(); - return img.toBuffer({resolveWithObject: true}); - }) - .then(({data, info}) => { - if (info.size > originalSize) { - return fs.writeFile(options.out, originalData); - } else { - return fs.writeFile(options.out, data); - } - }) - .catch((err) => { - throw new common.errors.InternalServerError({ - message: 'Unable to manipulate image.', - err: err, - code: 'IMAGE_PROCESSING' + return unsafeResizeImage(data, { + width: options.width }); + }) + .then((data) => { + return fs.writeFile(options.out, data); }); }; -const resizeImage = (originalBuffer, {width, height} = {}) => { +const unsafeResizeImage = (originalBuffer, {width, height} = {}) => { const sharp = require('sharp'); return sharp(originalBuffer) .resize(width, height, { @@ -76,12 +36,24 @@ const resizeImage = (originalBuffer, {width, height} = {}) => { }); }; -module.exports.process = process; -module.exports.safeResizeImage = (buffer, options) => { +const makeSafe = fn => (...args) => { try { require('sharp'); - return resizeImage(buffer, options); - } catch (e) { - return Promise.resolve(buffer); + } catch (err) { + return Promise.reject(new common.errors.InternalServerError({ + message: 'Sharp wasn\'t installed', + code: 'SHARP_INSTALLATION', + err: err + })); } + return fn(...args).catch((err) => { + throw new common.errors.InternalServerError({ + message: 'Unable to manipulate image.', + err: err, + code: 'IMAGE_PROCESSING' + }); + }); }; + +module.exports.process = makeSafe(unsafeProcess); +module.exports.resizeImage = makeSafe(unsafeResizeImage); diff --git a/core/server/web/shared/middlewares/image/handle-image-sizes.js b/core/server/web/shared/middlewares/image/handle-image-sizes.js index 169c3e8f73..0a66c09385 100644 --- a/core/server/web/shared/middlewares/image/handle-image-sizes.js +++ b/core/server/web/shared/middlewares/image/handle-image-sizes.js @@ -51,12 +51,17 @@ module.exports = function (req, res, next) { return storageInstance.read({path: originalImagePath}) .then((originalImageBuffer) => { - return image.manipulator.safeResizeImage(originalImageBuffer, imageDimensionConfig); + return image.manipulator.resizeImage(originalImageBuffer, imageDimensionConfig); }) .then((resizedImageBuffer) => { return storageInstance.saveRaw(resizedImageBuffer, req.url); }); }).then(() => { next(); - }).catch(next); + }).catch(function (err) { + if (err.code === 'SHARP_INSTALLATION') { + return redirectToOriginal(); + } + next(err); + }); }; diff --git a/core/test/unit/lib/image/manipulator_spec.js b/core/test/unit/lib/image/manipulator_spec.js index 939d77983b..7d1931f304 100644 --- a/core/test/unit/lib/image/manipulator_spec.js +++ b/core/test/unit/lib/image/manipulator_spec.js @@ -20,9 +20,8 @@ describe('lib/image: manipulator', function () { sandbox.stub(fs, 'writeFile').resolves(); sharpInstance = { - metadata: sandbox.stub(), - resize: sandbox.stub(), - rotate: sandbox.stub(), + resize: sandbox.stub().returnsThis(), + rotate: sandbox.stub().returnsThis(), toBuffer: sandbox.stub(), }; @@ -30,23 +29,14 @@ describe('lib/image: manipulator', function () { return sharpInstance; }); - sharp.cache = sandbox.stub(); testUtils.mockNotExistingModule('sharp', sharp); }); it('resize image', function () { - sharpInstance.metadata.resolves({width: 2000, height: 2000}); - sharpInstance.toBuffer.resolves({ - data: 'manipulated', - info: { - size: 42 - } - }); + sharpInstance.toBuffer.resolves('manipulated'); return manipulator.process({width: 1000}) .then(() => { - sharp.cache.calledOnce.should.be.true(); - sharpInstance.metadata.calledOnce.should.be.true(); sharpInstance.resize.calledOnce.should.be.true(); sharpInstance.rotate.calledOnce.should.be.true(); @@ -56,20 +46,14 @@ describe('lib/image: manipulator', function () { }); it('skip resizing if image is too small', function () { - sharpInstance.metadata.resolves({width: 50, height: 50}); - sharpInstance.toBuffer.resolves({ - data: 'manipulated', - info: { - size: 42 - } - }); + sharpInstance.toBuffer.resolves('manipulated'); return manipulator.process({width: 1000}) .then(() => { - sharp.cache.calledOnce.should.be.true(); - sharpInstance.metadata.calledOnce.should.be.true(); - sharpInstance.resize.calledOnce.should.be.false(); - sharpInstance.rotate.calledOnce.should.be.true(); + sharpInstance.resize.calledOnce.should.be.true(); + should.deepEqual(sharpInstance.resize.args[0][2], { + withoutEnlargement: true + }); fs.writeFile.calledOnce.should.be.true(); fs.writeFile.calledWith('manipulated'); @@ -77,22 +61,10 @@ describe('lib/image: manipulator', function () { }); it('uses original image as an output when the size (bytes) is bigger after manipulation', function () { - sharpInstance.metadata.resolves({ - width: 2000, - size: 10 - }); - - sharpInstance.toBuffer.resolves({ - data: 'manipulated', - info: { - size: 42 - } - }); + sharpInstance.toBuffer.resolves('manipulated to a very very very very very very very large size'); return manipulator.process({width: 1000}) .then(() => { - sharp.cache.calledOnce.should.be.true(); - sharpInstance.metadata.calledOnce.should.be.true(); sharpInstance.resize.calledOnce.should.be.true(); sharpInstance.rotate.calledOnce.should.be.true(); sharpInstance.toBuffer.calledOnce.should.be.true(); @@ -103,13 +75,7 @@ describe('lib/image: manipulator', function () { }); it('sharp throws error during processing', function () { - sharpInstance.metadata.resolves({width: 500, height: 500}); - sharpInstance.toBuffer.resolves({ - data: 'manipulated', - info: { - size: 42 - } - }); + sharpInstance.toBuffer.resolves('manipulated'); fs.writeFile.rejects(new Error('whoops')); @@ -120,12 +86,6 @@ describe('lib/image: manipulator', function () { .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.resize.calledOnce.should.be.false(); - sharpInstance.rotate.calledOnce.should.be.true(); - - fs.writeFile.calledOnce.should.be.true(); }); }); });