Avoid adding nil values to mail headers #1392

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

3 files changed, 26 insertions, 5 deletions

diff --git a/actionmailer/CHANGELOG b/actionmailer/CHANGELOG
index c21a89e87c..010799ed25 100644
--- a/actionmailer/CHANGELOG
+++ b/actionmailer/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Avoid adding nil header values #1392
+
 * Better multipart support with implicit multipart/alternative and sorting of subparts [John Long]
 
 * Allow for nested parts in multipart mails #1570 [Flurin Egger]
diff --git a/actionmailer/lib/action_mailer/part.rb b/actionmailer/lib/action_mailer/part.rb
index b47961a744..7b9260f225 100644
--- a/actionmailer/lib/action_mailer/part.rb
+++ b/actionmailer/lib/action_mailer/part.rb
@@ -40,9 +40,9 @@ module ActionMailer
         # non-attachment parts)
         if content_disposition == "attachment"
           part.set_content_type(content_type || defaults.content_type, nil,
-            "charset" => nil,
-            "name" => filename)
-          part.set_content_disposition(content_disposition, "filename" => filename) 
+            squish("charset" => nil, "name" => filename))
+          part.set_content_disposition(content_disposition,
+            squish("filename" => filename))
         else
           part.set_content_type(content_type || defaults.content_type, nil,
             "charset" => (charset || defaults.charset))      
@@ -65,9 +65,14 @@ module ActionMailer
         part.set_content_type(content_type, nil, { "charset" => charset }) if content_type =~ /multipart/
       end
     
-        
       part
     end
+
+    private
+
+      def squish(values={})
+        values.delete_if { |k,v| v.nil? }
+      end
   end
 
 end
diff --git a/actionmailer/test/mail_service_test.rb b/actionmailer/test/mail_service_test.rb
index 85fe3ad69b..f4de8c1894 100755
--- a/actionmailer/test/mail_service_test.rb
+++ b/actionmailer/test/mail_service_test.rb
@@ -156,7 +156,15 @@ class TestMailer < ActionMailer::Base
       p.part :content_type => "text/html", :body => "<b>test</b> HTML<br/>\nline #2"
     end
     attachment :content_type => "application/octet-stream",:filename => "test.txt", :body => "test abcdefghijklmnopqstuvwxyz"
-    
+  end
+
+  def unnamed_attachment(recipient)
+    recipients   recipient
+    subject      "nested multipart"
+    from         "test@example.com"
+    body         "multipart/mixed"
+    part :content_type => "text/plain", :body => "hullo"
+    attachment :content_type => "application/octet-stream", :body => "test abcdefghijklmnopqstuvwxyz"
   end
   
 
@@ -621,5 +629,11 @@ EOF
     mail = TMail::Mail.parse(fixture)
     assert_not_nil mail.from
   end
+
+  def test_empty_header_values_omitted
+    result = TestMailer.create_unnamed_attachment(@recipient).encoded
+    assert_match %r{Content-Type: application/octet-stream[^;]}, result
+    assert_match %r{Content-Disposition: attachment[^;]}, result
+  end
 end