From 933ae8aa497cdb80538a69dad9c6d389e1997c90 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Fri, 15 Nov 2024 09:07:24 +0400 Subject: [PATCH] fix: Email attachments created with empty filename (#10420) - We observed in prod for certain emails active storage blob objects were getting created with empty file name. The conversations further causes conversation and filter pages to break. This change will fix the mentioned issue. fixes: https://linear.app/chatwoot/issue/CW-3331/missing-file-name-for-some-of-the-uploads-for-emails --------- Co-authored-by: Pranav --- app/presenters/mail_presenter.rb | 2 +- .../files/attachments_without_filename.eml | 43 +++++++++++++++++++ spec/mailboxes/imap/imap_mailbox_spec.rb | 16 +++++++ 3 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 spec/fixtures/files/attachments_without_filename.eml diff --git a/app/presenters/mail_presenter.rb b/app/presenters/mail_presenter.rb index 7cff55f6c..1c951cbe3 100644 --- a/app/presenters/mail_presenter.rb +++ b/app/presenters/mail_presenter.rb @@ -77,7 +77,7 @@ class MailPresenter < SimpleDelegator mail.attachments.map do |attachment| blob = ActiveStorage::Blob.create_and_upload!( io: StringIO.new(attachment.body.to_s), - filename: attachment.filename, + filename: attachment.filename.presence || "attachment_#{SecureRandom.hex(4)}", content_type: attachment.content_type ) { original: attachment, blob: blob } diff --git a/spec/fixtures/files/attachments_without_filename.eml b/spec/fixtures/files/attachments_without_filename.eml new file mode 100644 index 000000000..58b1e722a --- /dev/null +++ b/spec/fixtures/files/attachments_without_filename.eml @@ -0,0 +1,43 @@ +From: test@gmail.com +Date: Thu, 4 May 2023 10:35:52 +0530 +Message-ID: <6215d536e0484_10bc6191402183@tejaswinis-MacBook-Pro.local.mail> +Subject: multiple attachments +To: test@outlook.com +Content-Type: multipart/mixed; boundary="0000000000002488f405fad721cc" + +--0000000000002488f405fad721cc +Content-Type: multipart/alternative; boundary="0000000000002488f205fad721ca" + +--0000000000002488f205fad721ca +Content-Type: text/plain; charset="UTF-8" +Content-Transfer-Encoding: quoted-printable +Hi people! + +We are excited to inform you that we have recently released some new +features and several updates to our platform. These features and updates +are designed to enhance your experience and make your trading journey +seamless and efficient. + + + +> Okay noted + + +--0000000000002488f205fad721ca-- +--0000000000002488f405fad721cc +Content-Type: image/png; name="" +Content-Disposition: attachment; filename="" +Content-Transfer-Encoding: base64 +Content-ID: +X-Attachment-Id: f_lh8nwk8l3 + + +--0000000000002488f405fad721cc +Content-Type: image/png; name="" +Content-Disposition: attachment; filename="" +Content-Transfer-Encoding: base64 +Content-ID: +X-Attachment-Id: f_lh8nwk8l2 + + +--0000000000002488f405fad721cc-- diff --git a/spec/mailboxes/imap/imap_mailbox_spec.rb b/spec/mailboxes/imap/imap_mailbox_spec.rb index bbf3629e8..fdea5b9d4 100644 --- a/spec/mailboxes/imap/imap_mailbox_spec.rb +++ b/spec/mailboxes/imap/imap_mailbox_spec.rb @@ -43,6 +43,22 @@ RSpec.describe Imap::ImapMailbox do end end + context 'when the email has attachments with no filename' do + let(:inbound_mail) { create_inbound_email_from_fixture('attachments_without_filename.eml') } + + it 'creates a conversation and a message with properly named attachments' do + expect do + class_instance.process(inbound_mail.mail, channel) + end.to change(Conversation, :count).by(1) + + last_message = conversation.messages.last + expect(last_message.attachments.count).to be 2 + + filenames = last_message.attachments.map(&:file).map { |file| file.blob.filename.to_s } + expect(filenames.all? { |filename| filename.present? && filename.start_with?('attachment_') }).to be true + end + end + context 'when the email has 15 or more attachments' do let(:inbound_mail) { create_inbound_email_from_fixture('multiple_attachments.eml') }