From 9be9c5f5658489325b0bd1068683d86d3a4d20b7 Mon Sep 17 00:00:00 2001
From: Toshi MARUYAMA <marutosijp2@yahoo.co.jp>
Date: Wed, 23 Nov 2011 05:30:53 +0000
Subject: [PATCH] fix inconsistent image filename extensions (#9638)

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@7891 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/helpers/application_helper.rb            |  2 +-
 app/models/attachment.rb                     |  2 +-
 test/test_helper.rb                          | 11 ++++
 test/unit/helpers/application_helper_test.rb | 53 ++++++++++++++++++++
 4 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 56807adb3..9573a3291 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -537,7 +537,7 @@ module ApplicationHelper
     # when using an image link, try to use an attachment, if possible
     if options[:attachments] || (obj && obj.respond_to?(:attachments))
       attachments = nil
-      text.gsub!(/src="([^\/"]+\.(bmp|gif|jpg|jpeg|png))"(\s+alt="([^"]*)")?/i) do |m|
+      text.gsub!(/src="([^\/"]+\.(bmp|gif|jpg|jpe|jpeg|png))"(\s+alt="([^"]*)")?/i) do |m|
         filename, ext, alt, alttext = $1.downcase, $2, $3, $4
         attachments ||= (options[:attachments] || obj.attachments).sort_by(&:created_on).reverse
         # search for the picture in attachments
diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index dceb67893..c5a5187d1 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -123,7 +123,7 @@ class Attachment < ActiveRecord::Base
   end
 
   def image?
-    self.filename =~ /\.(jpe?g|gif|png)$/i
+    self.filename =~ /\.(bmp|gif|jpg|jpe|jpeg|png)$/i
   end
 
   def is_text?
diff --git a/test/test_helper.rb b/test/test_helper.rb
index cf79cf209..78b83c06d 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -77,6 +77,17 @@ class ActiveSupport::TestCase
     self.class.mock_file
   end
 
+  def mock_file_with_options(options={})
+    file = ''
+    file.stubs(:size).returns(32)
+    original_filename = options[:original_filename] || nil
+    file.stubs(:original_filename).returns(original_filename)
+    content_type = options[:content_type] || nil
+    file.stubs(:content_type).returns(content_type)
+    file.stubs(:read).returns(false)
+    file
+  end
+
   # Use a temporary directory for attachment related tests
   def set_tmp_attachments_directory
     Dir.mkdir "#{Rails.root}/tmp/test" unless File.directory?("#{Rails.root}/tmp/test")
diff --git a/test/unit/helpers/application_helper_test.rb b/test/unit/helpers/application_helper_test.rb
index a154729f3..131959a25 100644
--- a/test/unit/helpers/application_helper_test.rb
+++ b/test/unit/helpers/application_helper_test.rb
@@ -129,6 +129,59 @@ RAW
     to_test.each { |text, result| assert_equal "<p>#{result}</p>", textilizable(text, :attachments => attachments) }
   end
 
+  def test_attached_images_filename_extension
+    set_tmp_attachments_directory
+    a1 = Attachment.new(
+            :container => Issue.find(1),
+            :file => mock_file_with_options({:original_filename => "testtest.JPG"}),
+            :author => User.find(1))
+    assert a1.save
+    assert_equal "testtest.JPG", a1.filename
+    assert_equal "image/jpeg", a1.content_type
+    assert a1.image?
+
+    a2 = Attachment.new(
+            :container => Issue.find(1),
+            :file => mock_file_with_options({:original_filename => "testtest.jpeg"}),
+            :author => User.find(1))
+    assert a2.save
+    assert_equal "testtest.jpeg", a2.filename
+    assert_equal "image/jpeg", a2.content_type
+    assert a2.image?
+
+    a3 = Attachment.new(
+            :container => Issue.find(1),
+            :file => mock_file_with_options({:original_filename => "testtest.JPE"}),
+            :author => User.find(1))
+    assert a3.save
+    assert_equal "testtest.JPE", a3.filename
+    assert_equal "image/jpeg", a3.content_type
+    assert a3.image?
+
+    a4 = Attachment.new(
+            :container => Issue.find(1),
+            :file => mock_file_with_options({:original_filename => "Testtest.BMP"}),
+            :author => User.find(1))
+    assert a4.save
+    assert_equal "Testtest.BMP", a4.filename
+    assert_equal "image/x-ms-bmp", a4.content_type
+    assert a4.image?
+
+    to_test = {
+      'Inline image: !testtest.jpg!' =>
+        'Inline image: <img src="/attachments/download/' + a1.id.to_s + '" alt="" />',
+      'Inline image: !testtest.jpeg!' =>
+        'Inline image: <img src="/attachments/download/' + a2.id.to_s + '" alt="" />',
+      'Inline image: !testtest.jpe!' =>
+        'Inline image: <img src="/attachments/download/' + a3.id.to_s + '" alt="" />',
+      'Inline image: !testtest.bmp!' =>
+        'Inline image: <img src="/attachments/download/' + a4.id.to_s + '" alt="" />',
+    }
+
+    attachments = [a1, a2, a3, a4]
+    to_test.each { |text, result| assert_equal "<p>#{result}</p>", textilizable(text, :attachments => attachments) }
+  end
+
   def test_textile_external_links
     to_test = {
       'This is a "link":http://foo.bar' => 'This is a <a href="http://foo.bar" class="external">link</a>',
-- 
2.39.5