From 4d981cec53aa80813d50018c2ddab6e9947a1d4e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 22 Feb 2016 12:57:24 +0100
Subject: [PATCH] FIX: don't try to optimize large PNGs (takes too much time)

---
 app/controllers/uploads_controller.rb | 34 +++++++++++++++------------
 app/models/upload.rb                  | 20 ++++++++++++----
 spec/models/upload_spec.rb            |  1 -
 3 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb
index 65be0cea906..f91f70763ad 100644
--- a/app/controllers/uploads_controller.rb
+++ b/app/controllers/uploads_controller.rb
@@ -51,13 +51,16 @@ class UploadsController < ApplicationController
     render nothing: true, status: 404
   end
 
+  MAXIMUM_UPLOAD_SIZE ||= 10.megabytes
+  DOWNSIZE_RATIO ||= 0.8
+
   def create_upload(type, file, url)
     begin
       # ensure we have a file
       if file.nil?
         # API can provide a URL
         if url.present? && is_api?
-          tempfile = FileHelper.download(url, 10.megabytes, "discourse-upload-#{type}") rescue nil
+          tempfile = FileHelper.download(url, MAXIMUM_UPLOAD_SIZE, "discourse-upload-#{type}") rescue nil
           filename = File.basename(URI.parse(url).path)
         end
       else
@@ -69,20 +72,21 @@ class UploadsController < ApplicationController
       return { errors: I18n.t("upload.file_missing") } if tempfile.nil?
 
       # allow users to upload (not that) large images that will be automatically reduced to allowed size
-      uploaded_size = File.size(tempfile.path)
-      if SiteSetting.max_image_size_kb > 0 && FileHelper.is_image?(filename) && uploaded_size > 0 && uploaded_size < 10.megabytes
-        attempt = 2
-        allow_animation = type == "avatar" ? SiteSetting.allow_animated_avatars : SiteSetting.allow_animated_thumbnails
-        while attempt > 0
-          downsized_size = File.size(tempfile.path)
-          break if downsized_size > uploaded_size
-          break if downsized_size < SiteSetting.max_image_size_kb.kilobytes
-          image_info = FastImage.new(tempfile.path) rescue nil
-          w, h = *(image_info.try(:size) || [0, 0])
-          break if w == 0 || h == 0
-          dimensions = "#{(w * 0.8).floor}x#{(h * 0.8).floor}"
-          OptimizedImage.downsize(tempfile.path, tempfile.path, dimensions, filename: filename, allow_animation: allow_animation)
-          attempt -= 1
+      if SiteSetting.max_image_size_kb > 0 && FileHelper.is_image?(filename)
+        uploaded_size = File.size(tempfile.path)
+        if 0 < uploaded_size && uploaded_size < MAXIMUM_UPLOAD_SIZE && Upload.should_optimize?(tempfile.path)
+          attempt = 2
+          allow_animation = type == "avatar" ? SiteSetting.allow_animated_avatars : SiteSetting.allow_animated_thumbnails
+          while attempt > 0
+            downsized_size = File.size(tempfile.path)
+            break if uploaded_size < downsized_size || downsized_size < SiteSetting.max_image_size_kb.kilobytes
+            image_info = FastImage.new(tempfile.path) rescue nil
+            w, h = *(image_info.try(:size) || [0, 0])
+            break if w == 0 || h == 0
+            dimensions = "#{(w * DOWNSIZE_RATIO).floor}x#{(h * DOWNSIZE_RATIO).floor}"
+            OptimizedImage.downsize(tempfile.path, tempfile.path, dimensions, filename: filename, allow_animation: allow_animation)
+            attempt -= 1
+          end
         end
       end
 
diff --git a/app/models/upload.rb b/app/models/upload.rb
index 6c693730184..49480639d43 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -73,8 +73,8 @@ class Upload < ActiveRecord::Base
           w = svg["width"].to_i
           h = svg["height"].to_i
         else
-          # fix orientation first (but not for GIFs)
-          fix_image_orientation(file.path) unless filename =~ /\.GIF$/i
+          # fix orientation first
+          fix_image_orientation(file.path) if should_optimize?(file.path)
           # retrieve image info
           image_info = FastImage.new(file) rescue nil
           w, h = *(image_info.try(:size) || [0, 0])
@@ -107,8 +107,8 @@ class Upload < ActiveRecord::Base
           end
         end
 
-        # optimize image (but not for GIFs)
-        if filename !~ /\.GIF$/i
+        # optimize image (except GIFs and large PNGs)
+        if should_optimize?(file.path)
           ImageOptim.new.optimize_image!(file.path) rescue nil
           # update the file size
           filesize = File.size(file.path)
@@ -163,6 +163,18 @@ class Upload < ActiveRecord::Base
     end
   end
 
+  LARGE_PNG_SIZE ||= 3.megabytes
+
+  def self.should_optimize?(path)
+    # don't optimize GIFs
+    return false if path =~ /\.gif$/i
+    return true  if path !~ /\.png$/i
+    image_info = FastImage.new(path) rescue nil
+    w, h = *(image_info.try(:size) || [0, 0])
+    # don't optimize large PNGs
+    w > 0 && h > 0 && w * h < LARGE_PNG_SIZE
+  end
+
   def self.is_dimensionless_image?(filename, width, height)
     FileHelper.is_image?(filename) && (width.blank? || width == 0 || height.blank? || height == 0)
   end
diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb
index b67196ec6d9..deddf2b795c 100644
--- a/spec/models/upload_spec.rb
+++ b/spec/models/upload_spec.rb
@@ -62,7 +62,6 @@ describe Upload do
     end
 
     it "computes width & height for images" do
-      FastImage.any_instance.expects(:size).returns([100, 200])
       ImageSizer.expects(:resize)
       image.expects(:rewind).twice
       Upload.create_for(user_id, image, image_filename, image_filesize)