FEATURE: automatically correct extension for bad uploads

This fixes with post thumbnails on the fly

app/controllers/user_avatars_controller.rb

@@ -184,45 +184,10 @@ class UserAvatarsController < ApplicationController
   # consider removal of hacks some time in 2019
 
   def get_optimized_image(upload, size)
-    if (!upload.extension || upload.extension.length == 0)
-      fix_extension(upload)
-    end
+    return if !upload
 
-    begin
-      try_optimize(upload, size, true)
-    rescue
-      if fix_extension(upload)
-        try_optimize(upload, size, false)
-        # TODO decide if we want to detach faulty avatar here?
-      else
-        nil
-      end
-    end
-  end
-
-  def fix_extension(upload)
-    # this is relatively cheap
-    original_path = Discourse.store.path_for(upload)
-    if original_path.blank?
-      external_copy = Discourse.store.download(upload) rescue nil
-      original_path = external_copy.try(:path)
-    end
-
-    image_info = FastImage.new(original_path) rescue nil
-    if image_info && image_info.type.to_s != upload.extension
-      upload.update_columns(extension: image_info.type.to_s)
-      true
-    end
-  end
-
-  def try_optimize(upload, size, raise_on_error)
-    OptimizedImage.create_for(
-      upload,
-      size,
-      size,
-      allow_animation: SiteSetting.allow_animated_avatars,
-      raise_on_error: raise_on_error
-    )
+    upload.get_optimized_image(size, size, allow_animation: SiteSetting.allow_animated_avatars)
+    # TODO decide if we want to detach here
   end
 
 end

app/models/optimized_image.rb

@@ -24,6 +24,20 @@ class OptimizedImage < ActiveRecord::Base
     return unless width > 0 && height > 0
     return if upload.try(:sha1).blank?
 
+    # no extension so try to guess it
+    if (!upload.extension)
+      upload.fix_image_extension
+    end
+
+    if !upload.extension.match?(IM_DECODERS)
+      if !opts[:raise_on_error]
+        # nothing to do ... bad extension, not an image
+        return
+      else
+        raise InvalidAccess
+      end
+    end
+
     lock(upload.id, width, height) do
       # do we already have that thumbnail?
       thumbnail = find_by(upload_id: upload.id, width: width, height: height)
@@ -127,7 +141,7 @@ class OptimizedImage < ActiveRecord::Base
 
   def self.prepend_decoder!(path, ext_path = nil)
     extension = File.extname(ext_path || path)[1..-1]
-    raise Discourse::InvalidAccess unless extension.present? && extension[IM_DECODERS]
+    raise Discourse::InvalidAccess if !extension || !extension.match?(IM_DECODERS)
     "#{extension}:#{path}"
   end
 

app/models/upload.rb

@@ -40,13 +40,58 @@ class Upload < ActiveRecord::Base
       crop: crop
     }
 
-    if _thumbnail = OptimizedImage.create_for(self, width, height, opts)
+    if get_optimized_image(width, height, opts)
+      # TODO: this code is not right, we may have multiple
+      # thumbs
       self.width = width
       self.height = height
       save(validate: false)
     end
   end
 
+  # this method attempts to correct old incorrect extensions
+  def get_optimized_image(width, height, opts)
+    if (!extension || extension.length == 0)
+      fix_image_extension
+    end
+
+    opts = opts.merge(raise_on_error: true)
+    begin
+      OptimizedImage.create_for(self, width, height, opts)
+    rescue
+      opts = opts.merge(raise_on_error: false)
+      if fix_image_extension
+        OptimizedImage.create_for(self, width, height, opts)
+      else
+        nil
+      end
+    end
+  end
+
+  def fix_image_extension
+    return false if extension == "unknown"
+
+    begin
+      # this is relatively cheap once cached
+      original_path = Discourse.store.path_for(self)
+      if original_path.blank?
+        external_copy = Discourse.store.download(self) rescue nil
+        original_path = external_copy.try(:path)
+      end
+
+      image_info = FastImage.new(original_path) rescue nil
+      new_extension = image_info&.type&.to_s || "unknown"
+
+      if new_extension != self.extension
+        self.update_columns(extension: new_extension)
+        true
+      end
+    rescue
+      self.update_columns(extension: "unknown")
+      true
+    end
+  end
+
   def destroy
     Upload.transaction do
       Discourse.store.remove_upload(self)