From ff7d0958b2a95bc93dcb57a52f80312ef9e05f14 Mon Sep 17 00:00:00 2001 From: Alex Castillo Date: Tue, 8 Sep 2026 13:16:13 -0400 Subject: [PATCH 1/3] Split recipient strings without breaking a quoted display name address_list splits to/cc/bcc/from/reply_to on every comma, so a name with a comma in it -- "Doe, Jane" , which is exactly what Courrier::Email::Address.with_name produces -- is torn into two malformed addresses. address_line then rejoins the pieces with ", ", which hides the break for a single recipient but still rewrites any name whose comma is not already followed by one space. Splitting now skips commas inside a double-quoted display name. Smtp2go and Lettermint, which still split by hand, go through the shared helper too, which also drops their empty "" entries the way #61 did elsewhere. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GtYgwYNfJ3wHrw9xrooVoB --- lib/courrier/email/providers/base.rb | 11 ++- lib/courrier/email/providers/lettermint.rb | 8 +- lib/courrier/email/providers/smtp2go.rb | 6 +- test/courrier/email/providers/brevo_test.rb | 9 +++ .../email/providers/lettermint_test.rb | 79 +++++++++++++++++++ test/courrier/email/providers/mailgun_test.rb | 9 +++ test/courrier/email/providers/smtp2go_test.rb | 66 ++++++++++++++++ test/courrier/providers/base_test.rb | 37 +++++++++ 8 files changed, 216 insertions(+), 9 deletions(-) create mode 100644 test/courrier/email/providers/lettermint_test.rb create mode 100644 test/courrier/email/providers/smtp2go_test.rb diff --git a/lib/courrier/email/providers/base.rb b/lib/courrier/email/providers/base.rb index 0e2fe2e..70c4371 100644 --- a/lib/courrier/email/providers/base.rb +++ b/lib/courrier/email/providers/base.rb @@ -41,14 +41,21 @@ def headers def default_headers = {} def address_list(value, as: :email) - list = value&.to_s&.split(",")&.map(&:strip)&.reject(&:empty?) - return unless list && !list.empty? + list = split_addresses(value).map(&:strip).reject(&:empty?) + return if list.empty? list.map { |address| address_element(address, as) } end def address_line(value) = address_list(value, as: :plain)&.join(", ") + # Split a recipient string on the commas that separate addresses, while + # leaving a comma inside a quoted display name — `"Doe, Jane" `, + # which is what `Courrier::Email::Address.with_name` produces — untouched. + def split_addresses(value) + value.to_s.scan(/(?:"(?:\\.|[^"\\])*"|[^,])+/) + end + def address_element(address, as) case as when :email then {"email" => address} diff --git a/lib/courrier/email/providers/lettermint.rb b/lib/courrier/email/providers/lettermint.rb index 9248f53..7c6bade 100644 --- a/lib/courrier/email/providers/lettermint.rb +++ b/lib/courrier/email/providers/lettermint.rb @@ -12,10 +12,10 @@ def body { "route" => @provider_options.route, "from" => @options.from, - "to" => @options.to.to_s.split(",").map(&:strip), - "cc" => @options.cc&.split(",")&.map(&:strip), - "bcc" => @options.bcc&.split(",")&.map(&:strip), - "reply_to" => @options.reply_to&.split(",")&.map(&:strip), + "to" => address_list(@options.to, as: :plain), + "cc" => address_list(@options.cc, as: :plain), + "bcc" => address_list(@options.bcc, as: :plain), + "reply_to" => address_list(@options.reply_to, as: :plain), "subject" => @options.subject, "html" => @options.html, "text" => @options.text diff --git a/lib/courrier/email/providers/smtp2go.rb b/lib/courrier/email/providers/smtp2go.rb index 38dd83b..e3c2b56 100644 --- a/lib/courrier/email/providers/smtp2go.rb +++ b/lib/courrier/email/providers/smtp2go.rb @@ -9,9 +9,9 @@ class Smtp2go < Base def body { "sender" => @options.from, - "to" => @options.to.to_s.split(",").map(&:strip), - "cc" => @options.cc&.split(",")&.map(&:strip), - "bcc" => @options.bcc&.split(",")&.map(&:strip), + "to" => address_list(@options.to, as: :plain), + "cc" => address_list(@options.cc, as: :plain), + "bcc" => address_list(@options.bcc, as: :plain), "subject" => @options.subject, "html_body" => @options.html, "text_body" => @options.text diff --git a/test/courrier/email/providers/brevo_test.rb b/test/courrier/email/providers/brevo_test.rb index b6a0438..0caf21a 100644 --- a/test/courrier/email/providers/brevo_test.rb +++ b/test/courrier/email/providers/brevo_test.rb @@ -30,6 +30,15 @@ def test_formats_transactional_email ) end + def test_keeps_a_comma_inside_a_quoted_display_name + recipient = Courrier::Email::Address.with_name("jane@example.com", "Doe, Jane") + email = TestEmail.new(from: "devs@railsdesigner.com", to: "#{recipient}, bob@example.com") + + body = Brevo.new(api_key: "test_key", options: email.options).body + + assert_equal [{"email" => recipient}, {"email" => "bob@example.com"}], body["to"] + end + def test_authenticates_with_api_key assert_equal({"api-key" => "test_key"}, @provider.send(:default_headers)) end diff --git a/test/courrier/email/providers/lettermint_test.rb b/test/courrier/email/providers/lettermint_test.rb new file mode 100644 index 0000000..dec8562 --- /dev/null +++ b/test/courrier/email/providers/lettermint_test.rb @@ -0,0 +1,79 @@ +require "test_helper" + +module Courrier::Email::Providers + class LettermintTest < Minitest::Test + def setup + email = TestEmail.new( + from: "devs@railsdesigner.com", + to: "first@example.com, second@example.com", + reply_to: "support@railsdesigner.com", + cc: "copy@example.com", + bcc: "archive@example.com" + ) + + @provider = provider_for(email) + end + + def test_formats_transactional_email + assert_equal( + { + "route" => "test_route", + "from" => "devs@railsdesigner.com", + "to" => ["first@example.com", "second@example.com"], + "cc" => ["copy@example.com"], + "bcc" => ["archive@example.com"], + "reply_to" => ["support@railsdesigner.com"], + "subject" => "Test Subject", + "html" => "

Test HTML Body

", + "text" => "Test Body" + }, + @provider.body + ) + end + + def test_keeps_a_comma_inside_a_quoted_display_name + recipient = Courrier::Email::Address.with_name("jane@example.com", "Doe, Jane") + email = TestEmail.new(from: "devs@railsdesigner.com", to: "#{recipient}, bob@example.com") + + assert_equal [recipient, "bob@example.com"], provider_for(email).body["to"] + end + + def test_omits_empty_address_fields + email = TestEmail.new(from: "devs@railsdesigner.com", to: "first@example.com", cc: "", bcc: " ", reply_to: ",") + + body = provider_for(email).body + + refute_includes body.keys, "cc" + refute_includes body.keys, "bcc" + refute_includes body.keys, "reply_to" + end + + def test_authenticates_with_api_key + assert_equal({"x-lettermint-token" => "test_key"}, @provider.send(:default_headers)) + end + + def test_is_available_through_provider_registry + mock_provider = Minitest::Mock.new + mock_provider.expect(:deliver, nil) + + Lettermint.stub :new, mock_provider do + Courrier::Email::Provider.new( + provider: "lettermint", + api_key: "test_key", + options: @provider.instance_variable_get(:@options) + ).deliver + end + + mock_provider.verify + end + + private + + def provider_for(email) + provider_options = Courrier::Configuration::ProviderConfig.new + provider_options.route = "test_route" + + Lettermint.new(api_key: "test_key", options: email.options, provider_options: provider_options) + end + end +end diff --git a/test/courrier/email/providers/mailgun_test.rb b/test/courrier/email/providers/mailgun_test.rb index 56c68da..3ef4d7c 100644 --- a/test/courrier/email/providers/mailgun_test.rb +++ b/test/courrier/email/providers/mailgun_test.rb @@ -43,6 +43,15 @@ def test_omits_empty_address_fields refute_includes body.keys, "bcc" end + def test_keeps_a_comma_inside_a_quoted_display_name + # `address_line` splits then rejoins with ", ", so a name whose comma is not + # already followed by exactly one space is the case that gets rewritten. + recipient = Courrier::Email::Address.with_name("jane@example.com", "Doe,Jane") + body = provider_for(TestEmail.new(from: "devs@railsdesigner.com", to: "first@example.com", cc: recipient)).body + + assert_equal recipient, body["cc"] + end + def test_builds_endpoint_url_from_domain assert_equal "https://api.mailgun.net/v3/railsdesigner.com/messages", @provider.send(:endpoint_url) end diff --git a/test/courrier/email/providers/smtp2go_test.rb b/test/courrier/email/providers/smtp2go_test.rb new file mode 100644 index 0000000..ab9a6c3 --- /dev/null +++ b/test/courrier/email/providers/smtp2go_test.rb @@ -0,0 +1,66 @@ +require "test_helper" + +module Courrier::Email::Providers + class Smtp2goTest < Minitest::Test + def setup + email = TestEmail.new( + from: "devs@railsdesigner.com", + to: "first@example.com, second@example.com", + cc: "copy@example.com", + bcc: "archive@example.com" + ) + + @provider = Smtp2go.new(api_key: "test_key", options: email.options) + end + + def test_formats_transactional_email + assert_equal( + { + "sender" => "devs@railsdesigner.com", + "to" => ["first@example.com", "second@example.com"], + "cc" => ["copy@example.com"], + "bcc" => ["archive@example.com"], + "subject" => "Test Subject", + "html_body" => "

Test HTML Body

", + "text_body" => "Test Body" + }, + @provider.body + ) + end + + def test_keeps_a_comma_inside_a_quoted_display_name + recipient = Courrier::Email::Address.with_name("jane@example.com", "Doe, Jane") + email = TestEmail.new(from: "devs@railsdesigner.com", to: "#{recipient}, bob@example.com") + + assert_equal [recipient, "bob@example.com"], Smtp2go.new(api_key: "test_key", options: email.options).body["to"] + end + + def test_omits_empty_address_fields + email = TestEmail.new(from: "devs@railsdesigner.com", to: "first@example.com", cc: "", bcc: " ") + + body = Smtp2go.new(api_key: "test_key", options: email.options).body + + refute_includes body.keys, "cc" + refute_includes body.keys, "bcc" + end + + def test_authenticates_with_api_key + assert_equal({"X-Smtp2go-Api-Key" => "test_key"}, @provider.send(:default_headers)) + end + + def test_is_available_through_provider_registry + mock_provider = Minitest::Mock.new + mock_provider.expect(:deliver, nil) + + Smtp2go.stub :new, mock_provider do + Courrier::Email::Provider.new( + provider: "smtp2go", + api_key: "test_key", + options: @provider.instance_variable_get(:@options) + ).deliver + end + + mock_provider.verify + end + end +end diff --git a/test/courrier/providers/base_test.rb b/test/courrier/providers/base_test.rb index b5530cb..d21476d 100644 --- a/test/courrier/providers/base_test.rb +++ b/test/courrier/providers/base_test.rb @@ -68,4 +68,41 @@ def test_headers_returns_only_default_headers_when_no_custom_headers assert_equal({}, headers) end + + def test_address_list_splits_a_plain_comma_separated_list + assert_equal( + [{"email" => "first@example.com"}, {"email" => "second@example.com"}], + @provider.send(:address_list, "first@example.com, second@example.com") + ) + end + + def test_address_list_keeps_a_comma_inside_a_quoted_display_name + recipient = Courrier::Email::Address.with_name("jane@example.com", "Doe, Jane") + + assert_equal [{"email" => recipient}], @provider.send(:address_list, recipient) + assert_equal( + [{"email" => recipient}, {"email" => "bob@example.com"}], + @provider.send(:address_list, "#{recipient}, bob@example.com") + ) + end + + def test_address_list_tolerates_an_escaped_quote_in_the_display_name + recipient = Courrier::Email::Address.with_name("j@example.com", 'John "JD" Doe') + + assert_equal [recipient], @provider.send(:address_list, recipient, as: :plain) + end + + def test_address_list_returns_nil_for_blank_input + assert_nil @provider.send(:address_list, nil) + assert_nil @provider.send(:address_list, "") + assert_nil @provider.send(:address_list, " ") + assert_nil @provider.send(:address_list, ",") + end + + def test_address_line_joins_without_breaking_a_quoted_name + recipient = Courrier::Email::Address.with_name("jane@example.com", "Doe, Jane") + line = "#{recipient}, bob@example.com" + + assert_equal line, @provider.send(:address_line, line) + end end From fd46407de9239f8eac973999d93b68610e4741ef Mon Sep 17 00:00:00 2001 From: Alex Castillo Date: Tue, 8 Sep 2026 13:20:25 -0400 Subject: [PATCH 2/3] Route SES recipient lists through the shared address helper too SES built ToAddresses / CcAddresses / BccAddresses / ReplyToAddresses with Array(@options.to), which wraps a comma-separated string in a one-element list instead of splitting it -- so several recipients went out as one malformed address (the same shape as #59), and an empty cc became [""]. It now uses address_list, like every other provider. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GtYgwYNfJ3wHrw9xrooVoB --- lib/courrier/email/providers/ses.rb | 8 +-- test/courrier/email/providers/ses_test.rb | 79 +++++++++++++++++++++++ 2 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 test/courrier/email/providers/ses_test.rb diff --git a/lib/courrier/email/providers/ses.rb b/lib/courrier/email/providers/ses.rb index 55e52bb..ce87874 100644 --- a/lib/courrier/email/providers/ses.rb +++ b/lib/courrier/email/providers/ses.rb @@ -28,12 +28,12 @@ def body { "FromEmailAddress" => @options.from, "Destination" => { - "ToAddresses" => Array(@options.to), - "CcAddresses" => @options.cc ? Array(@options.cc) : nil, - "BccAddresses" => @options.bcc ? Array(@options.bcc) : nil + "ToAddresses" => address_list(@options.to, as: :plain), + "CcAddresses" => address_list(@options.cc, as: :plain), + "BccAddresses" => address_list(@options.bcc, as: :plain) }.compact, - "ReplyToAddresses" => @options.reply_to ? Array(@options.reply_to) : nil, + "ReplyToAddresses" => address_list(@options.reply_to, as: :plain), "Content" => { "Simple" => { "Subject" => {"Data" => @options.subject}, diff --git a/test/courrier/email/providers/ses_test.rb b/test/courrier/email/providers/ses_test.rb new file mode 100644 index 0000000..0075a4e --- /dev/null +++ b/test/courrier/email/providers/ses_test.rb @@ -0,0 +1,79 @@ +require "test_helper" + +module Courrier::Email::Providers + class SesTest < Minitest::Test + def setup + email = TestEmail.new( + from: "devs@railsdesigner.com", + to: "first@example.com, second@example.com", + reply_to: "support@railsdesigner.com", + cc: "copy@example.com", + bcc: "archive@example.com" + ) + + @provider = Ses.new(api_key: "test_key", options: email.options) + end + + def test_formats_transactional_email + assert_equal( + { + "FromEmailAddress" => "devs@railsdesigner.com", + "Destination" => { + "ToAddresses" => ["first@example.com", "second@example.com"], + "CcAddresses" => ["copy@example.com"], + "BccAddresses" => ["archive@example.com"] + }, + "ReplyToAddresses" => ["support@railsdesigner.com"], + "Content" => { + "Simple" => { + "Subject" => {"Data" => "Test Subject"}, + "Body" => { + "Text" => {"Data" => "Test Body"}, + "Html" => {"Data" => "

Test HTML Body

"} + } + } + } + }, + @provider.body + ) + end + + def test_splits_a_comma_separated_recipient_string + email = TestEmail.new(from: "devs@railsdesigner.com", to: "a@example.com, b@example.com, c@example.com") + + assert_equal ["a@example.com", "b@example.com", "c@example.com"], Ses.new(api_key: "k", options: email.options).body["Destination"]["ToAddresses"] + end + + def test_keeps_a_comma_inside_a_quoted_display_name + recipient = Courrier::Email::Address.with_name("jane@example.com", "Doe, Jane") + email = TestEmail.new(from: "devs@railsdesigner.com", to: "#{recipient}, bob@example.com") + + assert_equal [recipient, "bob@example.com"], Ses.new(api_key: "k", options: email.options).body["Destination"]["ToAddresses"] + end + + def test_omits_empty_address_fields + email = TestEmail.new(from: "devs@railsdesigner.com", to: "first@example.com", cc: "", bcc: " ", reply_to: ",") + + body = Ses.new(api_key: "k", options: email.options).body + + refute_includes body["Destination"].keys, "CcAddresses" + refute_includes body["Destination"].keys, "BccAddresses" + refute_includes body.keys, "ReplyToAddresses" + end + + def test_is_available_through_provider_registry + mock_provider = Minitest::Mock.new + mock_provider.expect(:deliver, nil) + + Ses.stub :new, mock_provider do + Courrier::Email::Provider.new( + provider: "ses", + api_key: "test_key", + options: @provider.instance_variable_get(:@options) + ).deliver + end + + mock_provider.verify + end + end +end From 34d2934b04aa9edff01bb35aee117220930a6ee0 Mon Sep 17 00:00:00 2001 From: Alex Castillo Date: Tue, 8 Sep 2026 13:22:38 -0400 Subject: [PATCH 3/3] Reword the split_addresses comment in plain ASCII Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GtYgwYNfJ3wHrw9xrooVoB --- lib/courrier/email/providers/base.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/courrier/email/providers/base.rb b/lib/courrier/email/providers/base.rb index 70c4371..09ccc28 100644 --- a/lib/courrier/email/providers/base.rb +++ b/lib/courrier/email/providers/base.rb @@ -49,9 +49,9 @@ def address_list(value, as: :email) def address_line(value) = address_list(value, as: :plain)&.join(", ") - # Split a recipient string on the commas that separate addresses, while - # leaving a comma inside a quoted display name — `"Doe, Jane" `, - # which is what `Courrier::Email::Address.with_name` produces — untouched. + # Split a recipient string on the commas that separate addresses, leaving + # a comma inside a quoted display name (`"Doe, Jane" `, + # which is what `Courrier::Email::Address.with_name` produces) untouched. def split_addresses(value) value.to_s.scan(/(?:"(?:\\.|[^"\\])*"|[^,])+/) end