From 0f32209e65089576b7e3b6cffab76bc4c613a530 Mon Sep 17 00:00:00 2001
From: Fabien O'Carroll <fabien@allou.is>
Date: Thu, 3 Jan 2019 16:28:37 +0700
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20responsive=20images=20fo?=
 =?UTF-8?q?r=20gifs=20&=20svgs=20(#10315)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

closes #10301

* Redirected to original image for gifs & svgs

* Created canTransformFileExtension method

* Updated image middlewares to use canTransformFileExtension
---
 core/server/lib/image/manipulator.js          |  7 +++++++
 .../middlewares/image/handle-image-sizes.js   |  6 ++++++
 .../web/shared/middlewares/image/normalize.js |  7 ++-----
 core/test/unit/lib/image/manipulator_spec.js  | 21 +++++++++++++++++++
 4 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/core/server/lib/image/manipulator.js b/core/server/lib/image/manipulator.js
index f0ecf92c7b..f70b4a88a9 100644
--- a/core/server/lib/image/manipulator.js
+++ b/core/server/lib/image/manipulator.js
@@ -36,6 +36,12 @@ const unsafeResizeImage = (originalBuffer, {width, height} = {}) => {
         });
 };
 
+// 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'].includes(ext);
+
 const makeSafe = fn => (...args) => {
     try {
         require('sharp');
@@ -55,5 +61,6 @@ const makeSafe = fn => (...args) => {
     });
 };
 
+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 0a66c09385..0251669809 100644
--- a/core/server/web/shared/middlewares/image/handle-image-sizes.js
+++ b/core/server/web/shared/middlewares/image/handle-image-sizes.js
@@ -16,6 +16,12 @@ module.exports = function (req, res, next) {
         return res.redirect(url);
     };
 
+    // CASE: image manipulator is uncapable of transforming file (e.g. .gif)
+    const requestUrlFileExtension = path.parse(req.url).ext;
+    if (!image.manipulator.canTransformFileExtension(requestUrlFileExtension)) {
+        return redirectToOriginal();
+    }
+
     const imageSizes = activeTheme.get().config('image_sizes');
     // CASE: no image_sizes config
     if (!imageSizes) {
diff --git a/core/server/web/shared/middlewares/image/normalize.js b/core/server/web/shared/middlewares/image/normalize.js
index 4c1a25da68..846dde6c02 100644
--- a/core/server/web/shared/middlewares/image/normalize.js
+++ b/core/server/web/shared/middlewares/image/normalize.js
@@ -7,11 +7,8 @@ const image = require('../../../../lib/image');
 module.exports = function normalize(req, res, next) {
     const imageOptimizationOptions = config.get('imageOptimization');
 
-    // 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
-    if (!imageOptimizationOptions.resize || ['.gif', '.svg', '.svgz'].includes(req.file.ext)) {
+    // CASE: image manipulator is uncapable of transforming file (e.g. .gif)
+    if (!image.manipulator.canTransformFileExtension(req.file.ext) || !imageOptimizationOptions.resize) {
         return next();
     }
 
diff --git a/core/test/unit/lib/image/manipulator_spec.js b/core/test/unit/lib/image/manipulator_spec.js
index 7d1931f304..4cef53b2c1 100644
--- a/core/test/unit/lib/image/manipulator_spec.js
+++ b/core/test/unit/lib/image/manipulator_spec.js
@@ -12,6 +12,27 @@ describe('lib/image: manipulator', function () {
         testUtils.unmockNotExistingModule();
     });
 
+    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
+            );
+        });
+    });
+
     describe('cases', function () {
         let sharp, sharpInstance;