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

@@ -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

test/unit/attachment_test.rb

@@ -23,6 +23,17 @@ 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
   end
@@ -76,6 +87,16 @@ class AttachmentTest < ActiveSupport::TestCase
     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$/
     assert_equal 'test_file.txt', Attachment.disk_filename("test_file.txt")[13..-1]