fix(mailbox): render inline images without Content-Disposition (#11949)
## Description This pull request addresses issue #11948, where inline images embedded in emails (such as those sent from Outlook) are not rendered correctly if the Content-Disposition header is missing. The solution ensures that images referenced via cid: in the HTML body are correctly identified and rewritten using url_for. Fixes #11948 ## Type of change Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? Added test: detects image inline attachment by cid reference when Content-Disposition is missing ## Checklist: - [X] My code follows the style guidelines of this project - [X] I have performed a self-review of my code - [X] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [X] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules --------- Co-authored-by: Pranav <pranav@chatwoot.com> Co-authored-by: Sojan Jose <sojan@pepalo.com> Co-authored-by: Sony Mathew <sony@chatwoot.com> Co-authored-by: Sony Mathew <2040199+sony-mathew@users.noreply.github.com>
This commit is contained in:
Wendell Gasparoni
co-authored by
Pranav
Sojan Jose
Sony Mathew
Sony Mathew
Pranav
Sojan Jose
Sony Mathew
Sony Mathew
parent
d43a87c9dc
commit
42bba748cf
@@ -1,4 +1,6 @@
|
||||
module MailboxHelper
|
||||
include MailboxInlineAttachmentHelper
|
||||
|
||||
private
|
||||
|
||||
def create_message
|
||||
@@ -24,6 +26,9 @@ module MailboxHelper
|
||||
def add_attachments_to_message
|
||||
return if @message.blank?
|
||||
|
||||
# Load email content once for all attachment processing
|
||||
load_email_content
|
||||
|
||||
# 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)
|
||||
@@ -38,7 +43,7 @@ module MailboxHelper
|
||||
# If the email lacks a text body or if inline attachments aren't images,
|
||||
# treat them as standard attachments for processing.
|
||||
inline_attachments = attachments.select do |attachment|
|
||||
mail_content.present? && attachment[:original].inline? && attachment[:original].content_type.to_s.start_with?('image/')
|
||||
inline_attachment?(attachment)
|
||||
end
|
||||
|
||||
regular_attachments = attachments - inline_attachments
|
||||
@@ -59,11 +64,6 @@ module MailboxHelper
|
||||
def process_inline_attachments(attachments)
|
||||
Rails.logger.info "[MailboxHelper] Processing inline attachments for message with ID: #{processed_mail.message_id}"
|
||||
|
||||
# create an instance variable here, the `embed_inline_image_source`
|
||||
# updates them directly. And then the value is eventaully used to update the message content
|
||||
@html_content = processed_mail.serialized_data[:html_content][:full]
|
||||
@text_content = processed_mail.serialized_data[:text_content][:reply]
|
||||
|
||||
attachments.each do |mail_attachment|
|
||||
embed_inline_image_source(mail_attachment)
|
||||
end
|
||||
@@ -81,12 +81,6 @@ module MailboxHelper
|
||||
end
|
||||
end
|
||||
|
||||
def upload_inline_image(mail_attachment)
|
||||
content_id = mail_attachment[:original].cid
|
||||
|
||||
@html_content = @html_content.gsub("cid:#{content_id}", inline_image_url(mail_attachment[:blob]).to_s)
|
||||
end
|
||||
|
||||
def embed_plain_text_email_with_inline_image(mail_attachment)
|
||||
attachment_name = mail_attachment[:original].filename
|
||||
img_tag = "<img src=\"#{inline_image_url(mail_attachment[:blob])}\" alt=\"#{attachment_name}\">"
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
module MailboxInlineAttachmentHelper
|
||||
private
|
||||
|
||||
def load_email_content
|
||||
@html_content = processed_mail.serialized_data[:html_content][:full]
|
||||
@text_content = processed_mail.serialized_data[:text_content][:reply]
|
||||
end
|
||||
|
||||
def inline_attachment?(attachment)
|
||||
# Only process images as potential inline attachments
|
||||
return false unless mail_content.present? && attachment[:original].content_type.to_s.start_with?('image/')
|
||||
|
||||
# Check if attachment is explicitly marked as inline
|
||||
return true if attachment[:original].inline?
|
||||
|
||||
# For Outlook compatibility: if not marked as inline but has CID and is referenced in body
|
||||
cid = attachment[:original].cid
|
||||
cid.present? && body_references_cid?(cid)
|
||||
end
|
||||
|
||||
def body_references_cid?(cid)
|
||||
# Check if CID is referenced in HTML content
|
||||
return false if @html_content.blank?
|
||||
|
||||
cid_urls_for(cid).any? { |cid_url| @html_content.include?(cid_url) }
|
||||
end
|
||||
|
||||
def upload_inline_image(mail_attachment)
|
||||
content_id = mail_attachment[:original].cid
|
||||
image_url = inline_image_url(mail_attachment[:blob]).to_s
|
||||
|
||||
cid_urls_for(content_id).each do |cid_url|
|
||||
@html_content = @html_content.gsub(cid_url, image_url)
|
||||
end
|
||||
end
|
||||
|
||||
def cid_urls_for(cid)
|
||||
# RFC 2392 cid URLs can contain URL-encoded Content-ID values.
|
||||
# Check both raw and encoded variants so clients using either form render inline images.
|
||||
encoded_cid = ERB::Util.url_encode(cid)
|
||||
lowercase_encoded_cid = encoded_cid.gsub(/%[0-9A-F]{2}/, &:downcase)
|
||||
|
||||
["cid:#{cid}", "cid:#{encoded_cid}", "cid:#{lowercase_encoded_cid}"].uniq
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,83 @@
|
||||
Delivered-To: test.user@example.com
|
||||
Received: by 192.0.2.1 with SMTP id smtp12345;
|
||||
Mon, 14 Jul 2025 15:23:57 -0700 (PDT)
|
||||
X-Google-Smtp-Source: TEST_SOURCE
|
||||
X-Received: by 198.51.100.2 with SMTP id smtp67890;
|
||||
Mon, 14 Jul 2025 15:23:57 -0700 (PDT)
|
||||
ARC-Seal: i=1; a=rsa-sha256; t=1752531837; cv=none;
|
||||
d=example.com; s=test;
|
||||
b=TEST_SIGNATURE==
|
||||
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=example.com; s=test;
|
||||
h=content-language:subject:to:from;
|
||||
bh=TESTHASH==;
|
||||
b=TEST_SIGNATURE==
|
||||
ARC-Authentication-Results: i=1; mx.example.com;
|
||||
dkim=pass header.i=@example.com header.s=test header.b="TESTKEY";
|
||||
spf=pass smtp.mailfrom=sender@example.com
|
||||
Return-Path: <sender@example.com>
|
||||
Received: from smtp.example.com (smtp.example.com. [203.0.113.10])
|
||||
by mx.example.com with ESMTPS id smtp7890
|
||||
for <test.user@example.com>;
|
||||
Mon, 14 Jul 2025 15:23:55 -0700 (PDT)
|
||||
Received-SPF: pass (example.com: domain of sender@example.com designates 203.0.113.10 as permitted sender)
|
||||
Authentication-Results: mx.example.com;
|
||||
dkim=pass header.i=@example.com header.s=test header.b="TESTKEY";
|
||||
spf=pass smtp.mailfrom=sender@example.com
|
||||
Received: from TEST-PC (unknown [192.0.2.100])
|
||||
(Authenticated sender: sender@example.com)
|
||||
by smtp.example.com (Postfix) with ESMTPA id ABCD123456
|
||||
for <test.user@example.com>; Mon, 14 Jul 2025 19:23:43 -0300 (BRT)
|
||||
DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=example.com;
|
||||
s=test; t=1752531824; bh=TESTHASH=;
|
||||
h=From:To:Subject:Date:From;
|
||||
b=TEST_SIGNATURE==
|
||||
From: <sender@example.com>
|
||||
To: <test.user@example.com>
|
||||
Subject: Test inline image without Content-Disposition
|
||||
Date: Mon, 14 Jul 2025 19:23:29 -0300
|
||||
Message-ID: <test-message-id@example.com>
|
||||
MIME-Version: 1.0
|
||||
Content-Type: multipart/related;
|
||||
boundary="----=_NextPart_000_0001"
|
||||
X-Mailer: Microsoft Outlook 16.0
|
||||
Thread-Index: TESTINDEX==
|
||||
Content-Language: en-us
|
||||
|
||||
This is a multipart message in MIME format.
|
||||
|
||||
------=_NextPart_000_0001
|
||||
Content-Type: multipart/alternative;
|
||||
boundary="----=_NextPart_001_0002"
|
||||
|
||||
------=_NextPart_001_0002
|
||||
Content-Type: text/plain;
|
||||
charset="us-ascii"
|
||||
Content-Transfer-Encoding: 7bit
|
||||
|
||||
This is a plain text version of the message.
|
||||
|
||||
------=_NextPart_001_0002
|
||||
Content-Type: text/html;
|
||||
charset="us-ascii"
|
||||
Content-Transfer-Encoding: quoted-printable
|
||||
|
||||
<html>
|
||||
<body>
|
||||
<p>This is an HTML message with an inline image.</p>
|
||||
<img src="cid:image001.jpg@test">
|
||||
</body>
|
||||
</html>
|
||||
|
||||
------=_NextPart_001_0002--
|
||||
|
||||
------=_NextPart_000_0001
|
||||
Content-Type: image/jpeg;
|
||||
filename="image001.jpg"
|
||||
Content-Transfer-Encoding: base64
|
||||
Content-ID: <image001.jpg@test>
|
||||
|
||||
/9j/4AAQSkZJRgABAQEASABIAAD/2wCEAAEBAQEBAQEBAQEBAQEBAQEBAQEB
|
||||
AQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQH/wAAL
|
||||
CABkAGQBAREA/8QA...
|
||||
|
||||
------=_NextPart_000_0001--
|
||||
@@ -77,4 +77,60 @@ RSpec.describe MailboxHelper do
|
||||
expect(text_content).to include(Rails.application.routes.url_helpers.url_for(mail_attachment[:blob]))
|
||||
end
|
||||
end
|
||||
|
||||
describe '#body_references_cid?' do
|
||||
let(:helper_instance) { mailbox_helper_obj.new(conversation, processed_mail) }
|
||||
|
||||
it 'detects percent-encoded CID references in HTML content' do
|
||||
helper_instance.instance_variable_set(:@html_content, '<img src="cid:image001.jpg%40test">')
|
||||
|
||||
expect(helper_instance.send(:body_references_cid?, 'image001.jpg@test')).to be true
|
||||
end
|
||||
end
|
||||
|
||||
describe '#upload_inline_image' do
|
||||
let(:mail_attachment) do
|
||||
{
|
||||
original: OpenStruct.new(cid: 'image001.jpg@test'),
|
||||
blob: get_blob_for('spec/assets/avatar.png', 'image/png')
|
||||
}
|
||||
end
|
||||
let(:helper_instance) { mailbox_helper_obj.new(conversation, processed_mail) }
|
||||
|
||||
it 'replaces percent-encoded CID references in HTML content' do
|
||||
allow(Rails.application.routes.url_helpers).to receive(:url_for).and_return('/fake-image-url')
|
||||
helper_instance.instance_variable_set(:@html_content, '<img src="cid:image001.jpg%40test">')
|
||||
|
||||
helper_instance.send(:upload_inline_image, mail_attachment)
|
||||
|
||||
html_content = helper_instance.instance_variable_get(:@html_content)
|
||||
expect(html_content).to include('/fake-image-url"')
|
||||
expect(html_content).not_to include('cid:')
|
||||
end
|
||||
end
|
||||
|
||||
describe '#add_attachments_to_message' do
|
||||
let(:mail) { create_inbound_email_from_fixture('cid_inline_images_without_disposition.eml').mail }
|
||||
let(:processed_mail) { MailPresenter.new(mail) }
|
||||
let(:conversation) { create(:conversation) }
|
||||
let(:helper_instance) { mailbox_helper_obj.new(conversation, processed_mail) }
|
||||
|
||||
before do
|
||||
helper_instance.send(:create_message)
|
||||
end
|
||||
|
||||
it 'detects inline image attachment by cid reference when Content-Disposition is missing' do
|
||||
allow(Rails.application.routes.url_helpers).to receive(:url_for).and_return('/fake-image-url')
|
||||
helper_instance.send(:add_attachments_to_message)
|
||||
|
||||
message = conversation.messages[0]
|
||||
|
||||
expect(message.attachments.count).to eq(0)
|
||||
|
||||
html_content = message.content_attributes[:email][:html_content][:full]
|
||||
|
||||
expect(html_content).to include('/fake-image-url"')
|
||||
expect(html_content).not_to include('cid:')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user