Split recipient strings without breaking a quoted display name - #63
Open
pcbeingused333 wants to merge 3 commits into
Open
pcbeingused333 wants to merge 3 commits into
pcbeingused333 wants to merge 3 commits into
Conversation
address_list splits to/cc/bcc/from/reply_to on every comma, so a name with a comma in it -- "Doe, Jane" <jane@example.com>, 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 Rails-Designer#61 did elsewhere. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GtYgwYNfJ3wHrw9xrooVoB
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 Rails-Designer#59), and an empty cc became [""]. It now uses address_list, like every other provider. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GtYgwYNfJ3wHrw9xrooVoB
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GtYgwYNfJ3wHrw9xrooVoB
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Courrier::Email::Address.with_namequotes any display name that contains a special character, sowith_name("jane@example.com", "Doe, Jane")returns:Every provider then feeds
to/cc/bcc/from/reply_tothroughProviders::Base#address_list, which doesvalue.to_s.split(","). That tears the quoted name in two:So the recipient goes out as two malformed addresses.
address_line(Mailgun, MailPace, Postmark, and SparkPost's CC header) rejoins the pieces with", ", which hides the break for a single recipient but still rewrites any name whose comma isn't already followed by exactly one space ("Doe,Jane"→"Doe, Jane").The address test suite already pins
with_nameproducing"Doe, John" <...>, so the two halves of the library disagree.The fix
split_addressesnow splits only on commas that are outside a double-quoted display name ("(?:\\.|[^"\\])*"), so a quoted name survives intact and a plaina@x, b@ylist still splits as before. Blank input (nil,""," ",",") still returnsnil.Three providers still hand-rolled their own recipient parsing and had the same class of bug; they now go through
address_listlike everyone else:@options.to.to_s.split(","), same comma-in-name break. The helper also drops their empty""entries the way Refactored into reusable email addressess list helper #61 did for the others.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 shape of Mailjet sends multiple recipients as a single malformed address #59) and an emptyccbecame[""].Tests
test/courrier/providers/base_test.rb—address_list/address_linedirectly: quoted comma, quoted comma among several addresses, escaped quote, blank input.smtp2go_test.rb,lettermint_test.rb,ses_test.rb(none had a test file) — full body, quoted-name recipient, comma-string splitting, empty-field omission, registry wiring.brevo_test(address_listpath) andmailgun_test(address_linepath).11 of the new assertions fail on
mainand pass with the fix.bundle exec rakeandbundle exec standardrbare green (181 runs, 0 failures).🤖 Generated with Claude Code