From 9613de7cc1d565e884d11736546ebb7fbceca442 Mon Sep 17 00:00:00 2001
From: Nazar Gargol <nazargargol@gmail.com>
Date: Thu, 6 Sep 2018 11:42:42 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20gif=20images=20are=20not?=
 =?UTF-8?q?=20converted=20to=20png=20(#9853)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

refs #9848

- Disabled image optimization for .gif files as a temporary solution until sharp library starts supporting latest version of libvips
- Disabled image optimization for .svg/.svgz files as permanent solution as these file types are being converted to .png
---
 core/server/web/middleware/image/normalize.js |  6 ++++-
 .../web/middleware/image/normalize_spec.js    | 22 ++++++++++++++-----
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/core/server/web/middleware/image/normalize.js b/core/server/web/middleware/image/normalize.js
index 71b5873b0e..2f04d0c512 100644
--- a/core/server/web/middleware/image/normalize.js
+++ b/core/server/web/middleware/image/normalize.js
@@ -7,7 +7,11 @@ const image = require('../../../lib/image');
 module.exports = function normalize(req, res, next) {
     const imageOptimizationOptions = config.get('imageOptimization');
 
-    if (!imageOptimizationOptions.resize) {
+    // 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)) {
         return next();
     }
 
diff --git a/core/test/unit/web/middleware/image/normalize_spec.js b/core/test/unit/web/middleware/image/normalize_spec.js
index 195d45cae1..387ef63c81 100644
--- a/core/test/unit/web/middleware/image/normalize_spec.js
+++ b/core/test/unit/web/middleware/image/normalize_spec.js
@@ -14,7 +14,8 @@ describe('normalize', function () {
         req = {
             file: {
                 name: 'test',
-                path: '/test/path'
+                path: '/test/path',
+                ext: '.jpg'
             }
         };
 
@@ -30,7 +31,7 @@ describe('normalize', function () {
     it('should do manipulation by default', function (done) {
         image.manipulator.process.resolves();
 
-        normalize(req, res, () => {
+        normalize(req, res, function () {
             image.manipulator.process.calledOnce.should.be.true();
             done();
         });
@@ -39,7 +40,7 @@ describe('normalize', function () {
     it('should add files array to request object with original and processed files', function (done) {
         image.manipulator.process.resolves();
 
-        normalize(req, res, () => {
+        normalize(req, res, function () {
             req.files.length.should.be.equal(2);
             done();
         });
@@ -52,13 +53,13 @@ describe('normalize', function () {
             }
         });
 
-        normalize(req, res, () => {
+        normalize(req, res, function () {
             image.manipulator.process.called.should.be.false();
             done();
         });
     });
 
-    it('should call manipulation when resize flag is explicitly set', function (done) {
+    it('should not create files array when processing fails', function (done) {
         image.manipulator.process.rejects();
 
         normalize(req, res, ()=> {
@@ -68,4 +69,15 @@ describe('normalize', function () {
             done();
         });
     });
+
+    ['.gif', '.svg', '.svgz'].forEach(function (extension) {
+        it(`should skip processing when file extension is ${extension}`, function (done) {
+            req.file.ext = extension;
+            normalize(req, res, function () {
+                req.file.should.not.be.equal(undefined);
+                should.not.exist(req.files);
+                done();
+            });
+        });
+    });
 });