diff options
-rw-r--r-- | app/controllers/application_controller.rb | 5 | ||||
-rw-r--r-- | app/controllers/request_controller.rb | 31 | ||||
-rw-r--r-- | app/views/admin_public_body/edit.html.erb | 2 | ||||
-rw-r--r-- | lib/mail_handler/backends/mail_backend.rb | 4 | ||||
-rw-r--r-- | lib/mail_handler/backends/mail_extensions.rb | 14 | ||||
-rwxr-xr-x | script/runner | 5 | ||||
-rw-r--r-- | spec/controllers/request_controller_spec.rb | 39 | ||||
-rw-r--r-- | spec/fixtures/files/subject-bad-utf-8-trailing-base64.email | 5 | ||||
-rw-r--r-- | spec/fixtures/files/subject-bad-utf-8-trailing-quoted-printable.email | 5 | ||||
-rw-r--r-- | spec/integration/errors_spec.rb | 17 | ||||
-rw-r--r-- | spec/lib/mail_handler/mail_handler_spec.rb | 21 | ||||
-rw-r--r-- | spec/models/info_request_event_spec.rb | 8 |
12 files changed, 130 insertions, 26 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 2615b61f2..d1d702616 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -142,7 +142,10 @@ class ApplicationController < ActionController::Base ExceptionNotifier::Notifier.exception_notification(request.env, exception).deliver @status = 500 end - render :template => "general/exception_caught", :status => @status + respond_to do |format| + format.html{ render :template => "general/exception_caught", :status => @status } + format.any{ render :nothing => true, :status => @status } + end end def local_request? diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index b8ccdf926..a455d1725 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -67,8 +67,7 @@ class RequestController < ApplicationController # Test for whole request being hidden if !@info_request.user_can_view?(authenticated_user) - render :template => 'request/hidden', :status => 410 # gone - return + return render_hidden end # Other parameters @@ -126,8 +125,7 @@ class RequestController < ApplicationController long_cache @info_request = InfoRequest.find_by_url_title!(params[:url_title]) if !@info_request.user_can_view?(authenticated_user) - render :template => 'request/hidden', :status => 410 # gone - return + return render_hidden end @columns = ['id', 'event_type', 'created_at', 'described_state', 'last_described_at', 'calculated_state' ] end @@ -146,8 +144,7 @@ class RequestController < ApplicationController raise ActiveRecord::RecordNotFound.new("Request not found") if @info_request.nil? if !@info_request.user_can_view?(authenticated_user) - render :template => 'request/hidden', :status => 410 # gone - return + return render_hidden 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') @@ -587,8 +584,7 @@ class RequestController < ApplicationController # Test for hidden requests if !authenticated_user.nil? && !@info_request.user_can_view?(authenticated_user) - render :template => 'request/hidden', :status => 410 # gone - return + return render_hidden end # Check address is good @@ -671,7 +667,7 @@ class RequestController < ApplicationController 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 + return render_hidden end # Is this a completely public request that we can cache attachments for # to be served up without authentication? @@ -711,7 +707,7 @@ class RequestController < ApplicationController logger.info("Reading cache for #{key_path}") if File.directory?(key_path) - render :text => "Directory listing not allowed", :status => 403 + render :text => "Directory listing not allowed", :status => 403 else render :text => foi_fragment_cache_read(key_path), :content_type => (AlaveteliFileTypes.filename_to_mimetype(params[:file_name]) || 'application/octet-stream') @@ -882,8 +878,7 @@ class RequestController < ApplicationController @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 + return render_hidden end if authenticated?( :web => _("To download the zip file"), @@ -943,5 +938,17 @@ class RequestController < ApplicationController end end end + + private + + def render_hidden + respond_to do |format| + response_code = 410 # gone + format.html{ render :template => 'request/hidden', :status => response_code } + format.any{ render :nothing => true, :status => response_code } + end + false + end + end diff --git a/app/views/admin_public_body/edit.html.erb b/app/views/admin_public_body/edit.html.erb index a24122671..11b7eec22 100644 --- a/app/views/admin_public_body/edit.html.erb +++ b/app/views/admin_public_body/edit.html.erb @@ -3,7 +3,7 @@ <div class="row"> <div class="span8"> <div id="public_body_form"> - <% form_for @public_body, :url => admin_body_update_path(@public_body), :html => { :class => "form form-horizontal" } do |f| %> + <%= form_for @public_body, :url => admin_body_update_path(@public_body), :html => { :class => "form form-horizontal" } do |f| %> <%= render :partial => 'form', :locals => {:f => f} %> <div class="form-actions"> <%= f.submit 'Save', :accesskey => 's', :class => "btn btn-success" %></p> diff --git a/lib/mail_handler/backends/mail_backend.rb b/lib/mail_handler/backends/mail_backend.rb index 03d78e0a3..561946980 100644 --- a/lib/mail_handler/backends/mail_backend.rb +++ b/lib/mail_handler/backends/mail_backend.rb @@ -367,7 +367,9 @@ module MailHandler end def address_from_string(string) - Mail::Address.new(string).address + mail = Mail.new + mail.from = string + mail.from[0] end end end diff --git a/lib/mail_handler/backends/mail_extensions.rb b/lib/mail_handler/backends/mail_extensions.rb index d25012e39..54599639b 100644 --- a/lib/mail_handler/backends/mail_extensions.rb +++ b/lib/mail_handler/backends/mail_extensions.rb @@ -73,7 +73,12 @@ module Mail if match encoding = match[1] str = Ruby18.decode_base64(match[2]) - str = Iconv.conv('UTF-8//IGNORE', fix_encoding(encoding), str) + # Adding and removing trailing spaces is a workaround + # for Iconv.conv throwing an exception if it finds an + # invalid character at the end of the string, even + # with UTF-8//IGNORE: + # http://po-ru.com/diary/fixing-invalid-utf-8-in-ruby-revisited/ + str = Iconv.conv('UTF-8//IGNORE', fix_encoding(encoding), str + " ")[0...-4] end str end @@ -86,7 +91,12 @@ module Mail # Remove trailing = if it exists in a Q encoding string = string.sub(/\=$/, '') str = Encodings::QuotedPrintable.decode(string) - str = Iconv.conv('UTF-8//IGNORE', fix_encoding(encoding), str) + # Adding and removing trailing spaces is a workaround + # for Iconv.conv throwing an exception if it finds an + # invalid character at the end of the string, even + # with UTF-8//IGNORE: + # http://po-ru.com/diary/fixing-invalid-utf-8-in-ruby-revisited/ + str = Iconv.conv('UTF-8//IGNORE', fix_encoding(encoding), str + " ")[0...-4] end str end diff --git a/script/runner b/script/runner index 73b03847d..32a0e6b7e 100755 --- a/script/runner +++ b/script/runner @@ -19,12 +19,13 @@ Dir.chdir(alaveteli_dir) do # Load the runner in a subprocess pid = fork do - `bundle exec rails runner #{ARGV[1]}` + exec("bundle exec rails runner #{ARGV[1]}") exit 0 end # If the environment variable PIDFILE is present, # write the pid of the daemon process to that file. + if ENV.has_key? "PIDFILE" File.open(ENV["PIDFILE"], 'w') do |fh| fh.puts pid @@ -34,6 +35,6 @@ Dir.chdir(alaveteli_dir) do Process.detach(pid) else # Not daemon mode - `bundle exec rails runner #{ARGV[1]}` + exec("bundle exec rails runner #{ARGV[1]}") end end diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 9cc60a103..83e2b1767 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -816,6 +816,16 @@ describe RequestController, "when changing prominence of a request" do response.should render_template('hidden') end + it 'should not show hidden requests if requested using json' do + ir = info_requests(:fancy_dog_request) + ir.prominence = 'hidden' + ir.save! + + session[:user_id] = ir.user.id # bob_smith_user + get :show, :url_title => 'why_do_you_have_such_a_fancy_dog', :format => 'json' + response.code.should == '410' + end + it "should show hidden requests if logged in as super user" do ir = info_requests(:fancy_dog_request) ir.prominence = 'hidden' @@ -1236,14 +1246,29 @@ describe RequestController, "when viewing an individual response for reply/follo response.body.should have_selector("div#other_recipients ul li", :content => "Frob") end - it "should not show individual responses if request hidden, even if request owner" do - ir = info_requests(:fancy_dog_request) - ir.prominence = 'hidden' - ir.save! + context 'when a request is hidden' do + + before do + ir = info_requests(:fancy_dog_request) + ir.prominence = 'hidden' + ir.save! + + session[:user_id] = users(:bob_smith_user).id + end + + it "should not show individual responses, even if request owner" do + get :show_response, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message) + response.should render_template('request/hidden') + end + + it 'should respond to a json request for a hidden request with a 410 code and no body' do + get :show_response, :id => info_requests(:fancy_dog_request).id, + :incoming_message_id => incoming_messages(:useless_incoming_message), + :format => 'json' + + response.code.should == '410' + end - session[:user_id] = users(:bob_smith_user).id - get :show_response, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message) - response.should render_template('request/hidden') end describe 'when viewing a response for an external request' do diff --git a/spec/fixtures/files/subject-bad-utf-8-trailing-base64.email b/spec/fixtures/files/subject-bad-utf-8-trailing-base64.email new file mode 100644 index 000000000..dad621877 --- /dev/null +++ b/spec/fixtures/files/subject-bad-utf-8-trailing-base64.email @@ -0,0 +1,5 @@ +From: foo@bar +To: baz@quux +Subject: =?UTF-8?B?aGVsbG/w?= + +Hello, this is the text of the email. diff --git a/spec/fixtures/files/subject-bad-utf-8-trailing-quoted-printable.email b/spec/fixtures/files/subject-bad-utf-8-trailing-quoted-printable.email new file mode 100644 index 000000000..b80deb4e8 --- /dev/null +++ b/spec/fixtures/files/subject-bad-utf-8-trailing-quoted-printable.email @@ -0,0 +1,5 @@ +From: foo@bar +To: baz@quux +Subject: =?UTF-8?Q?hello=F0=?= + +Hello, this is the text of the email. diff --git a/spec/integration/errors_spec.rb b/spec/integration/errors_spec.rb index ed0d7bfec..17a0153c2 100644 --- a/spec/integration/errors_spec.rb +++ b/spec/integration/errors_spec.rb @@ -62,6 +62,17 @@ describe "When errors occur" do response.code.should == "500" end + it 'should render a 500 for json errors' do + InfoRequest.stub!(:find_by_url_title!).and_raise("An example error") + get("/request/example.json") + response.code.should == '500' + end + + it 'should render a 404 for a non-found xml request' do + get("/frobsnasm.xml") + response.code.should == '404' + end + it 'should notify of a general error' do InfoRequest.stub!(:find_by_url_title!).and_raise("An example error") get("/request/example") @@ -97,6 +108,12 @@ describe "When errors occur" do response.code.should == "403" end + it "return a 403 for a JSON PermissionDenied error" do + InfoRequest.stub!(:find_by_url_title!).and_raise(ApplicationController::PermissionDenied) + get("/request/example.json") + response.code.should == '403' + end + context "in the admin interface" do it 'should show a full trace for general errors' do diff --git a/spec/lib/mail_handler/mail_handler_spec.rb b/spec/lib/mail_handler/mail_handler_spec.rb index 01bf179f8..272b56d0b 100644 --- a/spec/lib/mail_handler/mail_handler_spec.rb +++ b/spec/lib/mail_handler/mail_handler_spec.rb @@ -32,6 +32,19 @@ describe 'when creating a mail object from raw data' do MailHandler.get_part_body(mail).is_utf8?.should == true end + it 'should not be confused by subject lines with malformed UTF-8 at the end' do + # The base64 subject line was generated with: + # printf "hello\360" | base64 + # ... and wrapping the result in '=?UTF-8?B?' and '?=' + mail = get_fixture_mail('subject-bad-utf-8-trailing-base64.email') + mail.subject.should == 'hello' + # The quoted printable subject line was generated with: + # printf "hello\360" | qprint -b -e + # ... and wrapping the result in '=?UTF-8?Q?' and '?=' + mail = get_fixture_mail('subject-bad-utf-8-trailing-quoted-printable.email') + mail.subject.should == 'hello' + end + it 'should convert a Windows-1252 body mislabelled as ISO-8859-1 to UTF-8' do mail = get_fixture_mail('mislabelled-as-iso-8859-1.email') body = MailHandler.get_part_body(mail) @@ -465,3 +478,11 @@ describe 'when getting attachment attributes' do end end end + +describe 'when getting the address part from an address string' do + + it 'should handle non-ascii characters in the name input' do + address = "\"Someone’s name\" <test@example.com>" + MailHandler.address_from_string(address).should == 'test@example.com' + end +end diff --git a/spec/models/info_request_event_spec.rb b/spec/models/info_request_event_spec.rb index 842246fd8..eb0de8c86 100644 --- a/spec/models/info_request_event_spec.rb +++ b/spec/models/info_request_event_spec.rb @@ -1,3 +1,4 @@ +# coding: utf-8 require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe InfoRequestEvent do @@ -118,6 +119,13 @@ describe InfoRequestEvent do @info_request_event.same_email_as_previous_send?.should be_true end + it 'should handle non-ascii characters in the name input' do + address = "\"Someone’s name\" <test@example.com>" + @info_request_event.stub!(:params).and_return(:email => address) + @info_request_event.stub_chain(:info_request, :get_previous_email_sent_to).and_return(address) + @info_request_event.same_email_as_previous_send?.should be_true + end + end end |