From 58536a63b350e1facae4323a73f739564156d7be Mon Sep 17 00:00:00 2001 From: Michael Barrett Date: Thu, 25 Apr 2024 11:45:14 +0100 Subject: [PATCH] Added timeout when resizing an image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs [ENG-827](https://linear.app/tryghost/issue/ENG-827/🐛-crash-on-resizing-animated-gif) Added a timeout to the image resizing middleware to prevent crashes when an image is taking too long to resize. When the timeout is reached and the image has not been resized, the middleware will return the original image --- .../web/middleware/handle-image-sizes.js | 12 ++- .../web/middleware/handle-image-sizes.test.js | 76 +++++++++++++++++-- yarn.lock | 31 +------- 3 files changed, 84 insertions(+), 35 deletions(-) diff --git a/ghost/core/core/frontend/web/middleware/handle-image-sizes.js b/ghost/core/core/frontend/web/middleware/handle-image-sizes.js index f565b87e80ce..1fcc68e2153e 100644 --- a/ghost/core/core/frontend/web/middleware/handle-image-sizes.js +++ b/ghost/core/core/frontend/web/middleware/handle-image-sizes.js @@ -9,9 +9,10 @@ const {imageSize} = require('../../../server/lib/image'); const SIZE_PATH_REGEX = /^\/size\/([^/]+)\//; const FORMAT_PATH_REGEX = /^\/format\/([^./]+)\//; - const TRAILING_SLASH_REGEX = /\/+$/; +const RESIZE_TIMEOUT_SECONDS = 10; + module.exports = function handleImageSizes(req, res, next) { // In admin we need to read images and calculate the average color (blocked by CORS otherwise) res.setHeader('Access-Control-Allow-Origin', '*'); @@ -123,7 +124,12 @@ module.exports = function handleImageSizes(req, res, next) { if (originalImageBuffer.length <= 0) { throw new NoContentError(); } - return imageTransform.resizeFromBuffer(originalImageBuffer, {withoutEnlargement: requestUrlFileExtension !== '.svg', ...imageDimensionConfig, format}); + return imageTransform.resizeFromBuffer(originalImageBuffer, { + withoutEnlargement: requestUrlFileExtension !== '.svg', + ...imageDimensionConfig, + format, + timeout: RESIZE_TIMEOUT_SECONDS + }); }) .then((resizedImageBuffer) => { return storageInstance.saveRaw(resizedImageBuffer, req.url); @@ -147,3 +153,5 @@ module.exports = function handleImageSizes(req, res, next) { next(err); }); }; + +module.exports.RESIZE_TIMEOUT_SECONDS = RESIZE_TIMEOUT_SECONDS; diff --git a/ghost/core/test/unit/frontend/web/middleware/handle-image-sizes.test.js b/ghost/core/test/unit/frontend/web/middleware/handle-image-sizes.test.js index bb5fe5b42291..d4b8e5a4550d 100644 --- a/ghost/core/test/unit/frontend/web/middleware/handle-image-sizes.test.js +++ b/ghost/core/test/unit/frontend/web/middleware/handle-image-sizes.test.js @@ -294,6 +294,47 @@ describe('handleImageSizes middleware', function () { }); }); + it('redirects if timeout is exceeded', function (done) { + sinon.stub(imageTransform, 'canTransformFiles').returns(true); + + dummyStorage.exists = async function () { + return false; + }; + + dummyStorage.read = async function () { + return buffer; + }; + + const error = new Error('Resize timeout'); + error.code = 'IMAGE_PROCESSING'; + + resizeFromBufferStub.throws(error); + + const fakeReq = { + url: '/size/w1000/blank.png', + originalUrl: '/blog/content/images/size/w1000/blank.png' + }; + + const fakeRes = { + redirect(url) { + try { + url.should.equal('/blog/content/images/blank.png'); + } catch (e) { + return done(e); + } + done(); + }, + setHeader() {} + }; + + handleImageSizes(fakeReq, fakeRes, function next(err) { + if (err) { + return done(err); + } + done(new Error('Should not have called next')); + }); + }); + it('continues if file exists', function (done) { dummyStorage.exists = async function (path) { if (path === '/size/w1000/blank.png') { @@ -526,7 +567,12 @@ describe('handleImageSizes middleware', function () { return done(err); } try { - resizeFromBufferStub.calledOnceWithExactly(buffer, {withoutEnlargement: false, width: 1000, format: 'png'}).should.be.true(); + resizeFromBufferStub.calledOnceWithExactly(buffer, { + withoutEnlargement: false, + width: 1000, + format: 'png', + timeout: handleImageSizes.RESIZE_TIMEOUT_SECONDS + }).should.be.true(); typeStub.calledOnceWithExactly('png').should.be.true(); } catch (e) { return done(e); @@ -561,7 +607,12 @@ describe('handleImageSizes middleware', function () { return done(err); } try { - resizeFromBufferStub.calledOnceWithExactly(buffer, {withoutEnlargement: true, width: 1000, format: 'webp'}).should.be.true(); + resizeFromBufferStub.calledOnceWithExactly(buffer, { + withoutEnlargement: true, + width: 1000, + format: 'webp', + timeout: handleImageSizes.RESIZE_TIMEOUT_SECONDS + }).should.be.true(); typeStub.calledOnceWithExactly('webp').should.be.true(); } catch (e) { return done(e); @@ -596,7 +647,12 @@ describe('handleImageSizes middleware', function () { return done(err); } try { - resizeFromBufferStub.calledOnceWithExactly(buffer, {withoutEnlargement: true, width: 1000, format: 'avif'}).should.be.true(); + resizeFromBufferStub.calledOnceWithExactly(buffer, { + withoutEnlargement: true, + width: 1000, + format: 'avif', + timeout: handleImageSizes.RESIZE_TIMEOUT_SECONDS + }).should.be.true(); typeStub.calledOnceWithExactly('image/avif').should.be.true(); } catch (e) { return done(e); @@ -631,7 +687,12 @@ describe('handleImageSizes middleware', function () { return done(err); } try { - resizeFromBufferStub.calledOnceWithExactly(buffer, {withoutEnlargement: true, width: 1000, format: 'webp'}).should.be.true(); + resizeFromBufferStub.calledOnceWithExactly(buffer, { + withoutEnlargement: true, + width: 1000, + format: 'webp', + timeout: handleImageSizes.RESIZE_TIMEOUT_SECONDS + }).should.be.true(); typeStub.calledOnceWithExactly('webp').should.be.true(); } catch (e) { return done(e); @@ -666,7 +727,12 @@ describe('handleImageSizes middleware', function () { return done(err); } try { - resizeFromBufferStub.calledOnceWithExactly(buffer, {withoutEnlargement: true, width: 1000, format: 'gif'}).should.be.true(); + resizeFromBufferStub.calledOnceWithExactly(buffer, { + withoutEnlargement: true, + width: 1000, + format: 'gif', + timeout: handleImageSizes.RESIZE_TIMEOUT_SECONDS + }).should.be.true(); typeStub.calledOnceWithExactly('gif').should.be.true(); } catch (e) { return done(e); diff --git a/yarn.lock b/yarn.lock index c71fb449d4e3..2c83103e024a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -28227,7 +28227,7 @@ string-template@~0.2.1: resolved "https://registry.yarnpkg.com/string-template/-/string-template-0.2.1.tgz#42932e598a352d01fc22ec3367d9d84eec6c9add" integrity sha512-Yptehjogou2xm4UJbxJ4CxgZx12HBfeystp0y3x7s4Dj32ltVVG1Gg8YhKjHZkHicuKpZX/ffilA8505VbUbpw== -"string-width-cjs@npm:string-width@^4.2.0": +"string-width-cjs@npm:string-width@^4.2.0", "string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -28245,15 +28245,6 @@ string-width@^1.0.1: is-fullwidth-code-point "^1.0.0" strip-ansi "^3.0.0" -"string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - string-width@^2.1.0: version "2.1.1" resolved "https://registry.yarnpkg.com/string-width/-/string-width-2.1.1.tgz#ab93f27a8dc13d28cac815c462143a6d9012ae9e" @@ -28342,7 +28333,7 @@ stringify-entities@^2.0.0: is-decimal "^1.0.2" is-hexadecimal "^1.0.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -28370,13 +28361,6 @@ strip-ansi@^5.1.0, strip-ansi@^5.2.0: dependencies: ansi-regex "^4.1.0" -strip-ansi@^6.0.0, strip-ansi@^6.0.1: - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - strip-ansi@^7.0.1: version "7.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.0.1.tgz#61740a08ce36b61e50e65653f07060d000975fb2" @@ -30819,7 +30803,7 @@ workerpool@^6.0.2, workerpool@^6.0.3, workerpool@^6.1.5, workerpool@^6.4.0: resolved "https://registry.yarnpkg.com/workerpool/-/workerpool-6.5.1.tgz#060f73b39d0caf97c6db64da004cd01b4c099544" integrity sha512-Fs4dNYcsdpYSAfVxhnl1L5zTksjvOJxtC5hzMNl+1t9B8hTJTdKDyZ5ju7ztgPy+ft9tBFXoOlDNiOT9WUXZlA== -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -30837,15 +30821,6 @@ wrap-ansi@^6.0.1: string-width "^4.1.0" strip-ansi "^6.0.0" -wrap-ansi@^7.0.0: - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - wrap-ansi@^8.0.1, wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214"