Fix encoding

After extensive testing, there should be no need to mess with the transfer encoding or the body encoding. All we need to ensure is that the proper charset is configured in the `Content-Type` header when assigning the body, which needs to be the encoding of the string being set as the body. Fixes #240
fphilipe · Mar 17, 2020 · b196ab0 · b196ab0
1 parent 97ba182
commit b196ab0
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 16 deletions.
diff --git a/lib/premailer/rails/hook.rb b/lib/premailer/rails/hook.rb
@@ -73,10 +73,8 @@ def generate_html_part
         part = html_part
         html = premailer.to_inline_css
         Mail::Part.new do
-          content_transfer_encoding part.content_transfer_encoding
-          content_type "text/html; charset=#{part.charset}"
+          content_type "text/html; charset=#{html.encoding}"
           body html
-          body_encoding part.body.encoding
         end
       end
 
@@ -85,10 +83,8 @@ def generate_text_part
           part = html_part
           text = premailer.to_plain_text
           Mail::Part.new do
-            content_transfer_encoding part.content_transfer_encoding
-            content_type "text/plain; charset=#{part.charset}"
+            content_type "text/plain; charset=#{text.encoding}"
             body text
-            body_encoding part.body.encoding
           end
         end
       end

diff --git a/spec/integration/hook_spec.rb b/spec/integration/hook_spec.rb
@@ -6,7 +6,7 @@ def run_hook(message)
   end
 
   def body_content(message)
-    Nokogiri::HTML(message.html_string).at('body').content
+    Nokogiri::HTML(message.html_string).at('body').content.gsub("\r\n", "\n")
   end
 
   class Mail::Message
@@ -47,18 +47,31 @@ def html_string
     expect(processed_message.parts).to match_array(expected_parts)
   end
 
-  describe 'when the content-transfer-encoding is set' do
-    before { message.content_transfer_encoding = 'quoted-printable' }
+  it 'does not screw up the text by maintaining the original body encoding' do
+    raw_msg = Fixtures::Message.latin_message
+    processed_msg = Fixtures::Message.latin_message
+    run_hook(processed_msg)
+    expect(body_content(processed_msg)).to eq(body_content(raw_msg))
 
-    it 'should maintain the value' do
-      expect(processed_message.parts.first.content_transfer_encoding).to \
-        eq 'quoted-printable'
-      expect(processed_message.parts.last.content_transfer_encoding).to \
-        eq 'quoted-printable'
-    end
+    raw_msg = Fixtures::Message.non_latin_message
+    processed_msg = Fixtures::Message.non_latin_message
+    run_hook(processed_msg)
+    expect(body_content(processed_msg)).to eq(body_content(raw_msg))
+
+    raw_msg = Fixtures::Message.greek_message
+    processed_msg = Fixtures::Message.greek_message
+    run_hook(processed_msg)
+    expect(body_content(processed_msg)).to eq(body_content(raw_msg))
+
+    raw_msg = Fixtures::Message.dash_message
+    processed_msg = Fixtures::Message.dash_message
+    run_hook(processed_msg)
+    expect(body_content(processed_msg)).to eq(body_content(raw_msg))
   end
 
-  it 'does not screw up the text by maintaining the original body encoding' do
+  it 'supports US-ASCII output' do
+    Premailer::Rails.config.merge!(output_encoding: 'US-ASCII')
+
     raw_msg = Fixtures::Message.latin_message
     processed_msg = Fixtures::Message.latin_message
     run_hook(processed_msg)
@@ -68,6 +81,18 @@ def html_string
     processed_msg = Fixtures::Message.non_latin_message
     run_hook(processed_msg)
     expect(body_content(processed_msg)).to eq(body_content(raw_msg))
+
+    raw_msg = Fixtures::Message.greek_message
+    processed_msg = Fixtures::Message.greek_message
+    run_hook(processed_msg)
+    expect(body_content(processed_msg)).to eq(body_content(raw_msg))
+
+    raw_msg = Fixtures::Message.dash_message
+    processed_msg = Fixtures::Message.dash_message
+    run_hook(processed_msg)
+    expect(body_content(processed_msg)).to eq(body_content(raw_msg))
+  ensure
+    Premailer::Rails.config.delete(:output_encoding)
   end
 
   it 'generates a text part from the html' do

diff --git a/spec/support/fixtures/message.rb b/spec/support/fixtures/message.rb
@@ -35,6 +35,30 @@ module Message
 </html>
     HTML
 
+    HTML_PART_IN_GREEK = <<-HTML.encode(Encoding::ISO_8859_7)
+<html>
+  <head>
+  </head>
+  <body>
+    <p>
+      Αα Ββ Γγ Δδ Εε Ζζ Ηη Θθ Ιι Κκ Λλ Μμ Νν Ξξ Οο Ππ Ρρ Σσ Ττ Υυ Φφ Χχ Ψψ Ωω
+    </p>
+  </body>
+</html>
+    HTML
+
+    HTML_PART_WITH_DASHES = <<-HTML
+<html>
+  <head>
+  </head>
+  <body>
+    <p>
+      Hello there—yes you! What's up with – pardon the interrupion – dashes? I can also do &ndash; and &mdash;.
+    </p>
+  </body>
+</html>
+    HTML
+
     HTML_PART_WITH_CSS = <<-HTML
 <html>
   <head>
@@ -130,6 +154,22 @@ def non_latin_message
       end
     end
 
+    def greek_message
+      base_message.tap do |message|
+        message.body = HTML_PART_IN_GREEK
+        message.content_type 'text/html; charset=ISO-8859-7'
+        message.ready_to_send!
+      end
+    end
+
+    def dash_message
+      base_message.tap do |message|
+        message.body = HTML_PART_WITH_DASHES
+        message.content_type 'text/html; charset=UTF-8'
+        message.ready_to_send!
+      end
+    end
+
     private
 
     def base_message