Browse Source

Fixes issue #2894 - Emails with binary body (attachment as body) get handled as if they were text messages which breaks the attachment.

Mantas Masalskis 5 years ago
parent
commit
6692869dd8
3 changed files with 63 additions and 23 deletions
  1. 33 23
      app/models/channel/email_parser.rb
  2. 19 0
      test/data/mail/mail086.box
  3. 11 0
      test/data/mail/mail086.yml

+ 33 - 23
app/models/channel/email_parser.rb

@@ -574,38 +574,48 @@ process unprocessable_mails (tmp/unprocessable_mail/*.eml) again
   def collect_attachments(mail)
     attachments = []
 
-    # Add non-plaintext body as an attachment
-    if mail.html_part&.body.present? ||
-       (!mail.multipart? && mail.mime_type.present? && mail.mime_type != 'text/plain')
-      message = mail.html_part || mail
+    attachments.push(*get_nonplaintext_body_as_attachment(mail))
 
-      filename = message.filename.presence ||
-                 (message.mime_type.eql?('text/html') ? 'message.html' : '-no name-')
+    mail.parts.each do |part|
+      attachments.push(*gracefully_get_attachments(part, attachments, mail))
+    end
 
-      headers_store = {
-        'content-alternative' => true,
-        'original-format'     => message.mime_type.eql?('text/html'),
-        'Mime-Type'           => message.mime_type,
-        'Charset'             => message.charset,
-      }.reject { |_, v| v.blank? }
+    attachments
+  end
 
-      attachments.push({ data:        body_text(message),
-                         filename:    filename,
-                         preferences: headers_store })
+  def get_nonplaintext_body_as_attachment(mail)
+    if !(mail.html_part&.body.present? || (!mail.multipart? && mail.mime_type.present? && mail.mime_type != 'text/plain'))
+      return
     end
 
-    mail.parts.each do |part|
+    message = mail.html_part || mail
 
-      new_attachments = get_attachments(part, attachments, mail).flatten.compact
-      attachments.push(*new_attachments)
-    rescue => e # Protect process to work with spam emails (see test/fixtures/mail15.box)
-      raise e if (fail_count ||= 0).positive?
+    if !mail.mime_type.starts_with?('text/') && mail.html_part.blank?
+      return gracefully_get_attachments(message, [], mail)
+    end
 
-      (fail_count += 1) && retry
+    filename = message.filename.presence || (message.mime_type.eql?('text/html') ? 'message.html' : '-no name-')
 
-    end
+    headers_store = {
+      'content-alternative' => true,
+      'original-format'     => message.mime_type.eql?('text/html'),
+      'Mime-Type'           => message.mime_type,
+      'Charset'             => message.charset,
+    }.reject { |_, v| v.blank? }
 
-    attachments
+    [{
+      data:        body_text(message),
+      filename:    filename,
+      preferences: headers_store
+    }]
+  end
+
+  def gracefully_get_attachments(part, attachments, mail)
+    get_attachments(part, attachments, mail).flatten.compact
+  rescue => e # Protect process to work with spam emails (see test/fixtures/mail15.box)
+    raise e if (fail_count ||= 0).positive?
+
+    (fail_count += 1) && retry
   end
 
   def get_attachments(file, attachments, mail)

File diff suppressed because it is too large
+ 19 - 0
test/data/mail/mail086.box


File diff suppressed because it is too large
+ 11 - 0
test/data/mail/mail086.yml


Some files were not shown because too many files changed in this diff