FIX: do not treat TIFF, BMP, WEBP as images

approved

#1

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.

diff --git a/app/assets/javascripts/discourse/lib/utilities.js.es6 b/app/assets/javascripts/discourse/lib/utilities.js.es6
index e71223f..3ce16c3 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 8da1f87..fbcc562 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 0aa4af8..521b405 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 fae1ab4..e04266e 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 4c832a8..4a52540 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 d528074..f19ea0e 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(""));

GitHub sha: c50db76f


Approved #2