Merge branch 'release/0.11' into rails-3-develop

17 files changed, 167 insertions, 34 deletions

diff --git a/Rakefile b/Rakefile
index d9a53fc69..5fa2a360d 100644
--- a/Rakefile
+++ b/Rakefile
@@ -4,4 +4,6 @@
 require File.expand_path('../config/application', __FILE__)
 require 'rake'
 Alaveteli::Application.load_tasks
-Dir[File.join(File.dirname(__FILE__),'commonlib','rblib','tests','*.rake')].each { |file| load(file) }
+if Rails.env == 'test'
+    Dir[File.join(File.dirname(__FILE__),'commonlib','rblib','tests','*.rake')].each { |file| load(file) }
+end
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 3a1ec95cc..dffc16b63 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 6ca4e9f82..880a7b4b4 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?
@@ -692,7 +688,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')
@@ -863,8 +859,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"),
@@ -924,5 +919,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/config/httpd.conf-example b/config/httpd.conf-example
index 06170a5e2..1326252f5 100644
--- a/config/httpd.conf-example
+++ b/config/httpd.conf-example
@@ -1,4 +1,4 @@
-# Apache configuracreated_attion for FOI site.
+# Apache configuration for FOI site.
 #
 # For development ignore this, you can just run ./scripts/server as for any
 # Ruby on Rails application.
diff --git a/config/initializers/alaveteli.rb b/config/initializers/alaveteli.rb
index 263e5cf7b..22ea238b7 100644
--- a/config/initializers/alaveteli.rb
+++ b/config/initializers/alaveteli.rb
@@ -10,7 +10,7 @@ load "debug_helpers.rb"
 load "util.rb"
 
 # Application version
-ALAVETELI_VERSION = '0.10'
+ALAVETELI_VERSION = '0.11'
 
 # Add new inflection rules using the following format
 # (all these examples are active by default):
diff --git a/doc/CHANGES.md b/doc/CHANGES.md
index 9a027a3a8..e53dcfe0b 100644
--- a/doc/CHANGES.md
+++ b/doc/CHANGES.md
@@ -1,3 +1,13 @@
+# Version 0.11
+## Highlighted features
+* Upgrade of the Rails framework to version 3.1.12 (Henare Degan, Matthew Landauer, Mark Longair, Louise Crow)
+
+## Upgrade notes
+* Manually remove vendor/rails-locales
+* Themes created for 0.9 and below should be updated to work with Rails 3. See `THEMES-UPGRADE.md` for notes on upgrading your theme. You will need to manually remove your old theme directory before running `rails-post-deploy`.
+* The `config/httpd.conf` has moved to `config/httpd.conf`, as it may need customization before deploying. It also has a new line setting RackEnv to production - copy this to your config/httpd.conf file.
+* Alaveteli now uses the [mail gem](https://github.com/mikel/mail) rather than [tmail](https://github.com/mikel/tmail) to handle mail. If you're using Exim as your MTA, you'll need to use the setting `extract_addresses_remove_arguments = false` in your Exim conf (see INSTALL-exim4.md for details). This means it won't remove addresses specified with -t on command line from the mail recipient list.
+
 # Version 0.9
 ## Highlighted features
 * Consistent and more informative variable interpolation syntax in translated phrases. All of these phrases will now appear in the form "There are {{count}} people following this request", where some were previously in the form "There are %s people following this request". (Matthew Landauer)
diff --git a/doc/INSTALL-exim4.md b/doc/INSTALL-exim4.md
index e37da14ff..cdc33ab12 100644
--- a/doc/INSTALL-exim4.md
+++ b/doc/INSTALL-exim4.md
@@ -6,15 +6,16 @@ In `/etc/exim4/conf.d/main/04_alaveteli_options`:
     ALAVETELI_HOME=/path/to/alaveteli/software
     ALAVETELI_USER=www-data
     log_file_path=/var/log/exim4/exim-%slog-%D
-    MAIN_LOG_SELECTOR==+all -retry_defer 
+    MAIN_LOG_SELECTOR==+all -retry_defer
+    extract_addresses_remove_arguments=false
 
 (The user ALAVETELI_USER should have write permissions on ALAVETELI_HOME).
 
-Note that the name and location of the log files created by Exim must match 
+Note that the name and location of the log files created by Exim must match
 what the `load-mail-server-logs` script expects, hence the need for the extra
 `log_file_path` setting. And the `check-recent-requests-sent` scripts expects
-the logs to contain the `from=<...>` envelope information, so we make the 
-logs more verbose with `log_selector`. 
+the logs to contain the `from=<...>` envelope information, so we make the
+logs more verbose with `log_selector`.
 
 In `/etc/exim4/conf.d/router/04_alaveteli`:
 
@@ -33,7 +34,7 @@ In `/etc/exim4/conf.d/transport/04_alaveteli`:
        home_directory = ALAVETELI_HOME
        user = ALAVETELI_USER
        group = ALAVETELI_USER
-    
+
 And, assuming you set `INCOMING_EMAIL_PREFIX` in your config at
 `config/general` to "foi+", create `config/aliases` with the following
 content:
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..322c49bb5 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
@@ -102,4 +112,20 @@ module Mail
             end
         end
     end
+    class Ruby19
+
+        def Ruby19.q_value_decode(str)
+            match = str.match(/\=\?(.+)?\?[Qq]\?(.+)?\?\=/m)
+            if match
+                encoding = match[1]
+                str = Encodings::QuotedPrintable.decode(match[2].gsub(/_/, '=20'))
+                # Backport line from mail 2.5 to strip a trailing = character
+                # Remove trailing = if it exists in a Q encoding
+                str = str.sub(/\=$/, '')
+                str.force_encoding(fix_encoding(encoding))
+            end
+            decoded = str.encode("utf-8", :invalid => :replace, :replace => "")
+            decoded.valid_encoding? ? decoded : decoded.encode("utf-16le", :invalid => :replace, :replace => "").encode("utf-8")
+        end
+    end
 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 5d10c88ad..dedffde4f 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -848,6 +848,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'
@@ -1268,14 +1278,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