Fixed quoting for all address headers, not just to #955 [Jamis Buck] Added that quoting to UTF-8 only happens if the characters used are in that range #955 [Jamis Buck]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1142 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 91 insertions, 20 deletions

diff --git a/actionmailer/CHANGELOG b/actionmailer/CHANGELOG
index d7c2653df1..205fdc503e 100644
--- a/actionmailer/CHANGELOG
+++ b/actionmailer/CHANGELOG
@@ -1,5 +1,9 @@
 *SVN*
 
+* Fixed quoting for all address headers, not just to #955 [Jamis Buck]
+
+* Added that quoting to UTF-8 only happens if the characters used are in that range #955 [Jamis Buck] 
+
 * Fixed unquoting of emails that doesn't have an explicit charset #1036 [wolfgang@stufenlos.net]
 
 
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb
index 8661f44702..02e9bd2ad5 100644
--- a/actionmailer/lib/action_mailer/base.rb
+++ b/actionmailer/lib/action_mailer/base.rb
@@ -58,8 +58,6 @@ module ActionMailer #:nodoc:
   #
   # * <tt>default_charset</tt> - The default charset used for the body and to encode the subject. Defaults to UTF-8. You can also 
   #    pick a different charset from inside a method with <tt>@encoding</tt>.
-  #
-  # * <tt>encode_subject</tt> - Whether or not to encode the subject with the active charset. Defaults to true.
   class Base
     private_class_method :new #:nodoc:
 
@@ -91,15 +89,11 @@ module ActionMailer #:nodoc:
     @@default_charset = "utf-8"
     cattr_accessor :default_charset
 
-    @@encode_subject = true
-    cattr_accessor :encode_subject
-
-    attr_accessor :recipients, :subject, :body, :from, :sent_on, :headers, :bcc, :cc, :charset, :encode_subject
+    attr_accessor :recipients, :subject, :body, :from, :sent_on, :headers, :bcc, :cc, :charset
 
     def initialize
       @bcc = @cc = @from = @recipients = @sent_on = @subject = @body = nil
       @charset = @@default_charset.dup
-      @encode_subject = @@encode_subject
       @headers = {}
     end
 
@@ -118,17 +112,18 @@ module ActionMailer #:nodoc:
       end
 
       def mail(to, subject, body, from, timestamp = nil, headers = {},
-               encode = @@encode_subject, charset = @@default_charset
+               charset = @@default_charset
       ) #:nodoc:
         deliver(create(to, subject, body, from, timestamp, headers, charset))
       end
 
       def create(to, subject, body, from, timestamp = nil, headers = {},
-                 encode = @@encode_subject, charset = @@default_charset
+                 charset = @@default_charset
       ) #:nodoc:
         m = TMail::Mail.new
-        m.to, m.subject, m.body, m.from = to,
-          ( encode ? quoted_printable(subject,charset) : subject ), body, from
+        m.subject, m.body = quote_any_if_necessary(charset, subject, body)
+        m.to, m.from = quote_any_address_if_necessary(charset, to, from)
+
         m.date = timestamp.respond_to?("to_time") ? timestamp.to_time : (timestamp || Time.now)    
 
         m.set_content_type "text", "plain", { "charset" => charset }
@@ -150,6 +145,40 @@ module ActionMailer #:nodoc:
         "=?#{charset}?Q?#{text}?="
       end
 
+      CHARS_NEEDING_QUOTING = /[\000-\011\013\014\016-\037\177-\377]/
+
+      # Quote the given text if it contains any "illegal" characters
+      def quote_if_necessary(text, charset)
+        (text =~ CHARS_NEEDING_QUOTING) ?
+          quoted_printable(text, charset) :
+          text
+      end
+
+      # Quote any of the given strings if they contain any "illegal" characters
+      def quote_any_if_necessary(charset, *args)
+        args.map { |v| quote_if_necessary(v, charset) }
+      end
+
+      # Quote the given address if it needs to be. The address may be a
+      # regular email address, or it can be a phrase followed by an address in
+      # brackets. The phrase is the only part that will be quoted, and only if
+      # it needs to be. This allows extended characters to be used in the
+      # "to", "from", "cc", and "bcc" headers.
+      def quote_address_if_necessary(address, charset)
+        if address =~ /^([^<>\s]+) (<.*>)$/
+          address = $2
+          phrase = quote_if_necessary($1, charset)
+          "#{phrase} #{address}"
+        else
+          address
+        end
+      end
+
+      # Quote any of the given addresses, if they need to be.
+      def quote_any_address_if_necessary(charset, *args)
+        args.map { |v| quote_address_if_necessary(v, charset) }
+      end
+
       def receive(raw_email)
         logger.info "Received mail:\n #{raw_email}" unless logger.nil?
         new.receive(TMail::Mail.parse(raw_email))
@@ -185,10 +214,10 @@ module ActionMailer #:nodoc:
 
           mail = create(mailer.recipients, mailer.subject, mailer.body,
                         mailer.from, mailer.sent_on, mailer.headers,
-                        mailer.encode_subject, mailer.charset)
+                        mailer.charset)
 
-          mail.bcc = mailer.bcc unless mailer.bcc.nil?
-          mail.cc  = mailer.cc  unless mailer.cc.nil?
+          mail.bcc = quote_address_if_necessary(mailer.bcc, mailer.charset) unless mailer.bcc.nil?
+          mail.cc  = quote_address_if_necessary(mailer.cc, mailer.charset)  unless mailer.cc.nil?
 
           return mail
         end
diff --git a/actionmailer/test/mail_service_test.rb b/actionmailer/test/mail_service_test.rb
index 262ce988a2..31169d576a 100755
--- a/actionmailer/test/mail_service_test.rb
+++ b/actionmailer/test/mail_service_test.rb
@@ -33,7 +33,7 @@ class TestMailer < ActionMailer::Base
 
   def iso_charset(recipient)
     @recipients = recipient
-    @subject    = "testing iso charsets"
+    @subject    = "testing isø charsets"
     @from       = "system@loudthinking.com"
     @sent_on    = Time.local 2004, 12, 12
     @cc         = "nobody@loudthinking.com"
@@ -50,7 +50,17 @@ class TestMailer < ActionMailer::Base
     @cc         = "nobody@loudthinking.com"
     @bcc        = "root@loudthinking.com"
     @body       = "Nothing to see here."
-    @encode_subject = false
+  end
+
+  def extended_headers(recipient)
+    @recipients = recipient
+    @subject    = "testing extended headers"
+    @from       = "Grytøyr <stian1@example.net>"
+    @sent_on    = Time.local 2004, 12, 12
+    @cc         = "Grytøyr <stian2@example.net>"
+    @bcc        = "Grytøyr <stian3@example.net>"
+    @body       = "Nothing to see here."
+    @charset    = "iso-8859-1"
   end
 
 end
@@ -82,7 +92,7 @@ class ActionMailerTest < Test::Unit::TestCase
   def test_signed_up
     expected = new_mail
     expected.to      = @recipient
-    expected.subject = encode "[Signed up] Welcome #{@recipient}"
+    expected.subject = "[Signed up] Welcome #{@recipient}"
     expected.body    = "Hello there, \n\nMr. #{@recipient}"
     expected.from    = "system@loudthinking.com"
     expected.date    = Time.local(2004, 12, 12)
@@ -100,7 +110,7 @@ class ActionMailerTest < Test::Unit::TestCase
   def test_cancelled_account
     expected = new_mail
     expected.to      = @recipient
-    expected.subject = encode "[Cancelled] Goodbye #{@recipient}"
+    expected.subject = "[Cancelled] Goodbye #{@recipient}"
     expected.body    = "Goodbye, Mr. #{@recipient}"
     expected.from    = "system@loudthinking.com"
     expected.date    = Time.local(2004, 12, 12)
@@ -118,7 +128,7 @@ class ActionMailerTest < Test::Unit::TestCase
   def test_cc_bcc
     expected = new_mail
     expected.to      = @recipient
-    expected.subject = encode "testing bcc/cc"
+    expected.subject = "testing bcc/cc"
     expected.body    = "Nothing to see here."
     expected.from    = "system@loudthinking.com"
     expected.cc      = "nobody@loudthinking.com"
@@ -143,7 +153,7 @@ class ActionMailerTest < Test::Unit::TestCase
   def test_iso_charset
     expected = new_mail( "iso-8859-1" )
     expected.to      = @recipient
-    expected.subject = encode "testing iso charsets", "iso-8859-1"
+    expected.subject = encode "testing isø charsets", "iso-8859-1"
     expected.body    = "Nothing to see here."
     expected.from    = "system@loudthinking.com"
     expected.cc      = "nobody@loudthinking.com"
@@ -228,5 +238,33 @@ EOF
     assert_equal "This_is_a_test\n2 + 2 =3D 4\n", mail.quoted_body
   end
 
+  def test_extended_headers
+    @recipient = "Grytøyr <test@localhost>"
+
+    expected = new_mail "iso-8859-1"
+    expected.to      = TestMailer.quote_address_if_necessary @recipient, "iso-8859-1"
+    expected.subject = "testing extended headers"
+    expected.body    = "Nothing to see here."
+    expected.from    = TestMailer.quote_address_if_necessary "Grytøyr <stian1@example.net>", "iso-8859-1"
+    expected.cc      = TestMailer.quote_address_if_necessary "Grytøyr <stian2@example.net>", "iso-8859-1"
+    expected.bcc     = TestMailer.quote_address_if_necessary "Grytøyr <stian3@example.net>", "iso-8859-1"
+    expected.date    = Time.local 2004, 12, 12
+
+    created = nil
+    assert_nothing_raised do
+      created = TestMailer.create_extended_headers @recipient
+    end
+
+    assert_not_nil created
+    assert_equal expected.encoded, created.encoded
+
+    assert_nothing_raised do
+      TestMailer.deliver_extended_headers @recipient
+    end
+
+    assert_not_nil ActionMailer::Base.deliveries.first
+    assert_equal expected.encoded, ActionMailer::Base.deliveries.first.encoded
+  end
+
 end