From 3910f7f545177cdb69a5ee0196ffa54a9dba0541 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 13 Dec 2012 12:16:46 +0000 Subject: Don't offer or allow viewing of an HTML version of a response attachment if the request is hidden, or requester_only. Google docs viewer won't be able to access it, and our own conversion process currently can produce image files that will then be publicly viewable directly from the webserver (see config/httpd.conf). If necessary we can revisit this code to enable admins and requesters to view the HTML version created by our own conversion without adding these files to a path that is served directly by the web server. --- app/controllers/request_controller.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'app/controllers/request_controller.rb') diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index c732a4b32..2c95114e6 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -743,6 +743,12 @@ class RequestController < ApplicationController end def get_attachment_as_html + + # The conversion process can generate files in the cache directory that can be served up + # directly by the webserver according to httpd.conf, so don't allow it unless that's OK. + if @files_can_be_cached != true + raise ActiveRecord::RecordNotFound.new("Attachment HTML not found.") + end get_attachment_internal(true) # images made during conversion (e.g. images in PDF files) are put in the cache directory, so -- cgit v1.2.3 From 482d0d351b48877ac1bf1e13678c5591997755b4 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 13 Dec 2012 19:05:17 +0000 Subject: Check that a request is publicly visible before generating a download link. --- app/controllers/request_controller.rb | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) (limited to 'app/controllers/request_controller.rb') diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 2c95114e6..6755afcda 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -868,13 +868,20 @@ class RequestController < ApplicationController def download_entire_request @locale = self.locale_from_params() PublicBody.with_locale(@locale) do - info_request = InfoRequest.find_by_url_title!(params[:url_title]) + @info_request = InfoRequest.find_by_url_title!(params[:url_title]) + # Test for whole request being hidden or requester-only + if !@info_request.all_can_view? + render :template => 'request/hidden', :status => 410 # gone + return + end if authenticated?( :web => _("To download the zip file"), - :email => _("Then you can download a zip file of {{info_request_title}}.",:info_request_title=>info_request.title), - :email_subject => _("Log in to download a zip file of {{info_request_title}}",:info_request_title=>info_request.title) + :email => _("Then you can download a zip file of {{info_request_title}}.", + :info_request_title=>@info_request.title), + :email_subject => _("Log in to download a zip file of {{info_request_title}}", + :info_request_title=>@info_request.title) ) - updated = Digest::SHA1.hexdigest(info_request.get_last_event.created_at.to_i.to_s + info_request.updated_at.to_i.to_s) + updated = Digest::SHA1.hexdigest(@info_request.get_last_event.created_at.to_i.to_s + @info_request.updated_at.to_i.to_s) @url_path = "/download/#{updated[0..1]}/#{updated}/#{params[:url_title]}.zip" file_path = File.expand_path(File.join(File.dirname(__FILE__), '../../cache/zips', @url_path)) if !File.exists?(file_path) @@ -883,7 +890,7 @@ class RequestController < ApplicationController convert_command = Configuration::html_to_pdf_command done = false if !convert_command.blank? && File.exists?(convert_command) - url = "http://#{Configuration::domain}#{request_url(info_request)}?print_stylesheet=1" + url = "http://#{Configuration::domain}#{request_url(@info_request)}?print_stylesheet=1" tempfile = Tempfile.new('foihtml2pdf') output = AlaveteliExternalCommand.run(convert_command, url, tempfile.path) if !output.nil? @@ -892,22 +899,21 @@ class RequestController < ApplicationController } done = true else - logger.error("Could not convert info request #{info_request.id} to PDF with command '#{convert_command} #{url} #{tempfile.path}'") + logger.error("Could not convert info request #{@info_request.id} to PDF with command '#{convert_command} #{url} #{tempfile.path}'") end tempfile.close else logger.warn("No HTML -> PDF converter found at #{convert_command}") end if !done - @info_request = info_request - @info_request_events = info_request.info_request_events + @info_request_events = @info_request.info_request_events template = File.read(File.join(File.dirname(__FILE__), "..", "views", "request", "simple_correspondence.rhtml")) output = ERB.new(template).result(binding) zipfile.get_output_stream("correspondence.txt") { |f| f.puts(output) } end - for message in info_request.incoming_messages + for message in @info_request.incoming_messages attachments = message.get_attachments_for_display for attachment in attachments filename = "#{attachment.url_part_number}_#{attachment.display_filename}" -- cgit v1.2.3 From 611411fc7907a97e3aa8c2339bc9f5b70a5d1a01 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 13 Dec 2012 19:11:48 +0000 Subject: Use helper method for download path, and use Rails.root instead of file location. --- app/controllers/request_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/controllers/request_controller.rb') diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 6755afcda..42cfa11c6 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -883,7 +883,7 @@ class RequestController < ApplicationController ) updated = Digest::SHA1.hexdigest(@info_request.get_last_event.created_at.to_i.to_s + @info_request.updated_at.to_i.to_s) @url_path = "/download/#{updated[0..1]}/#{updated}/#{params[:url_title]}.zip" - file_path = File.expand_path(File.join(File.dirname(__FILE__), '../../cache/zips', @url_path)) + file_path = File.expand_path(File.join(download_zip_dir(), @url_path)) if !File.exists?(file_path) FileUtils.mkdir_p(File.dirname(file_path)) Zip::ZipFile.open(file_path, Zip::ZipFile::CREATE) { |zipfile| -- cgit v1.2.3 From d2dc193066222b279faa52a66a22760e739dd87e Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 13 Dec 2012 20:15:54 +0000 Subject: Put download zips in a predictable location - sharded folders based on request ID, rather than distributing them across the download directories by the generated SHA. Preserve the uniqueness of the subdirectory. --- app/controllers/request_controller.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'app/controllers/request_controller.rb') diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 42cfa11c6..d8c34c2dd 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -882,7 +882,10 @@ class RequestController < ApplicationController :info_request_title=>@info_request.title) ) updated = Digest::SHA1.hexdigest(@info_request.get_last_event.created_at.to_i.to_s + @info_request.updated_at.to_i.to_s) - @url_path = "/download/#{updated[0..1]}/#{updated}/#{params[:url_title]}.zip" + @url_path = File.join("/download", + request_dirs(@info_request), + updated, + "#{params[:url_title]}.zip") file_path = File.expand_path(File.join(download_zip_dir(), @url_path)) if !File.exists?(file_path) FileUtils.mkdir_p(File.dirname(file_path)) -- cgit v1.2.3 From 153984fd2e6841f3b0bc62e25e1718800a7c63ed Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Mon, 17 Dec 2012 15:32:39 +0000 Subject: Only serve up 'similar' pages up to the offset we use for list. --- app/controllers/request_controller.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'app/controllers/request_controller.rb') diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index d8c34c2dd..5d950ceb2 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -139,6 +139,11 @@ class RequestController < ApplicationController short_cache @per_page = 25 @page = (params[:page] || "1").to_i + + # Later pages are very expensive to load + if @page > MAX_RESULTS / PER_PAGE + raise ActiveRecord::RecordNotFound.new("Sorry. No pages after #{MAX_RESULTS / PER_PAGE}.") + end @info_request = InfoRequest.find_by_url_title!(params[:url_title]) raise ActiveRecord::RecordNotFound.new("Request not found") if @info_request.nil? -- cgit v1.2.3 From 9eb02d20ace73f74161c7f0b02ff2c6567cf5125 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Mon, 17 Dec 2012 15:50:36 +0000 Subject: Limit pagination on similar pages in line with new upper limit on page offset. --- app/controllers/request_controller.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'app/controllers/request_controller.rb') diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 5d950ceb2..970dfca45 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -153,6 +153,8 @@ class RequestController < ApplicationController end @xapian_object = ::ActsAsXapian::Similar.new([InfoRequestEvent], @info_request.info_request_events, :offset => (@page - 1) * @per_page, :limit => @per_page, :collapse_by_prefix => 'request_collapse') + @matches_estimated = @xapian_object.matches_estimated + @show_no_more_than = (@matches_estimated > MAX_RESULTS) ? MAX_RESULTS : @matches_estimated if (@page > 1) @page_desc = " (page " + @page.to_s + ")" -- cgit v1.2.3