diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 923f62407..1036e1936 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -59,13 +59,7 @@ class UploadsController < ApplicationController content_type = file.content_type end - # when we're dealing with an avatar, crop it to its maximum size - if type == "avatar" && FileHelper.is_image?(filename) - max = Discourse.avatar_sizes.max - OptimizedImage.resize(tempfile.path, tempfile.path, max, max, allow_animation: SiteSetting.allow_animated_avatars) - end - - upload = Upload.create_for(current_user.id, tempfile, filename, tempfile.size, content_type: content_type) + upload = Upload.create_for(current_user.id, tempfile, filename, tempfile.size, content_type: content_type, image_type: type) if upload.errors.empty? && current_user.admin? retain_hours = params[:retain_hours].to_i diff --git a/app/models/upload.rb b/app/models/upload.rb index 995a185b2..b8e0a1e5f 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -49,13 +49,62 @@ class Upload < ActiveRecord::Base File.extname(original_filename) end + # list of image types that will be cropped + CROPPED_IMAGE_TYPES ||= ["avatar", "profile_background", "card_background"] + # options # - content_type # - origin + # - image_type def self.create_for(user_id, file, filename, filesize, options = {}) - sha1 = Digest::SHA1.file(file).hexdigest + DistributedMutex.synchronize("upload_#{user_id}_#{filename}") do + # do some work on images + if FileHelper.is_image?(filename) + if filename =~ /\.svg$/i + svg = Nokogiri::XML(file).at_css("svg") + w = svg["width"].to_i + h = svg["height"].to_i + else + # fix orientation first + fix_image_orientation(file.path) + # retrieve image info + image_info = FastImage.new(file) rescue nil + w, h = *(image_info.try(:size) || [0, 0]) + end + + # default size + width, height = ImageSizer.resize(w, h) + + # make sure we're at the beginning of the file (both FastImage and Nokogiri move the pointer) + file.rewind + + # crop images depending on their type + if CROPPED_IMAGE_TYPES.include?(options[:image_type]) + allow_animation = false + max_pixel_ratio = Discourse::PIXEL_RATIOS.max + + case options[:image_type] + when "avatar" + allow_animation = SiteSetting.allow_animated_avatars + width = height = Discourse.avatar_sizes.max + when "profile_background" + max_width = 850 * max_pixel_ratio + width, height = ImageSizer.resize(w, h, max_width: max_width, max_height: max_width) + when "card_background" + max_width = 590 * max_pixel_ratio + width, height = ImageSizer.resize(w, h, max_width: max_width, max_height: max_width) + end + + OptimizedImage.resize(file.path, file.path, width, height, allow_animation: allow_animation) + end + + # optimize image + ImageOptim.new.optimize_image!(file.path) rescue nil + end + + # compute the sha of the file + sha1 = Digest::SHA1.file(file).hexdigest - DistributedMutex.synchronize("upload_#{sha1}") do # do we already have that upload? upload = find_by(sha1: sha1) @@ -75,13 +124,12 @@ class Upload < ActiveRecord::Base upload.filesize = filesize upload.sha1 = sha1 upload.url = "" + upload.width = width + upload.height = height upload.origin = options[:origin][0...1000] if options[:origin] - if FileHelper.is_image?(filename) - # deal with width & height for images - upload = resize_image(filename, file, upload) - # optimize image - ImageOptim.new.optimize_image!(file.path) rescue nil + if FileHelper.is_image?(filename) && (upload.width == 0 || upload.height == 0) + upload.errors.add(:base, I18n.t("upload.images.size_not_found")) end return upload unless upload.save @@ -97,43 +145,10 @@ class Upload < ActiveRecord::Base end end - # return the uploaded file upload end end - def self.resize_image(filename, file, upload) - begin - if filename =~ /\.svg$/i - svg = Nokogiri::XML(file).at_css("svg") - width, height = svg["width"].to_i, svg["height"].to_i - if width == 0 || height == 0 - upload.errors.add(:base, I18n.t("upload.images.size_not_found")) - else - upload.width, upload.height = ImageSizer.resize(width, height) - end - else - # fix orientation first - Upload.fix_image_orientation(file.path) - # retrieve image info - image_info = FastImage.new(file, raise_on_failure: true) - # compute image aspect ratio - upload.width, upload.height = ImageSizer.resize(*image_info.size) - end - # make sure we're at the beginning of the file - # (FastImage and Nokogiri move the pointer) - file.rewind - rescue FastImage::ImageFetchFailure - upload.errors.add(:base, I18n.t("upload.images.fetch_failure")) - rescue FastImage::UnknownImageType - upload.errors.add(:base, I18n.t("upload.images.unknown_image_type")) - rescue FastImage::SizeNotFound - upload.errors.add(:base, I18n.t("upload.images.size_not_found")) - end - - upload - end - def self.get_from_url(url) return if url.blank? # we store relative urls, so we need to remove any host/cdn diff --git a/app/models/user_avatar.rb b/app/models/user_avatar.rb index bc33d1889..a2e6d1e63 100644 --- a/app/models/user_avatar.rb +++ b/app/models/user_avatar.rb @@ -20,7 +20,7 @@ class UserAvatar < ActiveRecord::Base max = Discourse.avatar_sizes.max gravatar_url = "http://www.gravatar.com/avatar/#{email_hash}.png?s=#{max}&d=404" tempfile = FileHelper.download(gravatar_url, SiteSetting.max_image_size_kb.kilobytes, "gravatar") - upload = Upload.create_for(user.id, tempfile, 'gravatar.png', tempfile.size, { origin: gravatar_url }) + upload = Upload.create_for(user.id, tempfile, 'gravatar.png', tempfile.size, origin: gravatar_url, image_type: "avatar") if gravatar_upload_id != upload.id gravatar_upload.try(:destroy!) @@ -68,7 +68,7 @@ class UserAvatar < ActiveRecord::Base ext = FastImage.type(tempfile).to_s tempfile.rewind - upload = Upload.create_for(user.id, tempfile, "external-avatar." + ext, tempfile.size, { origin: avatar_url }) + upload = Upload.create_for(user.id, tempfile, "external-avatar." + ext, tempfile.size, origin: avatar_url, image_type: "avatar") user.uploaded_avatar_id = upload.id unless user.user_avatar diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 177e0e55d..f267c054d 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2055,8 +2055,6 @@ en: too_large: "Sorry, the file you are trying to upload is too big (maximum size is %{max_size_kb}KB)." images: too_large: "Sorry, the image you are trying to upload is too big (maximum size is %{max_size_kb}KB), please resize it and try again." - fetch_failure: "Sorry, there has been an error while fetching the image." - unknown_image_type: "Sorry, but the file you tried to upload doesn't appear to be an image." size_not_found: "Sorry, but we couldn't determine the size of the image. Maybe your image is corrupted?" flag_reason: diff --git a/lib/image_sizer.rb b/lib/image_sizer.rb index 3fa016004..cc4ee8a6b 100644 --- a/lib/image_sizer.rb +++ b/lib/image_sizer.rb @@ -1,18 +1,18 @@ module ImageSizer # Resize an image to the aspect ratio we want - def self.resize(width, height) + def self.resize(width, height, opts = {}) return if width.blank? || height.blank? - max_width = SiteSetting.max_image_width.to_f - max_height = SiteSetting.max_image_height.to_f + max_width = (opts[:max_width] || SiteSetting.max_image_width).to_f + max_height = (opts[:max_height] || SiteSetting.max_image_height).to_f w = width.to_f h = height.to_f return [w.floor, h.floor] if w <= max_width && h <= max_height - ratio = [max_width / w, max_height / h].min; + ratio = [max_width / w, max_height / h].min [(w * ratio).floor, (h * ratio).floor] end diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 24bc176be..a94951d18 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -303,8 +303,11 @@ This is a link http://example.com" let(:upload_sha) { '04df605be528d03876685c52166d4b063aabb78a' } it "creates a post with an attachment" do + Upload.stubs(:fix_image_orientation) + ImageOptim.any_instance.stubs(:optimize_image!) + start_count = topic.posts.count - Upload.find_by(sha1: upload_sha).try :destroy + Upload.find_by(sha1: upload_sha).try(:destroy) receiver.process diff --git a/spec/fixtures/images/logo-dev.png b/spec/fixtures/images/logo-dev.png index 7cbeb6d6c..b8475b693 100644 Binary files a/spec/fixtures/images/logo-dev.png and b/spec/fixtures/images/logo-dev.png differ diff --git a/spec/fixtures/images/logo.png b/spec/fixtures/images/logo.png index d09fee186..5c900b61f 100644 Binary files a/spec/fixtures/images/logo.png and b/spec/fixtures/images/logo.png differ diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 1e41637fe..2e3d17c6b 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -45,7 +45,10 @@ describe Upload do context "#create_for" do - before { Upload.stubs(:fix_image_orientation) } + before do + Upload.stubs(:fix_image_orientation) + ImageOptim.any_instance.stubs(:optimize_image!) + end it "does not create another upload if it already exists" do Upload.expects(:find_by).with(sha1: image_sha1).returns(upload) @@ -65,13 +68,6 @@ describe Upload do Upload.create_for(user_id, image, image_filename, image_filesize) end - it "does not create an upload when there is an error with FastImage" do - FileHelper.expects(:is_image?).returns(true) - Upload.expects(:save).never - upload = Upload.create_for(user_id, attachment, attachment_filename, attachment_filesize) - expect(upload.errors.size).to be > 0 - end - it "does not compute width & height for non-image" do FastImage.any_instance.expects(:size).never upload = Upload.create_for(user_id, attachment, attachment_filename, attachment_filesize)