Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/mailtrap/mail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ module Mail # rubocop:disable Metrics/ModuleLength
category
customvariables
contenttype
replyto
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reject multi-address Reply-To headers instead of truncating them.

Line 199 parses a valid Reply-To mailbox-list and then keeps only .first, while Line 20 ensures the original header is stripped from headers. That silently drops every address after the first. If the API only supports a single reply_to, please raise here rather than changing reply semantics.

Possible guard
-          reply_to: prepare_addresses(message['reply-to']).first,
+          reply_to: prepare_reply_to(message['reply-to']),
+      def prepare_reply_to(header)
+        addresses = prepare_addresses(header)
+        return nil if addresses.empty?
+
+        if addresses.length > 1
+          raise Mailtrap::Error, ["failed to parse 'Reply-To': multiple addresses are not supported"]
+        end
+
+        addresses.first
+      end

Also applies to: 199-199

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/mailtrap/mail.rb` at line 20, The current code silently drops additional
addresses by removing the original Reply-To header from headers and then keeping
only the first parsed mailbox; instead, detect when the parsed mailbox list (the
value used to set replyto/reply_to) contains more than one address and raise a
clear error (e.g., ArgumentError) indicating multi-address Reply-To is
unsupported; do not strip or mutate headers when rejecting — only accept a
single mailbox and proceed, otherwise raise before modifying headers. Use the
existing identifiers replyto (or reply_to) and headers and the mailbox-list
parsing site (where .first is currently used) to implement this guard and error
message.

].freeze
private_constant :SPECIAL_HEADERS

Expand Down Expand Up @@ -195,6 +196,7 @@ def from_message(message)
to: prepare_addresses(message['to']),
cc: prepare_addresses(message['cc']),
bcc: prepare_addresses(message['bcc']),
reply_to: prepare_addresses(message['reply-to']).first,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

여러 개의 reply-to 주소가 제공되었을 때 첫 번째 주소만 자동으로 사용하는 것은 예기치 않은 동작이나 데이터 손실로 이어질 수 있습니다. mail gem은 여러 개의 Reply-To 주소를 허용하지만, Mailtrap API는 하나만 지원하는 것으로 보입니다. 이 경우, 하나보다 많은 주소가 발견되면 오류를 발생시켜 사용자에게 호환성 문제를 알리는 것이 더 견고한 방법일 것입니다.

subject: message.subject,
text: prepare_text_part(message),
html: prepare_html_part(message),
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

61 changes: 60 additions & 1 deletion spec/mailtrap/mail_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
let(:expected_headers) do
{
'X-Special-Domain-Specific-Header' => 'SecretValue',
'Reply-To' => 'Reply To <reply-to@railsware.com>',
'One-more-custom-header' => 'CustomValue'
}
end
Expand All @@ -53,6 +52,35 @@

it { is_expected.to eq(expected_headers) }
end

context 'when reply-to is added in varying formats' do
[
['"메일트랩" <support@mailtrap.io>', { email: 'support@mailtrap.io', name: '메일트랩' }],
['"Mailtrap Team" <support@mailtrap.io>', { email: 'support@mailtrap.io', name: 'Mailtrap Team' }],
['support@mailtrap.io', { email: 'support@mailtrap.io' }]
].each do |reply_to, expected_reply_to|
it "maps '#{reply_to}' to structured field and excludes header copy" do
message.reply_to = reply_to

expect(mail.reply_to).to eq(expected_reply_to)
expect(headers).not_to have_key('Reply-To')
end
end
end

context 'when custom header and reply-to variants are present' do
before do
message.header['Reply-To'] = 'Reply To <reply-to@railsware.com>'
message.header['REPLY-TO'] = 'Upper Case <upper-reply-to@railsware.com>'
message.header['reply-to'] = 'Lower Case <lower-reply-to@railsware.com>'
message.header['X-Special-Domain-Specific-Header'] = 'SecretValue'
end

it 'keeps only custom headers and strips all reply-to header variants' do
expect(mail.reply_to).to eq({ email: 'lower-reply-to@railsware.com', name: 'Lower Case' })
expect(headers).to eq('X-Special-Domain-Specific-Header' => 'SecretValue')
end
end
end

describe '#attachment' do
Expand Down Expand Up @@ -167,5 +195,36 @@
end
end
end

context "when 'reply-to' is invalid" do
let(:invalid_reply_to) { 'invalid email@example.com' }

before do
message.header['Reply-To'] = invalid_reply_to
end

it 'raises an error' do
expect { mail }.to raise_error(
Mailtrap::Error,
"failed to parse 'Reply-To': 'invalid email@example.com'"
)
end

context 'when address contains a folded line break' do
let(:invalid_reply_to) do
encoded_name = ['메일트랩 팀'.encode('UTF-8')].pack('m0')
folded_domain = %w[exa mple.com].join("\r\n ")

"=?UTF-8?B?#{encoded_name}?= <no-reply@#{folded_domain}>"
end

it 'raises an error' do
expect { mail }.to raise_error(
Mailtrap::Error,
/failed to parse 'Reply-To': '.*no-reply@exa/m
)
end
end
end
end
end