Don't allow directory listings (better fix for and closes #340).

2 files changed, 13 insertions, 13 deletions

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 8714f03cf..1c7aeedcc 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -604,15 +604,12 @@ class RequestController < ApplicationController
 
     before_filter :authenticate_attachment, :only => [ :get_attachment, :get_attachment_as_html ]
     def authenticate_attachment
-        if request.path =~ /\/$/ || !(params[:part] =~ /^\d+$/)
-            raise PermissionDenied.new("Directory listing not allowed")
-        else
-            # Test for hidden
-            incoming_message = IncomingMessage.find(params[:incoming_message_id])
-            if !incoming_message.info_request.user_can_view?(authenticated_user)
-                @info_request = incoming_message.info_request # used by view
-                render :template => 'request/hidden', :status => 410 # gone
-            end
+        # Test for hidden
+        incoming_message = IncomingMessage.find(params[:incoming_message_id])
+        raise ActiveRecord::RecordNotFound.new("Message not found") if incoming_message.nil?
+        if !incoming_message.info_request.user_can_view?(authenticated_user)
+            @info_request = incoming_message.info_request # used by view
+            render :template => 'request/hidden', :status => 410 # gone
         end
     end
 
@@ -624,8 +621,8 @@ class RequestController < ApplicationController
         else
             key = params.merge(:only_path => true)
             key_path = foi_fragment_cache_path(key)
-
             if foi_fragment_cache_exists?(key_path)
+                raise PermissionDenied.new("Directory listing not allowed") if File.directory?(key_path)
                 cached = foi_fragment_cache_read(key_path)
                 response.content_type = AlaveteliFileTypes.filename_to_mimetype(params[:file_name].join("/")) || 'application/octet-stream'
                 render_for_text(cached)
diff --git a/spec/integration/errors_spec.rb b/spec/integration/errors_spec.rb
index 705c1fff8..d03323445 100644
--- a/spec/integration/errors_spec.rb
+++ b/spec/integration/errors_spec.rb
@@ -46,10 +46,13 @@ describe "When rendering errors" do
         response.code.should == "500"
     end
     it "should render a 403 for attempts at directory listing for attachments" do
-        get("/request/5/response/4/attach/html/3/" )
-        response.code.should == "403"
-        get("/request/5/response/4/attach/html" )
+        # make a fake cache
+        foi_cache_path = File.join(File.dirname(__FILE__), '../../cache')
+        FileUtils.mkdir_p(File.join(foi_cache_path, "views/en/request/101/101/response/1/attach/html/1"))
+        get("/request/101/response/1/attach/html/1/" )
         response.code.should == "403"
+        get("/request/101/response/1/attach/html" )
+        response.code.should == "403" 
     end
     it "should render a 404 for non-existent 'details' pages for requests" do
         get("/details/request/wobble" )