Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quote name portion of recipient for Mailgun #150

Merged
merged 3 commits into from
Aug 16, 2017

Conversation

tmjoen
Copy link
Contributor

@tmjoen tmjoen commented Aug 16, 2017

If the name portion of a recipient contains a comma, weird things will happen. With multiple recipients, some of them will be chopped off without any warnings or errors.

This changes the formatting from

Acme Inc, headquarters <[email protected]>

to

"Acme Inc, headquarters" <[email protected]>

Ref: https://stackoverflow.com/questions/35370571/how-to-add-a-comma-to-a-mailgun-from-field-for-outbound-emails

Ref: http://www.rfc-editor.org/rfc/rfc2822.txt

Note that the display names for Joe Q. Public and Giant; "Big" Box needed to be enclosed in double-quotes because the former contains the period and the latter contains both semicolon and double-quote characters (the double-quote characters appearing as quoted-pair construct).

This bit me HARD, so I hope you'll consider this PR.

Thanks!

@princemaple
Copy link
Member

Hi @tmjoen , thanks for the first time contribution! The test failure seems like an easy fix. If you could take care of that, I'll merge this PR right in. If not, I'll fix it tomorrow (Australia time).

@stevedomin this will need a patch release.

@stevedomin
Copy link
Member

Hello @tmjoen,

Thanks for the report and PR. This definitely needs to be fixed.

Your PR looks great, I've just one minor thing to ask. This bug was fixed in the Postmark adapter and I'd like to use a sigil there as well. Can you make the change?

Thanks

@stevedomin
Copy link
Member

Ah, didn't see @princemaple reply :)

I can take care of the test failure don't worry!

@tmjoen
Copy link
Contributor Author

tmjoen commented Aug 16, 2017

Thanks for the fast replies!

I pushed some fixes

@stevedomin
Copy link
Member

Thanks @tmjoen! It's perfect.

We'll push a minor version out now.

@stevedomin stevedomin merged commit eae78a6 into swoosh:master Aug 16, 2017
@stevedomin
Copy link
Member

@tmjoen version 0.9.1 is out: https://hex.pm/packages/swoosh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants