Skip to content

[Mailer] fix: stringify from address for ses+api transport #45691

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.

Sign up for GitHub

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

Jump to bottom
Merged
fabpot merged 1 commit into from
Mar 14, 2022
Merged

[Mailer] fix: stringify from address for ses+api transport #45691

fabpot merged 1 commit into from
Mar 14, 2022

Conversation

@everyx
Copy link
Contributor

@everyx everyx commented Mar 10, 2022

close: #45660

Q A
Branch? 6.0 for bug fixes
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #45660
License MIT
Doc PR

There are non-ASCII characters in the email address—The email address string must be 7-bit ASCII. If you want to send to or from email addresses that contain Unicode characters in the domain part of an address, you must encode the domain using Punycode. Punycode is not permitted in the local part of the email address (the part before the @ sign) nor in the "friendly from" name. If you want to use Unicode characters in the "friendly from" name, you must encode the "friendly from" name using MIME encoded-word syntax, as described in Sending raw email using the Amazon SES API. For more information about Punycode, see RFC 3492.

- from Amazon SES email sending errors

We have already stringify to addresses, and we should also stringify the from address.

@carsonbot carsonbot added this to the 6.0 milestone Mar 10, 2022
@carsonbot
Copy link

carsonbot commented Mar 10, 2022

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@carsonbot carsonbot changed the title fix: stringify from address for ses+api transport [Mailer] fix: stringify from address for ses+api transport Mar 10, 2022
@fancyweb fancyweb modified the milestones: 6.0, 5.4 Mar 10, 2022
@carsonbot
Copy link

carsonbot commented Mar 10, 2022

Hey!

I think @driesvints has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

driesvints
driesvints suggested changes Mar 14, 2022
View reviewed changes
Copy link
Contributor

@driesvints driesvints left a comment

Choose a reason for hiding this comment

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

Small code style here.

@fabpot fabpot changed the base branch from 6.0 to 5.4 March 14, 2022 10:39
@fabpot
Copy link
Member

fabpot commented Mar 14, 2022

Thank you @everyx.

@fabpot fabpot merged commit ed9f973 into symfony:5.4 Mar 14, 2022
@fabpot fabpot mentioned this pull request Apr 2, 2022
Merged
@fabpot fabpot mentioned this pull request Apr 2, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabpot fabpot fabpot approved these changes

@lyrixx lyrixx Awaiting requested review from lyrixx

@yceruto yceruto Awaiting requested review from yceruto

@wouterj wouterj Awaiting requested review from wouterj

@chalasr chalasr Awaiting requested review from chalasr

@dunglas dunglas Awaiting requested review from dunglas

@OskarStark OskarStark Awaiting requested review from OskarStark

@jderusse jderusse Awaiting requested review from jderusse

@xabbuh xabbuh Awaiting requested review from xabbuh

+1 more reviewer

@driesvints driesvints driesvints requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Bug Mailer Status: Needs Review

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

5 participants

@everyx @carsonbot @fabpot @driesvints @fancyweb