Escaping of & in authority's name

[kingqueen3065]

This authority sent a response "from" "FOI E&R". https://www.whatdotheyknow.com/request/s167_list_of_accessible_taxis_up_298#incoming-1064916

Alaveteli escaped the auto-filled name on my reply "Dear FOI E&R," https://www.whatdotheyknow.com/request/s167_list_of_accessible_taxis_up_298#outgoing-716397

I could have corrected it but it is unimportant, I guess.

[gavinrozzi]

We have been having similar issues with both initial outgoing requests and followups for any authority that has an apostrophe in their name, such as in this request the Ocean County Prosecutor's Office. Alaveteli has turned it into "Ocean County Prosecutor's Office."

https://opramachine.com/request/facebook_policies_and_procedures_5#outgoing-1061

[kingqueen3065]

It also apparently resulted in the email address in my reply being altered with the escaping and so resulting in a bounce. https://www.whatdotheyknow.com/request/s167_list_of_accessible_taxis_up_298#incoming-1081819

[lizconlan]

possibly related to #3465

[garethrees]

Looks like there are two parts to this

1. In the public view of the outgoing message, we should show & instead of E&R,
2. We get a bounce message. This part is a duplicate of Escaped characters in email address #3465 (comment)

OutgoingMessage.find(716397).to
# => "FOI E&R <enrg-e&[email protected]>"

[garethrees]

I've had a look in to this. I don't think it will be super complicated to fix, but need to give some thought in to how best to do it.

Here's one example of a "fix", but it doesn't cover all cases:

commit 5780ab790b1631c639b363f70bc61a4be7abfc2e (4419-salutation-escaping)
Author: Gareth Rees <[email protected]>
Date:   Fri Dec 8 12:34:50 2017 +0000

    Mark PublicBody#name as HTML safe in salutation

    We can be pretty confident about this being safe – it's admin approved –
    so don't escape ampersands in the authority name, otherwise outgoing
    messages read "FOI E&amp;R,".

diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb
index 6e7edb230..f88f089c3 100644
--- a/app/models/outgoing_message.rb
+++ b/app/models/outgoing_message.rb
@@ -76,7 +76,8 @@ class OutgoingMessage < ActiveRecord::Base
   end

   def self.default_salutation(public_body)
-    _("Dear {{public_body_name}},", :public_body_name => public_body.name)
+    _('Dear {{public_body_name}},',
+      public_body_name: public_body.name.html_safe)
   end

   def self.fill_in_salutation(text, public_body)
diff --git a/spec/models/outgoing_message_spec.rb b/spec/models/outgoing_message_spec.rb
index d1d9eec1c..aeb8a4220 100644
--- a/spec/models/outgoing_message_spec.rb
+++ b/spec/models/outgoing_message_spec.rb
@@ -23,13 +23,20 @@ describe OutgoingMessage do

   describe '.fill_in_salutation' do

+    let(:placeholder) { 'Dear [Authority name],' }
+
     it 'replaces the batch request salutation with the body name' do
-      text = 'Dear [Authority name],'
       public_body = FactoryGirl.build(:public_body, name: 'A Body')
-      expect(described_class.fill_in_salutation(text, public_body)).
+      expect(described_class.fill_in_salutation(placeholder, public_body)).
         to eq('Dear A Body,')
     end

+    it 'does not escape ampersands' do
+      public_body = FactoryGirl.build(:public_body, name: 'H & S Body')
+      expect(described_class.fill_in_salutation(placeholder, public_body)).
+        to eq('Dear H & S Body,')
+    end
+
   end

   describe '#initialize' do

OutgoingMessage#default_message_replacements might be a better place to start looking, as that handles the case of using either the default body name, or the name from the most recent incoming message.

The names from incoming messages will need to be sanitised in some way, as they're effectively user input (and we do get spam incoming messages), so need to put some thought in to where we do this in the codebase, and exactly how we perform sanitisation.

[garethrees]

Related to #4793

[HelenWDTK]

This issue has been automatically closed due to a lack of discussion or resolution for over 12 months.
Should we decide to revisit this issue in the future, it can be reopened.