diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb index d7f09460883..a263955c51d 100644 --- a/app/controllers/attachments_controller.rb +++ b/app/controllers/attachments_controller.rb @@ -13,11 +13,11 @@ class AttachmentsController < ApplicationController include ProminenceHeaders - around_action :cache_attachments - before_action :authenticate_attachment before_action :authenticate_attachment_as_html, only: :show_as_html + around_action :cache_attachments + def show # Prevent spam to magic request address. Note that the binary # substitution method used depends on the content type @@ -115,8 +115,12 @@ def authenticate_attachment return if @attachment - # If we can't find the right attachment, redirect to the incoming message: - redirect_to incoming_message_url(@incoming_message), status: 303 + if params[:file_name] + # If we can't find the right attachment, redirect to the incoming message: + redirect_to incoming_message_url(@incoming_message), status: 303 + else + render plain: 'Directory listing not allowed', status: 403 + end end def authenticate_attachment_as_html @@ -136,12 +140,8 @@ def cache_attachments if foi_fragment_cache_exists?(cache_key_path) logger.info("Reading cache for #{cache_key_path}") - if File.directory?(cache_key_path) - render plain: 'Directory listing not allowed', status: 403 - else - render body: foi_fragment_cache_read(cache_key_path), - content_type: content_type - end + render body: foi_fragment_cache_read(cache_key_path), + content_type: content_type return end @@ -166,6 +166,8 @@ def part_number def original_filename filename = params[:file_name] + return unless filename + if action_name == 'show_as_html' filename.gsub(/\.html$/, '') else diff --git a/config/routes.rb b/config/routes.rb index 9099ebfa4ec..51fee645d42 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -110,14 +110,16 @@ :as => :similar_request, :via => :get - match '/request/:id/response/:incoming_message_id/attach/html/:part/*file_name' => 'attachments#show_as_html', + match '/request/:id/response/:incoming_message_id/attach/html/(:part(/*file_name))' => 'attachments#show_as_html', :format => false, :as => :get_attachment_as_html, - :via => :get + :via => :get, + :constraints => { :part => /\d+/ } match '/request/:id/response/:incoming_message_id/attach/:part(/*file_name)' => 'attachments#show', :format => false, :as => :get_attachment, - :via => :get + :via => :get, + :constraints => { :part => /\d+/ } match '/request_event/:info_request_event_id' => 'request#show_request_event', :as => :info_request_event, diff --git a/spec/integration/errors_spec.rb b/spec/integration/errors_spec.rb index 3d008179166..6fb651ac8c0 100644 --- a/spec/integration/errors_spec.rb +++ b/spec/integration/errors_spec.rb @@ -99,7 +99,7 @@ # make a fake cache cache_key_path = Rails.root.join( - "cache/views/request/#{prefix}/#{id}/response/#{msg_id}/attach/0/1" + "cache/views/request/#{prefix}/#{id}/response/#{msg_id}/attach/html/1" ) FileUtils.mkdir_p(cache_key_path)