aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--app/controllers/application_controller.rb5
-rw-r--r--app/controllers/request_controller.rb31
-rw-r--r--app/views/admin_public_body/edit.html.erb2
-rw-r--r--lib/mail_handler/backends/mail_backend.rb4
-rw-r--r--lib/mail_handler/backends/mail_extensions.rb14
-rwxr-xr-xscript/runner5
-rw-r--r--spec/controllers/request_controller_spec.rb39
-rw-r--r--spec/fixtures/files/subject-bad-utf-8-trailing-base64.email5
-rw-r--r--spec/fixtures/files/subject-bad-utf-8-trailing-quoted-printable.email5
-rw-r--r--spec/integration/errors_spec.rb17
-rw-r--r--spec/lib/mail_handler/mail_handler_spec.rb21
-rw-r--r--spec/models/info_request_event_spec.rb8
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