From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sat, 25 Apr 2009 09:31:36 +0000 (+0000)
Subject: Returns a 404 error when trying to view/download an attachment that can't be read... 
X-Git-Tag: 0.9.0~504
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=15a14e55cdb9b8c2633786fa262382b6e1f7ea9f;p=redmine.git

Returns a 404 error when trying to view/download an attachment that can't be read from disk.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@2692 e93f8b46-1217-0410-a6f0-8f06a7374b81
---

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'