From c50db76f5d8f409cc5d22a440bbc40b9f25f2a4c Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 11 Feb 2019 00:28:43 -0500 Subject: [PATCH] FIX: do not treat TIFF, BMP, WEBP as images Treating TIFF and BMP as images cause us to add them to IMG tags, this is very inconsistent across browsers. You can still upload these files they will simply not be displayed in IMG tags. --- .../javascripts/discourse/lib/utilities.js.es6 | 6 +++--- app/models/optimized_image.rb | 3 +-- lib/upload_creator.rb | 8 +++----- script/downsize_uploads.rb | 2 +- spec/models/optimized_image_spec.rb | 4 ++-- test/javascripts/lib/utilities-test.js.es6 | 18 ++++++++---------- 6 files changed, 18 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/utilities.js.es6 b/app/assets/javascripts/discourse/lib/utilities.js.es6 index e71223f337d..3ce16c30dd8 100644 --- a/app/assets/javascripts/discourse/lib/utilities.js.es6 +++ b/app/assets/javascripts/discourse/lib/utilities.js.es6 @@ -282,7 +282,7 @@ export function validateUploadedFile(file, opts) { return true; } -const IMAGES_EXTENSIONS_REGEX = /(png|jpe?g|gif|bmp|tiff?|svg|webp|ico)/i; +const IMAGES_EXTENSIONS_REGEX = /(png|jpe?g|gif|svg|ico)/i; function extensionsToArray(exts) { return exts @@ -348,7 +348,7 @@ export function authorizedExtensions() { export function authorizedImagesExtensions() { return authorizesAllExtensions() - ? "png, jpg, jpeg, gif, bmp, tiff, svg, webp, ico" + ? "png, jpg, jpeg, gif, svg, ico" : imagesExtensions().join(", "); } @@ -376,7 +376,7 @@ export function authorizesOneOrMoreImageExtensions() { } export function isAnImage(path) { - return /\.(png|jpe?g|gif|bmp|tiff?|svg|webp|ico)$/i.test(path); + return /\.(png|jpe?g|gif|svg|ico)$/i.test(path); } function uploadTypeFromFileName(fileName) { diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb index 8da1f8753d0..fbcc5621d0f 100644 --- a/app/models/optimized_image.rb +++ b/app/models/optimized_image.rb @@ -29,7 +29,6 @@ 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? @@ -180,7 +179,7 @@ class OptimizedImage < ActiveRecord::Base end end - IM_DECODERS ||= /\A(jpe?g|png|tiff?|bmp|ico|gif)\z/i + IM_DECODERS ||= /\A(jpe?g|png|ico|gif)\z/i def self.prepend_decoder!(path, ext_path = nil, opts = nil) opts ||= {} diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb index 0aa4af84b15..521b405d552 100644 --- a/lib/upload_creator.rb +++ b/lib/upload_creator.rb @@ -3,8 +3,6 @@ require_dependency "image_sizer" class UploadCreator - TYPES_CONVERTED_TO_JPEG ||= %i{bmp png} - TYPES_TO_CROP ||= %w{avatar card_background custom_emoji profile_background}.each(&:freeze) WHITELISTED_SVG_ELEMENTS ||= %w{ @@ -47,7 +45,7 @@ class UploadCreator if @image_info.type.to_s == "svg" whitelist_svg! elsif !Rails.env.test? || @opts[:force_optimize] - convert_to_jpeg! if should_convert_to_jpeg? + convert_to_jpeg! if convert_png_to_jpeg? downsize! if should_downsize? return @upload if is_still_too_big? @@ -158,8 +156,8 @@ class UploadCreator MIN_PIXELS_TO_CONVERT_TO_JPEG ||= 1280 * 720 - def should_convert_to_jpeg? - return false if !TYPES_CONVERTED_TO_JPEG.include?(@image_info.type) + def convert_png_to_jpeg? + return false unless @image_info.type == :png return true if @opts[:pasted] return false if SiteSetting.png_to_jpg_quality == 100 pixels > MIN_PIXELS_TO_CONVERT_TO_JPEG diff --git a/script/downsize_uploads.rb b/script/downsize_uploads.rb index fae1ab4374e..e04266e3f04 100644 --- a/script/downsize_uploads.rb +++ b/script/downsize_uploads.rb @@ -7,7 +7,7 @@ puts '', "Downsizing uploads size to no more than #{max_image_pixels} pixels" count = 0 -Upload.where("lower(extension) in (?)", ['jpg', 'jpeg', 'gif', 'png', 'bmp', 'tif', 'tiff']).find_each do |upload| +Upload.where("lower(extension) in (?)", ['jpg', 'jpeg', 'gif', 'png']).find_each do |upload| count += 1 print "\r%8d".freeze % count absolute_path = Discourse.store.path_for(upload) diff --git a/spec/models/optimized_image_spec.rb b/spec/models/optimized_image_spec.rb index 4c832a8f4bf..4a525401610 100644 --- a/spec/models/optimized_image_spec.rb +++ b/spec/models/optimized_image_spec.rb @@ -155,8 +155,8 @@ describe OptimizedImage do describe ".safe_path?" do it "correctly detects unsafe paths" do - expect(OptimizedImage.safe_path?("/path/A-AA/22_00.TIFF")).to eq(true) - expect(OptimizedImage.safe_path?("/path/AAA/2200.TIFF")).to eq(true) + expect(OptimizedImage.safe_path?("/path/A-AA/22_00.JPG")).to eq(true) + expect(OptimizedImage.safe_path?("/path/AAA/2200.JPG")).to eq(true) expect(OptimizedImage.safe_path?("/tmp/a.png")).to eq(true) expect(OptimizedImage.safe_path?("../a.png")).to eq(false) expect(OptimizedImage.safe_path?("/tmp/a.png\\test")).to eq(false) diff --git a/test/javascripts/lib/utilities-test.js.es6 b/test/javascripts/lib/utilities-test.js.es6 index d5280741a53..f19ea0e804c 100644 --- a/test/javascripts/lib/utilities-test.js.es6 +++ b/test/javascripts/lib/utilities-test.js.es6 @@ -204,16 +204,14 @@ QUnit.test("replaces GUID in image alt text on iOS", assert => { }); QUnit.test("isAnImage", assert => { - ["png", "jpg", "jpeg", "bmp", "gif", "tif", "tiff", "ico"].forEach( - extension => { - var image = "image." + extension; - assert.ok(isAnImage(image), image + " is recognized as an image"); - assert.ok( - isAnImage("http://foo.bar/path/to/" + image), - image + " is recognized as an image" - ); - } - ); + ["png", "jpg", "jpeg", "gif", "ico"].forEach(extension => { + var image = "image." + extension; + assert.ok(isAnImage(image), image + " is recognized as an image"); + assert.ok( + isAnImage("http://foo.bar/path/to/" + image), + image + " is recognized as an image" + ); + }); assert.not(isAnImage("file.txt")); assert.not(isAnImage("http://foo.bar/path/to/file.txt")); assert.not(isAnImage(""));