From 933ae8aa497cdb80538a69dad9c6d389e1997c90 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Fri, 15 Nov 2024 09:07:24 +0400 Subject: [PATCH 1/2] 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') } From 8773929c0e6fc2cd9a2342ffe180a7033f1fecc4 Mon Sep 17 00:00:00 2001 From: caspar Date: Fri, 15 Nov 2024 21:27:58 +0800 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20Fix=20line=20sticker=20URL=20to=20pr?= =?UTF-8?q?event=20certain=20images=20from=20failing=20to=20d=E2=80=A6=20(?= =?UTF-8?q?#10416)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes the issue with Line stickers URLs to prevent certain images from failing to display. The problem was due to the use of incorrect URLs. The original URLs pointed to the `iphone` variant, which failed to load properly in some cases. The fix updates the URLs to use the `android` variant, ensuring all images are displayed correctly. ### Example: - Original (failing URL): `https://stickershop.line-scdn.net/stickershop/v1/sticker/17/iphone/sticker.png` - Fixed (working URL): `https://stickershop.line-scdn.net/stickershop/v1/sticker/17/android/sticker.png` ## How Has This Been Tested? 1. Verified the updated URLs by loading multiple Line sticker images to ensure they display correctly. 2. Tested in both local and production-like environments to confirm the fix resolves the issue. 3. Reviewed logs to ensure no additional errors are generated related to Line sticker URLs. --- app/services/line/incoming_message_service.rb | 2 +- spec/services/line/incoming_message_service_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/line/incoming_message_service.rb b/app/services/line/incoming_message_service.rb index f36bce0f0..4761292d0 100644 --- a/app/services/line/incoming_message_service.rb +++ b/app/services/line/incoming_message_service.rb @@ -4,7 +4,7 @@ class Line::IncomingMessageService include ::FileTypeHelper pattr_initialize [:inbox!, :params!] - LINE_STICKER_IMAGE_URL = 'https://stickershop.line-scdn.net/stickershop/v1/sticker/%s/iphone/sticker.png'.freeze + LINE_STICKER_IMAGE_URL = 'https://stickershop.line-scdn.net/stickershop/v1/sticker/%s/android/sticker.png'.freeze def perform # probably test events diff --git a/spec/services/line/incoming_message_service_spec.rb b/spec/services/line/incoming_message_service_spec.rb index 037aa18fa..3bcdf8b04 100644 --- a/spec/services/line/incoming_message_service_spec.rb +++ b/spec/services/line/incoming_message_service_spec.rb @@ -178,7 +178,7 @@ describe Line::IncomingMessageService do described_class.new(inbox: line_channel.inbox, params: sticker_params).perform expect(line_channel.inbox.conversations).not_to eq(0) expect(Contact.all.first.name).to eq('LINE Test') - expect(line_channel.inbox.messages.first.content).to eq('![sticker-52002738](https://stickershop.line-scdn.net/stickershop/v1/sticker/52002738/iphone/sticker.png)') + expect(line_channel.inbox.messages.first.content).to eq('![sticker-52002738](https://stickershop.line-scdn.net/stickershop/v1/sticker/52002738/android/sticker.png)') end end