From 0a442e319c45b91a2ce7b59efad1045596a323dd Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Wed, 7 Nov 2018 15:29:14 +1100
Subject: [PATCH] FIX: correct svg handling for images

We regressed and optimized images no longer worked with svg

The following adds the correct logic to simply copy file for svgs
and bypasses resizing for svg avatars
---
 app/controllers/user_avatars_controller.rb |  1 +
 app/models/optimized_image.rb              |  6 +++---
 lib/upload_creator.rb                      |  4 ++--
 spec/models/optimized_image_spec.rb        | 15 +++++++++++++++
 4 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/app/controllers/user_avatars_controller.rb b/app/controllers/user_avatars_controller.rb
index 39050be70bd..a7f32b11710 100644
--- a/app/controllers/user_avatars_controller.rb
+++ b/app/controllers/user_avatars_controller.rb
@@ -184,6 +184,7 @@ class UserAvatarsController < ApplicationController
 
   def get_optimized_image(upload, size)
     return if !upload
+    return upload if upload.extension == "svg"
 
     upload.get_optimized_image(size, size, allow_animation: SiteSetting.allow_animated_avatars)
     # TODO decide if we want to detach here
diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb
index fbf8b60bec7..c9cdbd94ef5 100644
--- a/app/models/optimized_image.rb
+++ b/app/models/optimized_image.rb
@@ -21,6 +21,7 @@ class OptimizedImage < ActiveRecord::Base
   end
 
   def self.create_for(upload, width, height, opts = {})
+
     return unless width > 0 && height > 0
     return if upload.try(:sha1).blank?
 
@@ -29,7 +30,7 @@ class OptimizedImage < ActiveRecord::Base
       upload.fix_image_extension
     end
 
-    if !upload.extension.match?(IM_DECODERS)
+    if !upload.extension.match?(IM_DECODERS) && upload.extension != "svg"
       if !opts[:raise_on_error]
         # nothing to do ... bad extension, not an image
         return
@@ -76,7 +77,7 @@ class OptimizedImage < ActiveRecord::Base
         temp_file = Tempfile.new(["discourse-thumbnail", extension])
         temp_path = temp_file.path
 
-        if extension =~ /\.svg$/i
+        if upload.extension == "svg"
           FileUtils.cp(original_path, temp_path)
           resized = true
         elsif opts[:crop]
@@ -86,7 +87,6 @@ class OptimizedImage < ActiveRecord::Base
         end
 
         if resized
-
           thumbnail = OptimizedImage.create!(
             upload_id: upload.id,
             sha1: Upload.generate_digest(temp_path),
diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb
index 04c3e670ec4..431f4f6b94a 100644
--- a/lib/upload_creator.rb
+++ b/lib/upload_creator.rb
@@ -44,7 +44,7 @@ class UploadCreator
         extract_image_info!
         return @upload if @upload.errors.present?
 
-        if @filename[/\.svg$/i]
+        if @image_info.type.to_s == "svg"
           whitelist_svg!
         elsif !Rails.env.test? || @opts[:force_optimize]
           convert_to_jpeg! if should_convert_to_jpeg?
@@ -131,7 +131,7 @@ class UploadCreator
         end
       end
 
-      if @upload.errors.empty? && is_image && @opts[:type] == "avatar"
+      if @upload.errors.empty? && is_image && @opts[:type] == "avatar" && @upload.extension != "svg"
         Jobs.enqueue(:create_avatar_thumbnails, upload_id: @upload.id, user_id: user_id)
       end
 
diff --git a/spec/models/optimized_image_spec.rb b/spec/models/optimized_image_spec.rb
index caef6150f11..1eefbb01c35 100644
--- a/spec/models/optimized_image_spec.rb
+++ b/spec/models/optimized_image_spec.rb
@@ -185,6 +185,21 @@ describe OptimizedImage do
 
   describe ".create_for" do
 
+    it "is able to 'optimize' an svg" do
+
+      # we don't really optimize anything, we simply copy
+      # but at least this confirms this actually works
+
+      SiteSetting.authorized_extensions = 'svg'
+      svg = file_from_fixtures('image.svg')
+      upload = UploadCreator.new(svg, 'image.svg').create_for(Discourse.system_user.id)
+      resized = upload.get_optimized_image(50, 50, {})
+
+      # we perform some basic svg mangling but expect the string Discourse to be there
+      expect(File.read(Discourse.store.path_for(resized))).to include("Discourse")
+      expect(File.read(Discourse.store.path_for(resized))).to eq(File.read(Discourse.store.path_for(upload)))
+    end
+
     context "when using an internal store" do
 
       let(:store) { FakeInternalStore.new }