From 8cdbdaaa07f937d8d8b3ac805edba35a3236b6e9 Mon Sep 17 00:00:00 2001 From: Pranav Date: Mon, 4 Nov 2024 01:25:01 -0800 Subject: [PATCH] fix: Process attachments as regular attachments if the text/plain or text/html part is empty (#10379) Some email clients automatically set Content-Disposition to inline for specific content types, such as images. In cases where the email body is empty, inline attachments may not display correctly due to our previous implementation. Our assumption was that these attachments are referenced within text/plain or text/html parts. Customer-reported issues, especially with Apple Mail, show emails with attachments marked as inline but without any corresponding text parts. This leads to missing attachments even though would have processed the attachment. This update introduces a check for the presence of a text part. If none exists, inline attachments are treated as regular attachments and added to the external attachments array, ensuring that all attachments display properly.
Script to update the existing emails that are already available in the system ```rb def update_content id message = Message.find id conversation = message.conversation message_id = message.source_id channel = message.inbox.channel authentication_type = 'XOAUTH2' imap_password = Google::RefreshOauthTokenService.new(channel: channel).access_token imap = Net::IMAP.new(channel.imap_address, port: channel.imap_port, ssl: true) imap.authenticate(authentication_type, channel.imap_login, imap_password) imap.select('INBOX') results = imap.search(['HEADER', 'MESSAGE-ID', message_id]) message_content = imap.fetch(results.first, 'RFC822').first.attr['RFC822'] mail = MailPresenter.new(Mail.read_from_string(message_content)) mail_content = if mail.text_content.present? mail.text_content[:reply] elsif mail.html_content.present? mail.html_content[:reply] end attachments = mail.attachments.last(Message::NUMBER_OF_PERMITTED_ATTACHMENTS) inline_attachments = attachments.select { |attachment| attachment[:original].inline? && mail_content.present? } regular_attachments = attachments - inline_attachments regular_attachments.each do |mail_attachment| attachment = message.attachments.new( account_id: conversation.account_id, file_type: 'file' ) attachment.file.attach(mail_attachment[:blob]) end message.save! end ```
--- app/mailboxes/mailbox_helper.rb | 20 +++--- .../files/attachments_without_text.eml | 65 +++++++++++++++++++ spec/mailboxes/imap/imap_mailbox_spec.rb | 13 ++++ 3 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 spec/fixtures/files/attachments_without_text.eml diff --git a/app/mailboxes/mailbox_helper.rb b/app/mailboxes/mailbox_helper.rb index 0f310e4ec..35f075983 100644 --- a/app/mailboxes/mailbox_helper.rb +++ b/app/mailboxes/mailbox_helper.rb @@ -26,16 +26,22 @@ module MailboxHelper # ensure we don't add more than the permitted number of attachments all_attachments = processed_mail.attachments.last(Message::NUMBER_OF_PERMITTED_ATTACHMENTS) + grouped_attachments = group_attachments(all_attachments) - inline_attachments = all_attachments.select { |attachment| attachment[:original].inline? } - regular_attachments = all_attachments - inline_attachments - - process_inline_attachments(inline_attachments) if inline_attachments.present? - process_regular_attachments(regular_attachments) if regular_attachments.present? + process_inline_attachments(grouped_attachments[:inline]) if grouped_attachments[:inline].present? + process_regular_attachments(grouped_attachments[:regular]) if grouped_attachments[:regular].present? @message.save! end + def group_attachments(attachments) + # If the email lacks a text body, treat inline attachments as standard attachments for processing. + inline_attachments = attachments.select { |attachment| attachment[:original].inline? && mail_content.present? } + + regular_attachments = attachments - inline_attachments + { inline: inline_attachments, regular: regular_attachments } + end + def process_regular_attachments(attachments) Rails.logger.info "[MailboxHelper] Processing regular attachments for message with ID: #{processed_mail.message_id}" attachments.each do |mail_attachment| @@ -102,9 +108,7 @@ module MailboxHelper contact_attributes: { name: identify_contact_name, email: processed_mail.original_sender, - additional_attributes: { - source_id: "email:#{processed_mail.message_id}" - } + additional_attributes: { source_id: "email:#{processed_mail.message_id}" } } ).perform diff --git a/spec/fixtures/files/attachments_without_text.eml b/spec/fixtures/files/attachments_without_text.eml new file mode 100644 index 000000000..35c4d5f80 --- /dev/null +++ b/spec/fixtures/files/attachments_without_text.eml @@ -0,0 +1,65 @@ +From: hello@icloud.com +Content-Type: multipart/mixed; + boundary="Apple-Mail=_F9915DAD-44A0-47DE-BB38-1168" +Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3818.100.11.1.3\)) +Subject: Hello there +Message-Id: <7DDDD-F45F@icloud.com> +Date: Thu, 31 Oct 2024 18:19:46 -0700 +To: Pranav Raj S + + +--Apple-Mail=_F9915DAD-44A0-47DE-BB38-1168 +Content-Disposition: inline; + filename=1kb.png +Content-Type: image/png; + x-unix-mode=0644; + name="1kb.png" +Content-Transfer-Encoding: base64 + +iVBORw0KGgoAAAANSUhEUgAAABEAAAAOCAMAAAD+MweGAAADAFBMVEUAAAAAAFUAAKoAAP8AJAAA +JFUAJKoAJP8ASQAASVUASaoASf8AbQAAbVUAbaoAbf8AkgAAklUAkqoAkv8AtgAAtlUAtqoAtv8A +2wAA21UA26oA2/8A/wAA/1UA/6oA//8kAAAkAFUkAKokAP8kJAAkJFUkJKokJP8kSQAkSVUkSaok +Sf8kbQAkbVUkbaokbf8kkgAkklUkkqokkv8ktgAktlUktqoktv8k2wAk21Uk26ok2/8k/wAk/1Uk +/6ok//9JAABJAFVJAKpJAP9JJABJJFVJJKpJJP9JSQBJSVVJSapJSf9JbQBJbVVJbapJbf9JkgBJ +klVJkqpJkv9JtgBJtlVJtqpJtv9J2wBJ21VJ26pJ2/9J/wBJ/1VJ/6pJ//9tAABtAFVtAKptAP9t +JABtJFVtJKptJP9tSQBtSVVtSaptSf9tbQBtbVVtbaptbf9tkgBtklVtkqptkv9ttgBttlVttqpt +tv9t2wBt21Vt26pt2/9t/wBt/1Vt/6pt//+SAACSAFWSAKqSAP+SJACSJFWSJKqSJP+SSQCSSVWS +SaqSSf+SbQCSbVWSbaqSbf+SkgCSklWSkqqSkv+StgCStlWStqqStv+S2wCS21WS26qS2/+S/wCS +/1WS/6qS//+2AAC2AFW2AKq2AP+2JAC2JFW2JKq2JP+2SQC2SVW2Saq2Sf+2bQC2bVW2baq2bf+2 +kgC2klW2kqq2kv+2tgC2tlW2tqq2tv+22wC221W226q22/+2/wC2/1W2/6q2///bAADbAFXbAKrb +AP/bJADbJFXbJKrbJP/bSQDbSVXbSarbSf/bbQDbbVXbbarbbf/bkgDbklXbkqrbkv/btgDbtlXb +tqrbtv/b2wDb21Xb26rb2//b/wDb/1Xb/6rb////AAD/AFX/AKr/AP//JAD/JFX/JKr/JP//SQD/ +SVX/Sar/Sf//bQD/bVX/bar/bf//kgD/klX/kqr/kv//tgD/tlX/tqr/tv//2wD/21X/26r/2/// +/wD//1X//6r////qm24uAAAA1ElEQVR42h1PMW4CQQwc73mlFJGCQChFIp0Rh0RBGV5AFUXKC/KP +fCFdqryEgoJ8IX0KEF64q0PPnow3jT2WxzNj+gAgAGfvvDdCQIHoSnGYcGDE2nH92DoRqTYJ2bTc +sKgqhIi47VdgAWNmwFSFA1UAAT2sSFcnq8a3x/zkkJrhaHT3N+hD3aH7ZuabGHX7bsSMhxwTJLr3 +evf1e0nBVcwmqcTZuatKoJaB7dSHjTZdM0G1HBTWefly//q2EB7/BEvk5vmzeQaJ7/xKPImpzv8/ +s4grhAxHl0DsqGUAAAAASUVORK5CYII= +--Apple-Mail=_F9915DAD-44A0-47DE-BB38-1168 +Content-Disposition: inline; + filename=1kb.png +Content-Type: image/png; + x-unix-mode=0644; + name="1kb.png" +Content-Transfer-Encoding: base64 + +iVBORw0KGgoAAAANSUhEUgAAABEAAAAOCAMAAAD+MweGAAADAFBMVEUAAAAAAFUAAKoAAP8AJAAA +JFUAJKoAJP8ASQAASVUASaoASf8AbQAAbVUAbaoAbf8AkgAAklUAkqoAkv8AtgAAtlUAtqoAtv8A +2wAA21UA26oA2/8A/wAA/1UA/6oA//8kAAAkAFUkAKokAP8kJAAkJFUkJKokJP8kSQAkSVUkSaok +Sf8kbQAkbVUkbaokbf8kkgAkklUkkqokkv8ktgAktlUktqoktv8k2wAk21Uk26ok2/8k/wAk/1Uk +/6ok//9JAABJAFVJAKpJAP9JJABJJFVJJKpJJP9JSQBJSVVJSapJSf9JbQBJbVVJbapJbf9JkgBJ +klVJkqpJkv9JtgBJtlVJtqpJtv9J2wBJ21VJ26pJ2/9J/wBJ/1VJ/6pJ//9tAABtAFVtAKptAP9t +JABtJFVtJKptJP9tSQBtSVVtSaptSf9tbQBtbVVtbaptbf9tkgBtklVtkqptkv9ttgBttlVttqpt +tv9t2wBt21Vt26pt2/9t/wBt/1Vt/6pt//+SAACSAFWSAKqSAP+SJACSJFWSJKqSJP+SSQCSSVWS +SaqSSf+SbQCSbVWSbaqSbf+SkgCSklWSkqqSkv+StgCStlWStqqStv+S2wCS21WS26qS2/+S/wCS +/1WS/6qS//+2AAC2AFW2AKq2AP+2JAC2JFW2JKq2JP+2SQC2SVW2Saq2Sf+2bQC2bVW2baq2bf+2 +kgC2klW2kqq2kv+2tgC2tlW2tqq2tv+22wC221W226q22/+2/wC2/1W2/6q2///bAADbAFXbAKrb +AP/bJADbJFXbJKrbJP/bSQDbSVXbSarbSf/bbQDbbVXbbarbbf/bkgDbklXbkqrbkv/btgDbtlXb +tqrbtv/b2wDb21Xb26rb2//b/wDb/1Xb/6rb////AAD/AFX/AKr/AP//JAD/JFX/JKr/JP//SQD/ +SVX/Sar/Sf//bQD/bVX/bar/bf//kgD/klX/kqr/kv//tgD/tlX/tqr/tv//2wD/21X/26r/2/// +/wD//1X//6r////qm24uAAAA1ElEQVR42h1PMW4CQQwc73mlFJGCQChFIp0Rh0RBGV5AFUXKC/KP +fCFdqryEgoJ8IX0KEF64q0PPnow3jT2WxzNj+gAgAGfvvDdCQIHoSnGYcGDE2nH92DoRqTYJ2bTc +sKgqhIi47VdgAWNmwFSFA1UAAT2sSFcnq8a3x/zkkJrhaHT3N+hD3aH7ZuabGHX7bsSMhxwTJLr3 +evf1e0nBVcwmqcTZuatKoJaB7dSHjTZdM0G1HBTWefly//q2EB7/BEvk5vmzeQaJ7/xKPImpzv8/ +s4grhAxHl0DsqGUAAAAASUVORK5CYII= +--Apple-Mail=_F9915DAD-44A0-47DE-BB38-1168-- diff --git a/spec/mailboxes/imap/imap_mailbox_spec.rb b/spec/mailboxes/imap/imap_mailbox_spec.rb index d91b8c8c4..bbf3629e8 100644 --- a/spec/mailboxes/imap/imap_mailbox_spec.rb +++ b/spec/mailboxes/imap/imap_mailbox_spec.rb @@ -30,6 +30,19 @@ RSpec.describe Imap::ImapMailbox do end end + context 'when the email with has empty text content' do + let(:inbound_mail) { create_inbound_email_from_fixture('attachments_without_text.eml') } + + it 'creates a converstation and a message properly' do + expect do + class_instance.process(inbound_mail.mail, channel) + end.to change(Conversation, :count).by(1) + + expect(conversation.contact.email).to eq(inbound_mail.mail.from.first) + expect(conversation.messages.last.attachments.count).to be 2 + end + end + context 'when the email has 15 or more attachments' do let(:inbound_mail) { create_inbound_email_from_fixture('multiple_attachments.eml') }