diff options
-rw-r--r-- | app/models/info_request.rb | 6 | ||||
-rw-r--r-- | app/models/request_mailer.rb | 6 | ||||
-rw-r--r-- | app/models/user.rb | 4 | ||||
-rw-r--r-- | lib/tmail_extensions.rb | 35 | ||||
-rw-r--r-- | spec/models/info_request_spec.rb | 10 | ||||
-rw-r--r-- | spec/models/request_mailer_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 20 |
7 files changed, 54 insertions, 31 deletions
diff --git a/app/models/info_request.rb b/app/models/info_request.rb index c12d364b6..0dec0528a 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -23,7 +23,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: info_request.rb,v 1.182 2009-04-06 16:28:44 louise Exp $ +# $Id: info_request.rb,v 1.183 2009-04-08 07:31:07 francis Exp $ require 'digest/sha1' require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian') @@ -148,7 +148,7 @@ public return self.magic_email("request-") end def incoming_name_and_email - return TMail::Address.encode_quoted_string(self.user.name) + " <" + self.incoming_email + ">" + return TMail::Address.address_from_name_and_email(self.user.name, self.incoming_email).to_s end # Subject lines for emails about the request @@ -462,7 +462,7 @@ public return self.public_body.request_email end def recipient_name_and_email - return TMail::Address.encode_quoted_string(self.law_used_short + " requests at " + self.public_body.short_or_long_name) + " <" + self.recipient_email + ">" + return TMail::Address.address_from_name_and_email(self.law_used_short + " requests at " + self.public_body.short_or_long_name, self.recipient_email).to_s end # History of some things that have happened diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb index 2e69be363..b246abf76 100644 --- a/app/models/request_mailer.rb +++ b/app/models/request_mailer.rb @@ -4,7 +4,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: request_mailer.rb,v 1.71 2009-04-08 05:29:35 francis Exp $ +# $Id: request_mailer.rb,v 1.72 2009-04-08 07:31:07 francis Exp $ class RequestMailer < ApplicationMailer @@ -37,7 +37,7 @@ class RequestMailer < ApplicationMailer if incoming_message_followup.nil? || !incoming_message_followup.valid_to_reply_to? return info_request.recipient_name_and_email else - return incoming_message_followup.mail.from_addrs[0].full_quoted_address + return incoming_message_followup.mail.from_addrs[0].to_s end end # Used in the preview of followup @@ -92,7 +92,7 @@ class RequestMailer < ApplicationMailer def stopped_responses(info_request, email) @from = contact_from_name_and_email headers 'Return-Path' => blackhole_email, 'Reply-To' => @from # we don't care about bounces, likely from spammers - @recipients = email.from_addrs[0].full_quoted_address + @recipients = email.from_addrs[0].to_s @subject = "Your response to an FOI request was not delivered" attachment :content_type => 'message/rfc822', :body => email.body @body = { diff --git a/app/models/user.rb b/app/models/user.rb index c281d7f18..4638583e2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -23,7 +23,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: user.rb,v 1.87 2009-04-07 10:32:54 louise Exp $ +# $Id: user.rb,v 1.88 2009-04-08 07:31:07 francis Exp $ require 'digest/sha1' @@ -174,7 +174,7 @@ class User < ActiveRecord::Base # For use in to/from in email messages def name_and_email - return TMail::Address.encode_quoted_string(self.name) + " <" + self.email + ">" + return TMail::Address.address_from_name_and_email(self.name, self.email).to_s end # The "internal admin" is a special user for internal use. diff --git a/lib/tmail_extensions.rb b/lib/tmail_extensions.rb index a02460f7f..711463050 100644 --- a/lib/tmail_extensions.rb +++ b/lib/tmail_extensions.rb @@ -4,7 +4,7 @@ # Copyright (c) 2009 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: tmail_extensions.rb,v 1.2 2009-04-08 05:29:36 francis Exp $ +# $Id: tmail_extensions.rb,v 1.3 2009-04-08 07:31:08 francis Exp $ # Monkeypatch! @@ -32,27 +32,22 @@ module TMail end class Address - # Monkeypatch! - def Address.encode_quoted_string(text) - # XXX have added space to this, so we don't excessive quoting - if text.match(/[^A-Za-z0-9!#\$%&'*+\-\/=?^_`{|}~ ]/) - # Contains characters which aren't valid in atoms, so make a - # quoted-pair instead. - text = text.gsub(/(["\\])/, "\\\\\\1") - text = '"' + text + '"' - end - return text + # Monkeypatch! Constructor which makes a TMail::Address given + # a name and an email + def Address.address_from_name_and_email(name, email) + # Botch an always quoted RFC address, then parse it + name = name.gsub(/(["\\])/, "\\\\\\1") + TMail::Address.parse('"' + name + '" <' + email + '>') end + end - # Monkeypatch! - def full_quoted_address - if self.name - # sanitise name - some mail servers can't cope with @ in the name part - name = self.name.gsub(/@/, " ") - Address.encode_quoted_string(name) + " <" + self.spec + ">" - else - self.spec - end + module TextUtils + # Monkeypatch! Much more aggressive list of characters to cause quoting + # than in normal TMail. e.g. Have found real cases where @ needs quoting. + # We list characters to allow, rather than characters not to allow. + NEW_PHRASE_UNSAFE=/[^A-Za-z0-9!#\$%&'*+\-\/=?^_`{|}~ ]/n + def quote_phrase( str ) + (NEW_PHRASE_UNSAFE === str) ? dquote(str) : str end end end diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index 7e87d0900..3147965c4 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -34,6 +34,14 @@ describe InfoRequest do @info_request.incoming_email.should_not be_nil end + it "should have a sensible incoming name and email" do + @info_request.incoming_name_and_email.should == "Bob Smith <" + @info_request.incoming_email + ">" + end + + it "should have a sensible recipient name and email" do + @info_request.recipient_name_and_email.should == "FOI requests at TGQ <geraldine-requests@localhost>" + end + it "should recognise its own incoming email" do incoming_email = @info_request.incoming_email found_info_request = InfoRequest.find_by_incoming_email(incoming_email) @@ -176,4 +184,4 @@ describe InfoRequest do end -end
\ No newline at end of file +end diff --git a/spec/models/request_mailer_spec.rb b/spec/models/request_mailer_spec.rb index 76eebd05f..7fa28cde4 100644 --- a/spec/models/request_mailer_spec.rb +++ b/spec/models/request_mailer_spec.rb @@ -119,14 +119,14 @@ describe RequestMailer, " when working out follow up addresses" do RequestMailer.email_for_followup(ir, im).should == "foiperson@localhost" end - it "should remove @ signs from name part in reply address as some mail servers hate it" do + it "should quote @ signs" do ir = info_requests(:fancy_dog_request) im = ir.incoming_messages[0] im.raw_email.data.sub!("FOI Person", "FOI @ Person") # check the basic entry in the fixture is fine - RequestMailer.name_and_email_for_followup(ir, im).should == "FOI Person <foiperson@localhost>" + RequestMailer.name_and_email_for_followup(ir, im).should == "\"FOI @ Person\" <foiperson@localhost>" RequestMailer.name_for_followup(ir, im).should == "FOI @ Person" RequestMailer.email_for_followup(ir, im).should == "foiperson@localhost" end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index dc14feff9..46aa046fb 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -124,3 +124,23 @@ describe User, 'when asked if a user owns every request' do end end + +describe User, " when making name and email address" do + it "should generate a name and email" do + @user = User.new + @user.name = "Sensible User" + @user.email = "sensible@localhost" + + @user.name_and_email.should == "Sensible User <sensible@localhost>" + end + + it "should quote name and email with funny characters in the name" do + @user = User.new + @user.name = "Silly @ User" + @user.email = "silly@localhost" + + @user.name_and_email.should == "\"Silly @ User\" <silly@localhost>" + end +end + + |