diff --git a/core/server/lib/image/index.js b/core/server/lib/image/index.js index 34808a6998..8e03194470 100644 --- a/core/server/lib/image/index.js +++ b/core/server/lib/image/index.js @@ -13,9 +13,5 @@ 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 deleted file mode 100644 index 2327e6a48e..0000000000 --- a/core/server/lib/image/manipulator.js +++ /dev/null @@ -1,66 +0,0 @@ -const Promise = require('bluebird'); -const errors = require('@tryghost/errors'); -const fs = require('fs-extra'); - -/** - * @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 unsafeProcess = (options = {}) => { - return fs.readFile(options.in) - .then((data) => { - return unsafeResizeImage(data, { - width: options.width - }); - }) - .then((data) => { - return fs.writeFile(options.out, data); - }); -}; - -const unsafeResizeImage = (originalBuffer, {width, height} = {}) => { - const sharp = require('sharp'); - return sharp(originalBuffer) - .resize(width, height, { - // CASE: dont make the image bigger than it was - withoutEnlargement: true - }) - // CASE: Automatically remove metadata and rotate based on the orientation. - .rotate() - .toBuffer() - .then((resizedBuffer) => { - return resizedBuffer.length < originalBuffer.length ? resizedBuffer : originalBuffer; - }); -}; - -// NOTE: .gif optimization is currently not supported by sharp but will be soon -// as there has been support added in underlying libvips library https://github.com/lovell/sharp/issues/1372 -// As for .svg files, sharp only supports conversion to png, and this does not -// play well with animated svg files -const canTransformFileExtension = ext => !['.gif', '.svg', '.svgz', '.ico'].includes(ext); - -const makeSafe = fn => (...args) => { - try { - require('sharp'); - } catch (err) { - return Promise.reject(new errors.InternalServerError({ - message: 'Sharp wasn\'t installed', - code: 'SHARP_INSTALLATION', - err: err - })); - } - return fn(...args).catch((err) => { - throw new errors.InternalServerError({ - message: 'Unable to manipulate image.', - err: err, - code: 'IMAGE_PROCESSING' - }); - }); -}; - -module.exports.canTransformFileExtension = canTransformFileExtension; -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 626bda0694..b39369746e 100644 --- a/core/server/web/shared/middlewares/image/handle-image-sizes.js +++ b/core/server/web/shared/middlewares/image/handle-image-sizes.js @@ -1,5 +1,5 @@ const path = require('path'); -const image = require('../../../../lib/image'); +const imageTransform = require('@tryghost/image-transform'); const storage = require('../../../../adapters/storage'); const activeTheme = require('../../../../../frontend/services/themes/active'); @@ -28,8 +28,8 @@ module.exports = function (req, res, next) { return next(); } - // CASE: image manipulator is uncapable of transforming file (e.g. .gif) - if (!image.manipulator.canTransformFileExtension(requestUrlFileExtension)) { + // CASE: image transform is not capable of transforming file (e.g. .gif) + if (!imageTransform.canTransformFileExtension(requestUrlFileExtension)) { return redirectToOriginal(); } @@ -86,7 +86,7 @@ module.exports = function (req, res, next) { return storageInstance.read({path}); }) .then((originalImageBuffer) => { - return image.manipulator.resizeImage(originalImageBuffer, imageDimensionConfig); + return imageTransform.resizeFromBuffer(originalImageBuffer, imageDimensionConfig); }) .then((resizedImageBuffer) => { return storageInstance.saveRaw(resizedImageBuffer, req.url); diff --git a/core/server/web/shared/middlewares/image/normalize.js b/core/server/web/shared/middlewares/image/normalize.js index 846dde6c02..a0e4a1c723 100644 --- a/core/server/web/shared/middlewares/image/normalize.js +++ b/core/server/web/shared/middlewares/image/normalize.js @@ -1,14 +1,14 @@ const cloneDeep = require('lodash/cloneDeep'); const path = require('path'); const config = require('../../../../config'); -const common = require('../../../../lib/common'); -const image = require('../../../../lib/image'); +const {logging} = require('../../../../lib/common'); +const imageTransform = require('@tryghost/image-transform'); module.exports = function normalize(req, res, next) { const imageOptimizationOptions = config.get('imageOptimization'); - // CASE: image manipulator is uncapable of transforming file (e.g. .gif) - if (!image.manipulator.canTransformFileExtension(req.file.ext) || !imageOptimizationOptions.resize) { + // CASE: image transform is not capable of transforming file (e.g. .gif) + if (!imageTransform.canTransformFileExtension(req.file.ext) || !imageOptimizationOptions.resize) { return next(); } @@ -22,7 +22,7 @@ module.exports = function normalize(req, res, next) { width: 2000 }, imageOptimizationOptions); - image.manipulator.process(options) + imageTransform.resizeFromPath(options) .then(() => { req.files = []; @@ -38,7 +38,7 @@ module.exports = function normalize(req, res, next) { }) .catch((err) => { err.context = `${req.file.name} / ${req.file.type}`; - common.logging.error(err); + logging.error(err); next(); }); }; diff --git a/core/test/unit/lib/image/manipulator_spec.js b/core/test/unit/lib/image/manipulator_spec.js deleted file mode 100644 index 8a3062f978..0000000000 --- a/core/test/unit/lib/image/manipulator_spec.js +++ /dev/null @@ -1,135 +0,0 @@ -const should = require('should'); -const sinon = require('sinon'); -const fs = require('fs-extra'); -const errors = require('@tryghost/errors'); -const manipulator = require('../../../../server/lib/image/manipulator'); -const mockUtils = require('../../../utils/mocks'); - -describe('lib/image: manipulator', function () { - afterEach(function () { - sinon.restore(); - mockUtils.modules.unmockNonExistentModule(); - }); - - describe('canTransformFileExtension', function () { - it('returns false for ".gif"', function () { - should.equal( - manipulator.canTransformFileExtension('.gif'), - false - ); - }); - it('returns false for ".svg"', function () { - should.equal( - manipulator.canTransformFileExtension('.svg'), - false - ); - }); - it('returns false for ".svgz"', function () { - should.equal( - manipulator.canTransformFileExtension('.svgz'), - false - ); - }); - it('returns false for ".ico"', function () { - should.equal( - manipulator.canTransformFileExtension('.ico'), - false - ); - }); - }); - - describe('cases', function () { - let sharp, sharpInstance; - - beforeEach(function () { - sinon.stub(fs, 'readFile').resolves('original'); - sinon.stub(fs, 'writeFile').resolves(); - - sharpInstance = { - resize: sinon.stub().returnsThis(), - rotate: sinon.stub().returnsThis(), - toBuffer: sinon.stub() - }; - - sharp = sinon.stub().callsFake(() => { - return sharpInstance; - }); - - mockUtils.modules.mockNonExistentModule('sharp', sharp); - }); - - it('resize image', function () { - sharpInstance.toBuffer.resolves('manipulated'); - - return manipulator.process({width: 1000}) - .then(() => { - sharpInstance.resize.calledOnce.should.be.true(); - sharpInstance.rotate.calledOnce.should.be.true(); - - fs.writeFile.calledOnce.should.be.true(); - fs.writeFile.calledWith('manipulated'); - }); - }); - - it('skip resizing if image is too small', function () { - sharpInstance.toBuffer.resolves('manipulated'); - - return manipulator.process({width: 1000}) - .then(() => { - 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'); - }); - }); - - it('uses original image as an output when the size (bytes) is bigger after manipulation', function () { - sharpInstance.toBuffer.resolves('manipulated to a very very very very very very very large size'); - - return manipulator.process({width: 1000}) - .then(() => { - sharpInstance.resize.calledOnce.should.be.true(); - sharpInstance.rotate.calledOnce.should.be.true(); - sharpInstance.toBuffer.calledOnce.should.be.true(); - - fs.writeFile.calledOnce.should.be.true(); - fs.writeFile.calledWith('original'); - }); - }); - - it('sharp throws error during processing', function () { - sharpInstance.toBuffer.resolves('manipulated'); - - fs.writeFile.rejects(new Error('whoops')); - - return manipulator.process({width: 2000}) - .then(() => { - '1'.should.eql(1, 'Expected to fail'); - }) - .catch((err) => { - (err instanceof errors.InternalServerError).should.be.true; - err.code.should.eql('IMAGE_PROCESSING'); - }); - }); - }); - - describe('installation', function () { - beforeEach(function () { - mockUtils.modules.mockNonExistentModule('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 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 index 50db2ee459..d255cae939 100644 --- a/core/test/unit/web/middleware/image/normalize_spec.js +++ b/core/test/unit/web/middleware/image/normalize_spec.js @@ -1,8 +1,8 @@ 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 imageTransform = require('@tryghost/image-transform'); +const {logging} = require('../../../../../server/lib/common'); const normalize = require('../../../../../server/web/shared/middlewares/image/normalize'); describe('normalize', function () { @@ -17,8 +17,8 @@ describe('normalize', function () { } }; - sinon.stub(image.manipulator, 'process'); - sinon.stub(common.logging, 'error'); + sinon.stub(imageTransform, 'resizeFromPath'); + sinon.stub(logging, 'error'); }); afterEach(function () { @@ -27,16 +27,16 @@ describe('normalize', function () { }); it('should do manipulation by default', function (done) { - image.manipulator.process.resolves(); + imageTransform.resizeFromPath.resolves(); normalize(req, res, function () { - image.manipulator.process.calledOnce.should.be.true(); + imageTransform.resizeFromPath.calledOnce.should.be.true(); done(); }); }); - it('should add files array to request object with original and processed files', function (done) { - image.manipulator.process.resolves(); + it('should add files array to request object with original and resized files', function (done) { + imageTransform.resizeFromPath.resolves(); normalize(req, res, function () { req.files.length.should.be.equal(2); @@ -52,16 +52,16 @@ describe('normalize', function () { }); normalize(req, res, function () { - image.manipulator.process.called.should.be.false(); + imageTransform.resizeFromPath.called.should.be.false(); done(); }); }); - it('should not create files array when processing fails', function (done) { - image.manipulator.process.rejects(); + it('should not create files array when resizing fails', function (done) { + imageTransform.resizeFromPath.rejects(); normalize(req, res, () => { - common.logging.error.calledOnce.should.be.true(); + logging.error.calledOnce.should.be.true(); req.file.should.not.be.equal(undefined); should.not.exist(req.files); done(); @@ -69,7 +69,7 @@ describe('normalize', function () { }); ['.gif', '.svg', '.svgz'].forEach(function (extension) { - it(`should skip processing when file extension is ${extension}`, function (done) { + it(`should skip resizing when file extension is ${extension}`, function (done) { req.file.ext = extension; normalize(req, res, function () { req.file.should.not.be.equal(undefined); diff --git a/package.json b/package.json index 19f31b51fd..0a1806ac5e 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "@sentry/node": "5.15.0", "@tryghost/errors": "^0.1.0", "@tryghost/helpers": "1.1.23", + "@tryghost/image-transform": "^0.1.0", "@tryghost/kg-markdown-html-renderer": "1.0.0", "@tryghost/members-api": "0.18.0", "@tryghost/members-ssr": "0.7.4", @@ -128,7 +129,6 @@ }, "optionalDependencies": { "@tryghost/html-to-mobiledoc": "0.6.4", - "sharp": "0.25.2", "sqlite3": "4.1.1" }, "devDependencies": { diff --git a/yarn.lock b/yarn.lock index d114d8c5b4..615b25496e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -298,6 +298,14 @@ dependencies: ghost-ignition "^4.1.0" +"@tryghost/errors@^0.1.1": + version "0.1.1" + resolved "https://registry.yarnpkg.com/@tryghost/errors/-/errors-0.1.1.tgz#c140f8558fc54cf69ac7e7ea719a189c763fea04" + integrity sha512-8gmWGDLbwGhrMJ6psn8uubanntfxCvywufzJyDFgB/Kjh6bjd2CYEL84nA3rzfnxLpuY7vL/i4sFjpF1MZ15KA== + dependencies: + ghost-ignition "^4.1.0" + lodash "^4.17.15" + "@tryghost/extract-zip@1.6.6", "@tryghost/extract-zip@^1.6.6": version "1.6.6" resolved "https://registry.yarnpkg.com/@tryghost/extract-zip/-/extract-zip-1.6.6.tgz#937e0e775fec6dea937ac49d73a068bcafb67f50" @@ -324,6 +332,17 @@ "@tryghost/mobiledoc-kit" "^0.12.4-ghost.1" jsdom "16.2.0" +"@tryghost/image-transform@^0.1.0": + version "0.1.0" + resolved "https://registry.yarnpkg.com/@tryghost/image-transform/-/image-transform-0.1.0.tgz#d59c3cb41915fd5596e4d8f90f5a8bb44f19edf9" + integrity sha512-yqECkOUo0nnPK7jdxvRF2B12jR2YfCpAzZdreHO3HIhVMMBN6vgPVxIsTd7Kh2cXpuWpBWS+0eAQ8r6sf6vJfQ== + dependencies: + "@tryghost/errors" "^0.1.1" + bluebird "^3.7.2" + fs-extra "^9.0.0" + optionalDependencies: + sharp "^0.25.2" + "@tryghost/kg-clean-basic-html@^0.1.5": version "0.1.5" resolved "https://registry.yarnpkg.com/@tryghost/kg-clean-basic-html/-/kg-clean-basic-html-0.1.5.tgz#21cbe9036472c04b2320b1feb359c9c40ff3a300" @@ -1513,11 +1532,16 @@ chokidar@3.3.0: optionalDependencies: fsevents "~2.1.1" -chownr@^1.1.1, chownr@^1.1.3: +chownr@^1.1.1: version "1.1.3" resolved "https://registry.yarnpkg.com/chownr/-/chownr-1.1.3.tgz#42d837d5239688d55f303003a508230fa6727142" integrity sha512-i70fVHhmV3DtTl6nqvZOnIjbY0Pe4kAUjwHj8z0zAdgBtYrJyYwLKCCuRBQ5ppkyL0AkN7HKRnETdmdp1zqNXw== +chownr@^1.1.3: + version "1.1.4" + resolved "https://registry.yarnpkg.com/chownr/-/chownr-1.1.4.tgz#6fc9d7b42d32a583596337666e7d08084da2cc6b" + integrity sha512-jJ0bqzaylmJtVnNgzTeSOs8DPavpbYgEr/b0YL8/2GO3xJEhInFmhKMUnEJQjZumK7KXGFhUy89PrsJWlakBVg== + chrono-node@~1.4.3: version "1.4.3" resolved "https://registry.yarnpkg.com/chrono-node/-/chrono-node-1.4.3.tgz#4c8e24643ec5e576f6f8fe0429370c3b554491b4" @@ -3292,9 +3316,9 @@ fs-minipass@^1.2.5: minipass "^2.6.0" fs-minipass@^2.0.0: - version "2.0.0" - resolved "https://registry.yarnpkg.com/fs-minipass/-/fs-minipass-2.0.0.tgz#a6415edab02fae4b9e9230bc87ee2e4472003cd1" - integrity sha512-40Qz+LFXmd9tzYVnnBmZvFfvAADfUA14TXPK1s7IfElJTIZ97rA8w4Kin7Wt5JBrC3ShnnFJO/5vPjPEeJIq9A== + version "2.1.0" + resolved "https://registry.yarnpkg.com/fs-minipass/-/fs-minipass-2.1.0.tgz#7f5036fdbf12c63c169190cbe4199c852271f9fb" + integrity sha512-V/JgOLFCS+R6Vcq0slCuaeWEdNC3ouDlJMNIsacH2VtALiu9mV4LPrHc5cDl8k5aw6J8jwgWWpiTo5RYhmIzvg== dependencies: minipass "^3.0.0" @@ -7848,7 +7872,7 @@ setprototypeof@1.1.1: resolved "https://registry.yarnpkg.com/setprototypeof/-/setprototypeof-1.1.1.tgz#7e95acb24aa92f5885e0abef5ba131330d4ae683" integrity sha512-JvdAWfbXeIGaZ9cILp38HntZSFSo3mWg6xGcJJsd+d4aRMOqauag1C63dJfDw7OaMYwEbHMOxEZ1lqVRYP2OAw== -sharp@0.25.2: +sharp@^0.25.2: version "0.25.2" resolved "https://registry.yarnpkg.com/sharp/-/sharp-0.25.2.tgz#f9003d73be50e9265e98f79f04fe53d8c66a3967" integrity sha512-l1GN0kFNtJr3U9i9pt7a+vo2Ij0xv4tTKDIPx8W6G9WELhPwrMyZZJKAAQNBSI785XB4uZfS5Wpz8C9jWV4AFQ==