diff options
| -rw-r--r-- | app/controllers/attachments_controller.rb | 7 | ||||
| -rw-r--r-- | app/models/attachment.rb | 5 | ||||
| -rw-r--r-- | test/functional/attachments_controller_test.rb | 9 |
3 files changed, 18 insertions, 3 deletions
diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb index 445ae3d12..92d60ee0f 100644 --- a/app/controllers/attachments_controller.rb +++ b/app/controllers/attachments_controller.rb @@ -17,7 +17,7 @@ class AttachmentsController < ApplicationController before_filter :find_project - before_filter :read_authorize, :except => :destroy + before_filter :file_readable, :read_authorize, :except => :destroy before_filter :delete_authorize, :only => :destroy verify :method => :post, :only => :destroy @@ -64,6 +64,11 @@ private render_404 end + # Checks that the file exists and is readable + def file_readable + @attachment.readable? ? true : render_404 + end + def read_authorize @attachment.visible? ? true : deny_access end diff --git a/app/models/attachment.rb b/app/models/attachment.rb index 97471d739..fe1d6afaa 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -126,6 +126,11 @@ class Attachment < ActiveRecord::Base self.filename =~ /\.(patch|diff)$/i end + # Returns true if the file is readable + def readable? + File.readable?(diskfile) + end + private def sanitize_filename(value) # get only the filename, not the whole path diff --git a/test/functional/attachments_controller_test.rb b/test/functional/attachments_controller_test.rb index 6738afbcf..a49caa876 100644 --- a/test/functional/attachments_controller_test.rb +++ b/test/functional/attachments_controller_test.rb @@ -23,8 +23,8 @@ class AttachmentsController; def rescue_action(e) raise e end; end class AttachmentsControllerTest < Test::Unit::TestCase - fixtures :users, :projects, :roles, :members, :enabled_modules, :issues, :attachments, - :versions, :wiki_pages, :wikis + fixtures :users, :projects, :roles, :members, :enabled_modules, :issues, :trackers, :attachments, + :versions, :wiki_pages, :wikis, :documents def setup @controller = AttachmentsController.new @@ -84,6 +84,11 @@ class AttachmentsControllerTest < Test::Unit::TestCase assert_equal 'application/x-ruby', @response.content_type end + def test_download_missing_file + get :download, :id => 2 + assert_response 404 + end + def test_anonymous_on_private_private get :download, :id => 7 assert_redirected_to '/login?back_url=http%3A%2F%2Ftest.host%2Fattachments%2Fdownload%2F7' |