tqt/Ghost
2
1
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2023-12-13 21:00:40 +01:00
Code Issues 1 Releases Wiki Activity

Cleaned up image manipulator (#10282)

Browse source 
no-issue

* Added InternalServerError to resizeImage

* Added a redirect to original image if sharp is missing

* Improved naming - safeMethod -> method

* Updated process method to follow same sharp check pattern

* Refactor safety wrapper into makeSafe function

* Moved generic manipulation error to makeSafe function

* Refactored unsafeProcess to use unsafeResizeImage

* Removed CRAZY catch
This commit is contained in:
Fabien O'Carroll 2018-12-14 11:54:52 +07:00 committed by GitHub
parent 7099dd45a5
commit 2d92793b3f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 41 additions and 104 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

76
core/server/lib/image/manipulator.js
View file

@ -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);

9
core/server/web/shared/middlewares/image/handle-image-sizes.js
View file

@ -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);
});
};

60
core/test/unit/lib/image/manipulator_spec.js
View file

@ -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();
});
});
});