From 902b3078d549ad533ad26878de5e74bb318fe1ea Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Thu, 24 Nov 2011 20:17:56 +0000
Subject: [PATCH] Limit the characters stripped by Attachment#sanitize_filename
 (#4324).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@7917 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/models/attachment.rb     |  6 ++----
 test/unit/attachment_test.rb | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index 2dda3f353..09e4057cf 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -177,11 +177,9 @@ private
   def sanitize_filename(value)
     # get only the filename, not the whole path
     just_filename = value.gsub(/^.*(\\|\/)/, '')
-    # NOTE: File.basename doesn't work right with Windows paths on Unix
-    # INCORRECT: just_filename = File.basename(value.gsub('\\\\', '/'))
 
-    # Finally, replace all non alphanumeric, hyphens or periods with underscore
-    @filename = just_filename.gsub(/[^\w\.\-]/,'_')
+    # Finally, replace invalid characters with underscore
+    @filename = just_filename.gsub(/[\/\?\%\*\:\|\"\'<>]+/, '_')
   end
 
   # Returns an ASCII or hashed filename
diff --git a/test/unit/attachment_test.rb b/test/unit/attachment_test.rb
index 00519d663..0cdec7b0f 100644
--- a/test/unit/attachment_test.rb
+++ b/test/unit/attachment_test.rb
@@ -22,6 +22,17 @@ require File.expand_path('../../test_helper', __FILE__)
 class AttachmentTest < ActiveSupport::TestCase
   fixtures :users, :projects, :roles, :members, :member_roles,
            :enabled_modules, :issues, :trackers, :attachments
+  
+  class MockFile
+    attr_reader :original_filename, :content_type, :content, :size
+    
+    def initialize(attributes)
+      @original_filename = attributes[:original_filename]
+      @content_type = attributes[:content_type]
+      @content = attributes[:content] || "Content"
+      @size = content.size
+    end
+  end
 
   def setup
     set_tmp_attachments_directory
@@ -75,6 +86,16 @@ class AttachmentTest < ActiveSupport::TestCase
                             :author => User.find(1))
     assert a1.disk_filename != a2.disk_filename
   end
+  
+  def test_filename_should_be_basenamed
+    a = Attachment.new(:file => MockFile.new(:original_filename => "path/to/the/file"))
+    assert_equal 'file', a.filename
+  end
+  
+  def test_filename_should_be_sanitized
+    a = Attachment.new(:file => MockFile.new(:original_filename => "valid:[] invalid:?%*|\"'<>chars"))
+    assert_equal 'valid_[] invalid_chars', a.filename
+  end
 
   def test_diskfilename
     assert Attachment.disk_filename("test_file.txt") =~ /^\d{12}_test_file.txt$/