jro_apps/redmine Commit - r7797:902b3078d549

Limit the characters stripped by Attachment#sanitize_filename (#4324)....

Jean-Philippe Lang -

r7797:902b3078d549

parent child

Context file:

r7797:902b3078d549

Collapse all files

app/models/attachment.rb +2 -4

             # Redmine - project management software
             # Copyright (C) 2006-2011  Jean-Philippe Lang
             #
             # This program is free software; you can redistribute it and/or
             # modify it under the terms of the GNU General Public License
             # as published by the Free Software Foundation; either version 2
             # of the License, or (at your option) any later version.
             #
             # This program is distributed in the hope that it will be useful,
             # but WITHOUT ANY WARRANTY; without even the implied warranty of
             # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
             # GNU General Public License for more details.
             #
             # You should have received a copy of the GNU General Public License
             # along with this program; if not, write to the Free Software
             # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
             require "digest/md5"
             class Attachment < ActiveRecord::Base
               belongs_to :container, :polymorphic => true
               belongs_to :author, :class_name => "User", :foreign_key => "author_id"
               validates_presence_of :container, :filename, :author
               validates_length_of :filename, :maximum => 255
               validates_length_of :disk_filename, :maximum => 255
               validate :validate_max_file_size
               acts_as_event :title => :filename,
                             :url => Proc.new {|o| {:controller => 'attachments', :action => 'download', :id => o.id, :filename => o.filename}}
               acts_as_activity_provider :type => 'files',
                                         :permission => :view_files,
                                         :author_key => :author_id,
                                         :find_options => {:select => "#{Attachment.table_name}.*",
                                                           :joins => "LEFT JOIN #{Version.table_name} ON #{Attachment.table_name}.container_type='Version' AND #{Version.table_name}.id = #{Attachment.table_name}.container_id " +
                                                                     "LEFT JOIN #{Project.table_name} ON #{Version.table_name}.project_id = #{Project.table_name}.id OR ( #{Attachment.table_name}.container_type='Project' AND #{Attachment.table_name}.container_id = #{Project.table_name}.id )"}
               acts_as_activity_provider :type => 'documents',
                                         :permission => :view_documents,
                                         :author_key => :author_id,
                                         :find_options => {:select => "#{Attachment.table_name}.*",
                                                           :joins => "LEFT JOIN #{Document.table_name} ON #{Attachment.table_name}.container_type='Document' AND #{Document.table_name}.id = #{Attachment.table_name}.container_id " +
                                                                     "LEFT JOIN #{Project.table_name} ON #{Document.table_name}.project_id = #{Project.table_name}.id"}
               cattr_accessor :storage_path
               @@storage_path = Redmine::Configuration['attachments_storage_path'] || "#{Rails.root}/files"
               before_save :files_to_final_location
               after_destroy :delete_from_disk
               def validate_max_file_size
                 if self.filesize > Setting.attachment_max_size.to_i.kilobytes
                   errors.add(:base, :too_long, :count => Setting.attachment_max_size.to_i.kilobytes)
                 end
               end
               def file=(incoming_file)
                 unless incoming_file.nil?
                   @temp_file = incoming_file
                   if @temp_file.size > 0
                     self.filename = sanitize_filename(@temp_file.original_filename)
                     self.disk_filename = Attachment.disk_filename(filename)
                     self.content_type = @temp_file.content_type.to_s.chomp
                     if content_type.blank?
                       self.content_type = Redmine::MimeType.of(filename)
                     end
                     self.filesize = @temp_file.size
                   end
                 end
               end
               def file
                 nil
               end
               # Copies the temporary file to its final location
               # and computes its MD5 hash
               def files_to_final_location
                 if @temp_file && (@temp_file.size > 0)
                   logger.info("Saving attachment '#{self.diskfile}' (#{@temp_file.size} bytes)")
                   md5 = Digest::MD5.new
                   File.open(diskfile, "wb") do |f|
                     buffer = ""
                     while (buffer = @temp_file.read(8192))
                       f.write(buffer)
                       md5.update(buffer)
                     end
                   end
                   self.digest = md5.hexdigest
                 end
                 @temp_file = nil
                 # Don't save the content type if it's longer than the authorized length
                 if self.content_type && self.content_type.length > 255
                   self.content_type = nil
                 end
               end
               # Deletes file on the disk
               def delete_from_disk
                 File.delete(diskfile) if !filename.blank? && File.exist?(diskfile)
               end
               # Returns file's location on disk
               def diskfile
                 "#{@@storage_path}/#{self.disk_filename}"
               end
               def increment_download
                 increment!(:downloads)
               end
               def project
                 container.project
               end
               def visible?(user=User.current)
                 container.attachments_visible?(user)
               end
               def deletable?(user=User.current)
                 container.attachments_deletable?(user)
               end
               def image?
                 self.filename =~ /\.(bmp|gif|jpg|jpe|jpeg|png)$/i
               end
               def is_text?
                 Redmine::MimeType.is_type?('text', filename)
               end
               def is_diff?
                 self.filename =~ /\.(patch|diff)$/i
               end
               # Returns true if the file is readable
               def readable?
                 File.readable?(diskfile)
               end
               # Bulk attaches a set of files to an object
               #
               # Returns a Hash of the results:
               # :files => array of the attached files
               # :unsaved => array of the files that could not be attached
               def self.attach_files(obj, attachments)
                 attached = []
                 if attachments && attachments.is_a?(Hash)
                   attachments.each_value do |attachment|
                     file = attachment['file']
                     next unless file && file.size > 0
                     a = Attachment.create(:container => obj,
                                           :file => file,
                                           :description => attachment['description'].to_s.strip,
                                           :author => User.current)
                     obj.attachments << a
                     if a.new_record?
                       obj.unsaved_attachments ||= []
                       obj.unsaved_attachments << a
                     else
                       attached << a
                     end
                   end
                 end
                 {:files => attached, :unsaved => obj.unsaved_attachments}
               end
               def self.latest_attach(attachments, filename)
                 attachments.sort_by(&:created_on).reverse.detect {
                   |att| att.filename.downcase == filename.downcase
                  }
               end
             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
+                # Finally, replace invalid characters with underscore
-                @filename = just_filename.gsub(/[^\w\.\-]/,'_')
+                @filename = just_filename.gsub(/[\/\?\%\*\:\|\"\'<>]+/, '_')
               end
               # Returns an ASCII or hashed filename
               def self.disk_filename(filename)
                 timestamp = DateTime.now.strftime("%y%m%d%H%M%S")
                 ascii = ''
                 if filename =~ %r{^[a-zA-Z0-9_\.\-]*$}
                   ascii = filename
                 else
                   ascii = Digest::MD5.hexdigest(filename)
                   # keep the extension if any
                   ascii << $1 if filename =~ %r{(\.[a-zA-Z0-9]+)$}
                 end
                 while File.exist?(File.join(@@storage_path, "#{timestamp}_#{ascii}"))
                   timestamp.succ!
                 end
                 "#{timestamp}_#{ascii}"
               end
             end

test/unit/attachment_test.rb +21 0

             # encoding: utf-8
             #
             # Redmine - project management software
             # Copyright (C) 2006-2011  Jean-Philippe Lang
             #
             # This program is free software; you can redistribute it and/or
             # modify it under the terms of the GNU General Public License
             # as published by the Free Software Foundation; either version 2
             # of the License, or (at your option) any later version.
             #
             # This program is distributed in the hope that it will be useful,
             # but WITHOUT ANY WARRANTY; without even the implied warranty of
             # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
             # GNU General Public License for more details.
             #
             # You should have received a copy of the GNU General Public License
             # along with this program; if not, write to the Free Software
             # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
             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
               end
               def test_create
                 a = Attachment.new(:container => Issue.find(1),
                                    :file => uploaded_test_file("testfile.txt", "text/plain"),
                                    :author => User.find(1))
                 assert a.save
                 assert_equal 'testfile.txt', a.filename
                 assert_equal 59, a.filesize
                 assert_equal 'text/plain', a.content_type
                 assert_equal 0, a.downloads
                 assert_equal '1478adae0d4eb06d35897518540e25d6', a.digest
                 assert File.exist?(a.diskfile)
                 assert_equal 59, File.size(a.diskfile)
               end
               def test_destroy
                 a = Attachment.new(:container => Issue.find(1),
                                    :file => uploaded_test_file("testfile.txt", "text/plain"),
                                    :author => User.find(1))
                 assert a.save
                 assert_equal 'testfile.txt', a.filename
                 assert_equal 59, a.filesize
                 assert_equal 'text/plain', a.content_type
                 assert_equal 0, a.downloads
                 assert_equal '1478adae0d4eb06d35897518540e25d6', a.digest
                 diskfile = a.diskfile
                 assert File.exist?(diskfile)
                 assert_equal 59, File.size(a.diskfile)
                 assert a.destroy
                 assert !File.exist?(diskfile)
               end
               def test_create_should_auto_assign_content_type
                 a = Attachment.new(:container => Issue.find(1),
                                    :file => uploaded_test_file("testfile.txt", ""),
                                    :author => User.find(1))
                 assert a.save
                 assert_equal 'text/plain', a.content_type
               end
               def test_identical_attachments_at_the_same_time_should_not_overwrite
                 a1 = Attachment.create!(:container => Issue.find(1),
                                         :file => uploaded_test_file("testfile.txt", ""),
                                         :author => User.find(1))
                 a2 = Attachment.create!(:container => Issue.find(1),
                                         :file => uploaded_test_file("testfile.txt", ""),
                                         :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$/
                 assert_equal 'test_file.txt', Attachment.disk_filename("test_file.txt")[13..-1]
                 assert_equal '770c509475505f37c2b8fb6030434d6b.txt', Attachment.disk_filename("test_accentué.txt")[13..-1]
                 assert_equal 'f8139524ebb8f32e51976982cd20a85d', Attachment.disk_filename("test_accentué")[13..-1]
                 assert_equal 'cbb5b0f30978ba03731d61f9f6d10011', Attachment.disk_filename("test_accentué.ça")[13..-1]
               end
               context "Attachmnet.attach_files" do
                 should "attach the file" do
                   issue = Issue.first
                   assert_difference 'Attachment.count' do
                     Attachment.attach_files(issue,
                       '1' => {
                         'file' => uploaded_test_file('testfile.txt', 'text/plain'),
                         'description' => 'test'
                       })
                   end
                   attachment = Attachment.first(:order => 'id DESC')
                   assert_equal issue, attachment.container
                   assert_equal 'testfile.txt', attachment.filename
                   assert_equal 59, attachment.filesize
                   assert_equal 'test', attachment.description
                   assert_equal 'text/plain', attachment.content_type
                   assert File.exists?(attachment.diskfile)
                   assert_equal 59, File.size(attachment.diskfile)
                 end
                 should "add unsaved files to the object as unsaved attachments" do
                   # Max size of 0 to force Attachment creation failures
                   with_settings(:attachment_max_size => 0) do
                     @project = Project.generate!
                     response = Attachment.attach_files(@project, {
                                                          '1' => {'file' => mock_file, 'description' => 'test'},
                                                          '2' => {'file' => mock_file, 'description' => 'test'}
                                                        })
                     assert response[:unsaved].present?
                     assert_equal 2, response[:unsaved].length
                     assert response[:unsaved].first.new_record?
                     assert response[:unsaved].second.new_record?
                     assert_equal response[:unsaved], @project.unsaved_attachments
                   end
                 end
               end
               def test_latest_attach
                 Attachment.storage_path = "#{Rails.root}/test/fixtures/files"
                 a1 = Attachment.find(16)
                 assert_equal "testfile.png", a1.filename
                 assert a1.readable?
                 assert (! a1.visible?(User.anonymous))
                 assert a1.visible?(User.find(2))
                 a2 = Attachment.find(17)
                 assert_equal "testfile.PNG", a2.filename
                 assert a2.readable?
                 assert (! a2.visible?(User.anonymous))
                 assert a2.visible?(User.find(2))
                 assert a1.created_on < a2.created_on
                 la1 = Attachment.latest_attach([a1, a2], "testfile.png")
                 assert_equal 17, la1.id
                 la2 = Attachment.latest_attach([a1, a2], "Testfile.PNG")
                 assert_equal 17, la2.id
                 set_tmp_attachments_directory
               end
             end

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages