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

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
15a14e55 · Jean-Philippe Lang · 914ef1cb · 15a14e55 · 15a14e55 · 15a14e55
Commit 15a14e55 authored Apr 25, 2009 by Jean-Philippe Lang
Showing with 18 additions and 3 deletions

attachments_controller.rb app/controllers/attachments_controller.rb +6 -1

attachment.rb app/models/attachment.rb +5 -0

attachments_controller_test.rb test/functional/attachments_controller_test.rb +7 -2

No files found.
--- 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

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

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