From 26d24174abe85592c2adb01ca2d004c59c1be494 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 19 Nov 2018 18:49:16 +0700 Subject: [PATCH 01/22] Implemented saveRaw method on local file storage --- .../adapters/storage/LocalFileStorage.js | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/core/server/adapters/storage/LocalFileStorage.js b/core/server/adapters/storage/LocalFileStorage.js index da46eb9d09cc..9cdbee300fd6 100644 --- a/core/server/adapters/storage/LocalFileStorage.js +++ b/core/server/adapters/storage/LocalFileStorage.js @@ -19,6 +19,31 @@ class LocalFileStore extends StorageBase { this.storagePath = config.getContentPath('images'); } + /** + * Saves a buffer in the targetPath + * - buffer is an instance of Buffer + * - returns a Promise with returns the full URL to retrieve the data + */ + saveRaw(buffer, targetPath) { + const storagePath = path.join(this.storagePath, targetPath); + const targetDir = path.dirname(storagePath); + + return fs.mkdirs(targetDir) + .then(() => { + return fs.writeFile(storagePath, buffer); + }) + .then(() => { + // For local file system storage can use relative path so add a slash + const fullUrl = ( + urlService.utils.urlJoin('/', urlService.utils.getSubdir(), + urlService.utils.STATIC_IMAGE_URL_PREFIX, + targetPath) + ).replace(new RegExp(`\\${path.sep}`, 'g'), '/'); + + return fullUrl; + }); + } + /** * Saves the image to storage (the file system) * - image is the express image object From 5f3e346f3b3ebd0108d875e516e1588cd51dc49c Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 19 Nov 2018 16:20:39 +0700 Subject: [PATCH 02/22] Added initial handleImageSizes middleware --- .../middlewares/image/handle-image-sizes.js | 50 +++++++++++++++++++ .../web/shared/middlewares/image/index.js | 3 ++ 2 files changed, 53 insertions(+) create mode 100644 core/server/web/shared/middlewares/image/handle-image-sizes.js diff --git a/core/server/web/shared/middlewares/image/handle-image-sizes.js b/core/server/web/shared/middlewares/image/handle-image-sizes.js new file mode 100644 index 000000000000..f3482bbc259e --- /dev/null +++ b/core/server/web/shared/middlewares/image/handle-image-sizes.js @@ -0,0 +1,50 @@ +const path = require('path'); +const sharp = require('sharp'); +const storage = require('../../../../adapters/storage'); +const activeTheme = require('../../../../services/themes/active'); + +const SIZE_PATH_REGEX = /^\/sizes\/([^/]+)\//; + +module.exports = function (req, res, next) { + const storageInstance = storage.getStorage(); + const imageSizes = activeTheme.get().config('image_sizes'); + + if (!SIZE_PATH_REGEX.test(req.url)) { + return next(); + } + + const [sizeImageDir, requestedSize] = req.url.match(SIZE_PATH_REGEX); + + // CASE: unknown size or missing size config + const imageSizeConfig = imageSizes[requestedSize]; + if (!imageSizeConfig || (!imageSizeConfig.width && !imageSizeConfig.height)) { + const url = req.originalUrl.replace(`/sizes/${requestedSize}`, ''); + return res.redirect(url); + } + + // CASE: unsupported storage adapter but theme is using custom image_sizes + if (typeof storageInstance.saveRaw !== 'function') { + const url = req.originalUrl.replace(`/sizes/${requestedSize}`, ''); + return res.redirect(url); + } + + storageInstance.exists(req.url).then((exists) => { + if (exists) { + return; + } + + const originalImagePath = path.relative(sizeImageDir, req.url); + + return storageInstance.read({path: originalImagePath}) + .then((originalImageBuffer) => { + return sharp(originalImageBuffer) + .resize(imageSizeConfig.width, imageSizeConfig.height) + .toBuffer(); + }) + .then((resizedImageBuffer) => { + return storageInstance.saveRaw(resizedImageBuffer, req.url); + }); + }).then(() => { + next(); + }).catch(next); +}; diff --git a/core/server/web/shared/middlewares/image/index.js b/core/server/web/shared/middlewares/image/index.js index f5bb85b7f30a..df419227b571 100644 --- a/core/server/web/shared/middlewares/image/index.js +++ b/core/server/web/shared/middlewares/image/index.js @@ -1,5 +1,8 @@ module.exports = { get normalize() { return require('./normalize'); + }, + get handleImageSizes() { + return require('./handle-image-sizes'); } }; From 20c78dab584e26a8082183fee11b635f64815c1e Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 26 Nov 2018 14:38:04 +0700 Subject: [PATCH 03/22] Wired up handleImageSizes middleware --- core/server/web/site/app.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/server/web/site/app.js b/core/server/web/site/app.js index 41ac6a202452..43ba05bb0fba 100644 --- a/core/server/web/site/app.js +++ b/core/server/web/site/app.js @@ -13,6 +13,8 @@ const themeMiddleware = require('../../services/themes').middleware; const siteRoutes = require('./routes'); const shared = require('../shared'); +const STATIC_IMAGE_URL_PREFIX = `/${urlService.utils.STATIC_IMAGE_URL_PREFIX}`; + let router; function SiteRouter(req, res, next) { @@ -58,7 +60,7 @@ module.exports = function setupSiteApp(options = {}) { siteApp.use(shared.middlewares.servePublicFile('public/404-ghost.png', 'png', constants.ONE_HOUR_S)); // Serve blog images using the storage adapter - siteApp.use('/' + urlService.utils.STATIC_IMAGE_URL_PREFIX, storage.getStorage().serve()); + siteApp.use(STATIC_IMAGE_URL_PREFIX, shared.middlewares.image.handleImageSizes, storage.getStorage().serve()); // @TODO find this a better home // We do this here, at the top level, because helpers require so much stuff. From 52fd7093953ea5da401b455229f22956ade2fb59 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 26 Nov 2018 14:49:27 +0700 Subject: [PATCH 04/22] Implemented delete for LocalFileStorage --- core/server/adapters/storage/LocalFileStorage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/server/adapters/storage/LocalFileStorage.js b/core/server/adapters/storage/LocalFileStorage.js index 9cdbee300fd6..c0c91f4abaed 100644 --- a/core/server/adapters/storage/LocalFileStorage.js +++ b/core/server/adapters/storage/LocalFileStorage.js @@ -135,8 +135,8 @@ class LocalFileStore extends StorageBase { * Not implemented. * @returns {Promise.<*>} */ - delete() { - return Promise.reject('not implemented'); + delete(fileName) { + return fs.remove(path.join(this.storagePath, fileName)); } /** From 274fc7e43fd25ea34a04beb54c9246da2bac034d Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 26 Nov 2018 14:50:02 +0700 Subject: [PATCH 05/22] Removed delete method from theme Storage class --- core/server/services/themes/Storage.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/server/services/themes/Storage.js b/core/server/services/themes/Storage.js index 50802521418e..d21cdde0f992 100644 --- a/core/server/services/themes/Storage.js +++ b/core/server/services/themes/Storage.js @@ -55,10 +55,6 @@ class ThemeStorage extends LocalFileStorage { }); }; } - - delete(fileName) { - return fs.remove(path.join(this.storagePath, fileName)); - } } module.exports = ThemeStorage; From 2b23205856540de26f5aa1a48ffa6eb2382a3c98 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 26 Nov 2018 15:00:55 +0700 Subject: [PATCH 06/22] Deleted sizes directory when theme is activated --- core/server/services/themes/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/server/services/themes/index.js b/core/server/services/themes/index.js index 4c9c9b578e8f..687f4c206817 100644 --- a/core/server/services/themes/index.js +++ b/core/server/services/themes/index.js @@ -7,6 +7,8 @@ var debug = require('ghost-ignition').debug('themes'), settingsCache = require('../settings/cache'), themeStorage; +const ImageStorage = require('../../adapters/storage'); + // @TODO: reduce the amount of things we expose to the outside world // Make this a nice clean sensible API we can all understand! module.exports = { @@ -85,6 +87,7 @@ module.exports = { try { active.set(loadedTheme, checkedTheme, error); common.events.emit('services.themes.activated'); + ImageStorage.getStorage().delete('sizes'); } catch (err) { common.logging.error(new common.errors.InternalServerError({ message: common.i18n.t('errors.middleware.themehandler.activateFailed', {theme: loadedTheme.name}), From 80d5fb84def832b5cab43b29b639b67933851df5 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 26 Nov 2018 18:09:49 +0700 Subject: [PATCH 07/22] Ensured that smaller images are not enlarged --- .../server/web/shared/middlewares/image/handle-image-sizes.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 f3482bbc259e..454ff04dadfd 100644 --- a/core/server/web/shared/middlewares/image/handle-image-sizes.js +++ b/core/server/web/shared/middlewares/image/handle-image-sizes.js @@ -38,7 +38,9 @@ module.exports = function (req, res, next) { return storageInstance.read({path: originalImagePath}) .then((originalImageBuffer) => { return sharp(originalImageBuffer) - .resize(imageSizeConfig.width, imageSizeConfig.height) + .resize(imageSizeConfig.width, imageSizeConfig.height, { + withoutEnlargement: true + }) .toBuffer(); }) .then((resizedImageBuffer) => { From 79f23718b85d61a3a572928596a5b87da1ad8484 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 3 Dec 2018 10:11:28 +0700 Subject: [PATCH 08/22] Renamed sizes -> size --- .../web/shared/middlewares/image/handle-image-sizes.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 454ff04dadfd..35a748a9be1a 100644 --- a/core/server/web/shared/middlewares/image/handle-image-sizes.js +++ b/core/server/web/shared/middlewares/image/handle-image-sizes.js @@ -3,7 +3,7 @@ const sharp = require('sharp'); const storage = require('../../../../adapters/storage'); const activeTheme = require('../../../../services/themes/active'); -const SIZE_PATH_REGEX = /^\/sizes\/([^/]+)\//; +const SIZE_PATH_REGEX = /^\/size\/([^/]+)\//; module.exports = function (req, res, next) { const storageInstance = storage.getStorage(); @@ -18,13 +18,13 @@ module.exports = function (req, res, next) { // CASE: unknown size or missing size config const imageSizeConfig = imageSizes[requestedSize]; if (!imageSizeConfig || (!imageSizeConfig.width && !imageSizeConfig.height)) { - const url = req.originalUrl.replace(`/sizes/${requestedSize}`, ''); + const url = req.originalUrl.replace(`/size/${requestedSize}`, ''); return res.redirect(url); } // CASE: unsupported storage adapter but theme is using custom image_sizes if (typeof storageInstance.saveRaw !== 'function') { - const url = req.originalUrl.replace(`/sizes/${requestedSize}`, ''); + const url = req.originalUrl.replace(`/size/${requestedSize}`, ''); return res.redirect(url); } From 9d3cdceb20061d63052fcce8076eb7207d0e9545 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 3 Dec 2018 13:34:18 +0700 Subject: [PATCH 09/22] Exited middleware as early as possible --- .../web/shared/middlewares/image/handle-image-sizes.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 35a748a9be1a..6dfeba1f2a53 100644 --- a/core/server/web/shared/middlewares/image/handle-image-sizes.js +++ b/core/server/web/shared/middlewares/image/handle-image-sizes.js @@ -6,13 +6,13 @@ const activeTheme = require('../../../../services/themes/active'); const SIZE_PATH_REGEX = /^\/size\/([^/]+)\//; module.exports = function (req, res, next) { - const storageInstance = storage.getStorage(); - const imageSizes = activeTheme.get().config('image_sizes'); - if (!SIZE_PATH_REGEX.test(req.url)) { return next(); } + const storageInstance = storage.getStorage(); + const imageSizes = activeTheme.get().config('image_sizes'); + const [sizeImageDir, requestedSize] = req.url.match(SIZE_PATH_REGEX); // CASE: unknown size or missing size config From ddbc78de32f4885e936dc175b428fb5a9aa0b3fd Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 3 Dec 2018 13:35:42 +0700 Subject: [PATCH 10/22] Called getStorage as late as possible --- core/server/web/shared/middlewares/image/handle-image-sizes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6dfeba1f2a53..835a00afd3e2 100644 --- a/core/server/web/shared/middlewares/image/handle-image-sizes.js +++ b/core/server/web/shared/middlewares/image/handle-image-sizes.js @@ -10,7 +10,6 @@ module.exports = function (req, res, next) { return next(); } - const storageInstance = storage.getStorage(); const imageSizes = activeTheme.get().config('image_sizes'); const [sizeImageDir, requestedSize] = req.url.match(SIZE_PATH_REGEX); @@ -22,6 +21,7 @@ module.exports = function (req, res, next) { return res.redirect(url); } + const storageInstance = storage.getStorage(); // CASE: unsupported storage adapter but theme is using custom image_sizes if (typeof storageInstance.saveRaw !== 'function') { const url = req.originalUrl.replace(`/size/${requestedSize}`, ''); From 6adbb2d38d671d7b7695e3fbab8efa5b5fdca042 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 3 Dec 2018 14:04:30 +0700 Subject: [PATCH 11/22] Updated image sizes middleware to handle dimension paths --- .../middlewares/image/handle-image-sizes.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) 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 835a00afd3e2..f3423c0f6fe4 100644 --- a/core/server/web/shared/middlewares/image/handle-image-sizes.js +++ b/core/server/web/shared/middlewares/image/handle-image-sizes.js @@ -11,20 +11,27 @@ module.exports = function (req, res, next) { } const imageSizes = activeTheme.get().config('image_sizes'); + const imageDimensions = Object.keys(imageSizes).reduce((dimensions, size) => { + const {width, height} = imageSizes[size]; + const dimension = (width ? 'w' + width : '') + (height ? 'h' + height : ''); + return Object.assign({ + [dimension]: imageSizes[size] + }, dimensions); + }, {}); - const [sizeImageDir, requestedSize] = req.url.match(SIZE_PATH_REGEX); + const [sizeImageDir, requestedDimension] = req.url.match(SIZE_PATH_REGEX); // CASE: unknown size or missing size config - const imageSizeConfig = imageSizes[requestedSize]; - if (!imageSizeConfig || (!imageSizeConfig.width && !imageSizeConfig.height)) { - const url = req.originalUrl.replace(`/size/${requestedSize}`, ''); + const imageDimensionConfig = imageDimensions[requestedDimension]; + if (!imageDimensionConfig || (!imageDimensionConfig.width && !imageDimensionConfig.height)) { + const url = req.originalUrl.replace(`/size/${requestedDimension}`, ''); return res.redirect(url); } const storageInstance = storage.getStorage(); // CASE: unsupported storage adapter but theme is using custom image_sizes if (typeof storageInstance.saveRaw !== 'function') { - const url = req.originalUrl.replace(`/size/${requestedSize}`, ''); + const url = req.originalUrl.replace(`/size/${requestedDimension}`, ''); return res.redirect(url); } @@ -38,7 +45,7 @@ module.exports = function (req, res, next) { return storageInstance.read({path: originalImagePath}) .then((originalImageBuffer) => { return sharp(originalImageBuffer) - .resize(imageSizeConfig.width, imageSizeConfig.height, { + .resize(imageDimensionConfig.width, imageDimensionConfig.height, { withoutEnlargement: true }) .toBuffer(); From 7e791edc02a1a580cc49d7ef7dc6db6717229bfb Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 3 Dec 2018 14:08:34 +0700 Subject: [PATCH 12/22] Revert "Deleted sizes directory when theme is activated" This reverts commit 9204dfcc73a6a79d597dbf23651817bcbfc59991. --- core/server/services/themes/index.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/core/server/services/themes/index.js b/core/server/services/themes/index.js index 687f4c206817..4c9c9b578e8f 100644 --- a/core/server/services/themes/index.js +++ b/core/server/services/themes/index.js @@ -7,8 +7,6 @@ var debug = require('ghost-ignition').debug('themes'), settingsCache = require('../settings/cache'), themeStorage; -const ImageStorage = require('../../adapters/storage'); - // @TODO: reduce the amount of things we expose to the outside world // Make this a nice clean sensible API we can all understand! module.exports = { @@ -87,7 +85,6 @@ module.exports = { try { active.set(loadedTheme, checkedTheme, error); common.events.emit('services.themes.activated'); - ImageStorage.getStorage().delete('sizes'); } catch (err) { common.logging.error(new common.errors.InternalServerError({ message: common.i18n.t('errors.middleware.themehandler.activateFailed', {theme: loadedTheme.name}), From 515e23ff1ec2d3ade7b66edd998b44a3cb598aae Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 3 Dec 2018 14:08:49 +0700 Subject: [PATCH 13/22] Revert "Removed delete method from theme Storage class" This reverts commit b45fdb405a05faeaf4bd87e977c4ac64ff96b057. --- core/server/services/themes/Storage.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/server/services/themes/Storage.js b/core/server/services/themes/Storage.js index d21cdde0f992..50802521418e 100644 --- a/core/server/services/themes/Storage.js +++ b/core/server/services/themes/Storage.js @@ -55,6 +55,10 @@ class ThemeStorage extends LocalFileStorage { }); }; } + + delete(fileName) { + return fs.remove(path.join(this.storagePath, fileName)); + } } module.exports = ThemeStorage; From e9d67e7beb260531d6f6c56725f20e73bdb58098 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 3 Dec 2018 14:08:54 +0700 Subject: [PATCH 14/22] Revert "Implemented delete for LocalFileStorage" This reverts commit a587cd6bae45b68a293b2d5cfd9b7705a29e7bfa. --- core/server/adapters/storage/LocalFileStorage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/server/adapters/storage/LocalFileStorage.js b/core/server/adapters/storage/LocalFileStorage.js index c0c91f4abaed..9cdbee300fd6 100644 --- a/core/server/adapters/storage/LocalFileStorage.js +++ b/core/server/adapters/storage/LocalFileStorage.js @@ -135,8 +135,8 @@ class LocalFileStore extends StorageBase { * Not implemented. * @returns {Promise.<*>} */ - delete(fileName) { - return fs.remove(path.join(this.storagePath, fileName)); + delete() { + return Promise.reject('not implemented'); } /** From c2ee4ea77373988769a4a3f24f7894efd9c59c2e Mon Sep 17 00:00:00 2001 From: Naz Gargol Date: Mon, 3 Dec 2018 16:43:41 +0700 Subject: [PATCH 15/22] Fixed typo Co-Authored-By: allouis --- core/server/adapters/storage/LocalFileStorage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/server/adapters/storage/LocalFileStorage.js b/core/server/adapters/storage/LocalFileStorage.js index 9cdbee300fd6..71d2af0f65bf 100644 --- a/core/server/adapters/storage/LocalFileStorage.js +++ b/core/server/adapters/storage/LocalFileStorage.js @@ -22,7 +22,7 @@ class LocalFileStore extends StorageBase { /** * Saves a buffer in the targetPath * - buffer is an instance of Buffer - * - returns a Promise with returns the full URL to retrieve the data + * - returns a Promise which returns the full URL to retrieve the data */ saveRaw(buffer, targetPath) { const storagePath = path.join(this.storagePath, targetPath); From 9186c3ea427328afa25c7fbf76aba6f2b1c38382 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 3 Dec 2018 19:35:08 +0700 Subject: [PATCH 16/22] Redirected to original image if no image_sizes config --- .../web/shared/middlewares/image/handle-image-sizes.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) 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 f3423c0f6fe4..a98c4d549308 100644 --- a/core/server/web/shared/middlewares/image/handle-image-sizes.js +++ b/core/server/web/shared/middlewares/image/handle-image-sizes.js @@ -10,7 +10,14 @@ module.exports = function (req, res, next) { return next(); } + const [sizeImageDir, requestedDimension] = req.url.match(SIZE_PATH_REGEX); + const imageSizes = activeTheme.get().config('image_sizes'); + if (!imageSizes) { + const url = req.originalUrl.replace(`/size/${requestedDimension}`, ''); + return res.redirect(url); + } + const imageDimensions = Object.keys(imageSizes).reduce((dimensions, size) => { const {width, height} = imageSizes[size]; const dimension = (width ? 'w' + width : '') + (height ? 'h' + height : ''); @@ -19,8 +26,6 @@ module.exports = function (req, res, next) { }, dimensions); }, {}); - const [sizeImageDir, requestedDimension] = req.url.match(SIZE_PATH_REGEX); - // CASE: unknown size or missing size config const imageDimensionConfig = imageDimensions[requestedDimension]; if (!imageDimensionConfig || (!imageDimensionConfig.width && !imageDimensionConfig.height)) { From 3a4e3f0d11510e48a25929623071a2f075b83249 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 3 Dec 2018 19:39:11 +0700 Subject: [PATCH 17/22] Refactored redirection because rule of three --- .../shared/middlewares/image/handle-image-sizes.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) 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 a98c4d549308..5aa160b7ff47 100644 --- a/core/server/web/shared/middlewares/image/handle-image-sizes.js +++ b/core/server/web/shared/middlewares/image/handle-image-sizes.js @@ -11,11 +11,14 @@ module.exports = function (req, res, next) { } const [sizeImageDir, requestedDimension] = req.url.match(SIZE_PATH_REGEX); + const redirectToOriginal = () => { + const url = req.originalUrl.replace(`/size/${requestedDimension}`, ''); + return res.redirect(url); + }; const imageSizes = activeTheme.get().config('image_sizes'); if (!imageSizes) { - const url = req.originalUrl.replace(`/size/${requestedDimension}`, ''); - return res.redirect(url); + return redirectToOriginal(); } const imageDimensions = Object.keys(imageSizes).reduce((dimensions, size) => { @@ -29,15 +32,13 @@ module.exports = function (req, res, next) { // CASE: unknown size or missing size config const imageDimensionConfig = imageDimensions[requestedDimension]; if (!imageDimensionConfig || (!imageDimensionConfig.width && !imageDimensionConfig.height)) { - const url = req.originalUrl.replace(`/size/${requestedDimension}`, ''); - return res.redirect(url); + return redirectToOriginal(); } const storageInstance = storage.getStorage(); // CASE: unsupported storage adapter but theme is using custom image_sizes if (typeof storageInstance.saveRaw !== 'function') { - const url = req.originalUrl.replace(`/size/${requestedDimension}`, ''); - return res.redirect(url); + return redirectToOriginal(); } storageInstance.exists(req.url).then((exists) => { From ad6d41a361c8df55d11ebdabbd9c65ff6ae5c86f Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 3 Dec 2018 19:39:24 +0700 Subject: [PATCH 18/22] Updated comments --- core/server/web/shared/middlewares/image/handle-image-sizes.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 5aa160b7ff47..c6a514587e4b 100644 --- a/core/server/web/shared/middlewares/image/handle-image-sizes.js +++ b/core/server/web/shared/middlewares/image/handle-image-sizes.js @@ -17,6 +17,7 @@ module.exports = function (req, res, next) { }; const imageSizes = activeTheme.get().config('image_sizes'); + // CASE: no image_sizes config if (!imageSizes) { return redirectToOriginal(); } @@ -29,8 +30,8 @@ module.exports = function (req, res, next) { }, dimensions); }, {}); - // CASE: unknown size or missing size config const imageDimensionConfig = imageDimensions[requestedDimension]; + // CASE: unknown dimension if (!imageDimensionConfig || (!imageDimensionConfig.width && !imageDimensionConfig.height)) { return redirectToOriginal(); } From 10884ee5d400288df6e95743f0e78dcce7119bfb Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 3 Dec 2018 20:03:41 +0700 Subject: [PATCH 19/22] Added rubbish tests --- .../image/handle-image-sizes_spec.js | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 core/test/unit/web/middleware/image/handle-image-sizes_spec.js diff --git a/core/test/unit/web/middleware/image/handle-image-sizes_spec.js b/core/test/unit/web/middleware/image/handle-image-sizes_spec.js new file mode 100644 index 000000000000..d1056007dd70 --- /dev/null +++ b/core/test/unit/web/middleware/image/handle-image-sizes_spec.js @@ -0,0 +1,43 @@ +const should = require('should'); +const handleImageSizes = require('../../../../../server/web/shared/middlewares/image/handle-image-sizes.js'); + +describe('handleImageSizes middleware', function () { + it('calls next immediately if the url does not match /size/something/', function (done) { + const fakeReq = { + url: '/size/something' + }; + // CASE: second thing middleware does is try to match to a regex + fakeReq.url.match = function () { + throw new Error('Should have exited immediately'); + }; + handleImageSizes(fakeReq, {}, function next() { + done(); + }); + }); + + it('calls next immediately if the url does not match /size/something/', function (done) { + const fakeReq = { + url: '/url/whatever/' + }; + // CASE: second thing middleware does is try to match to a regex + fakeReq.url.match = function () { + throw new Error('Should have exited immediately'); + }; + handleImageSizes(fakeReq, {}, function next() { + done(); + }); + }); + + it('calls next immediately if the url does not match /size/something/', function (done) { + const fakeReq = { + url: '/size//' + }; + // CASE: second thing middleware does is try to match to a regex + fakeReq.url.match = function () { + throw new Error('Should have exited immediately'); + }; + handleImageSizes(fakeReq, {}, function next() { + done(); + }); + }); +}); From 008a387a8663f11b2815843973216645f9470338 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Mon, 3 Dec 2018 20:05:46 +0700 Subject: [PATCH 20/22] Added @TODO comment for handleImageSizes tests --- core/test/unit/web/middleware/image/handle-image-sizes_spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/core/test/unit/web/middleware/image/handle-image-sizes_spec.js b/core/test/unit/web/middleware/image/handle-image-sizes_spec.js index d1056007dd70..60c46f3bb273 100644 --- a/core/test/unit/web/middleware/image/handle-image-sizes_spec.js +++ b/core/test/unit/web/middleware/image/handle-image-sizes_spec.js @@ -1,6 +1,7 @@ const should = require('should'); const handleImageSizes = require('../../../../../server/web/shared/middlewares/image/handle-image-sizes.js'); +// @TODO make these tests lovely and non specific to implementation describe('handleImageSizes middleware', function () { it('calls next immediately if the url does not match /size/something/', function (done) { const fakeReq = { From 58b3d8a4f0c72c3ca8c3c4a8ff60c31ba7877e7e Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 13 Dec 2018 18:09:17 +0700 Subject: [PATCH 21/22] Added safeResizeImage method to image manipulator --- core/server/lib/image/manipulator.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/core/server/lib/image/manipulator.js b/core/server/lib/image/manipulator.js index 2da4b93fd47e..9051272d7baa 100644 --- a/core/server/lib/image/manipulator.js +++ b/core/server/lib/image/manipulator.js @@ -61,4 +61,27 @@ const process = (options = {}) => { }); }; +const resizeImage = (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; + }); +}; + module.exports.process = process; +module.exports.safeResizeImage = (buffer, options) => { + try { + require('sharp'); + return resizeImage(buffer, options); + } catch (e) { + return Promise.resolve(buffer); + } +}; From 46bddcdba4cd016a3735e76168754d877dbcba76 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 13 Dec 2018 18:10:02 +0700 Subject: [PATCH 22/22] Used image manipulator lib in image_size middleware --- .../web/shared/middlewares/image/handle-image-sizes.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) 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 c6a514587e4b..169c3e8f730e 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 sharp = require('sharp'); +const image = require('../../../../lib/image'); const storage = require('../../../../adapters/storage'); const activeTheme = require('../../../../services/themes/active'); @@ -51,11 +51,7 @@ module.exports = function (req, res, next) { return storageInstance.read({path: originalImagePath}) .then((originalImageBuffer) => { - return sharp(originalImageBuffer) - .resize(imageDimensionConfig.width, imageDimensionConfig.height, { - withoutEnlargement: true - }) - .toBuffer(); + return image.manipulator.safeResizeImage(originalImageBuffer, imageDimensionConfig); }) .then((resizedImageBuffer) => { return storageInstance.saveRaw(resizedImageBuffer, req.url);