Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Mailer] Remove line breaks in email attachment content with Sendgrid API#33672
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| $email =newEmail(); | ||
| $email->from('foo@example.com') | ||
| ->to('bar@example.com') | ||
| ->attach('Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod','lorem.txt'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There is no line-break in this content, so this is not testing your patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@stof it does, line-breaks are added by the MIME-compliant base64 encoder:
| return$firstLine.trim(chunk_split($encodedString,$maxLineLength,"\r\n")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is so confusing that we should add a comment before the->attach() method. Something like:
// even it content doesn't include new lines, the Base64 encoding performed later may add them->attach('...)src/Symfony/Component/Mailer/Bridge/Sendgrid/Tests/Http/Api/SendgridTransportTest.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| $email =newEmail(); | ||
| $email->from('foo@example.com') | ||
| ->to('bar@example.com') | ||
| ->attach('Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod','lorem.txt'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is so confusing that we should add a comment before the->attach() method. Something like:
// even it content doesn't include new lines, the Base64 encoding performed later may add them->attach('...)stof commentedSep 24, 2019
Maybe the actual issue is the usage of Btw, this may also impact other API providers. |
fabpot commentedSep 24, 2019
@stof is right. We had the same kind of issue with |
chalasr commentedSep 24, 2019
The SendGrid HTTP API requires attachments to be base64 encoded though (but with "strict" base64 i.e. no line-break). But API transports could indeed bypass the encoding system and use |
d329461 toa28a7f9Comparenicolas-grekas commentedJan 4, 2020
Thank you@fyfey. |
…tuart Fyfe)This PR was squashed before being merged into the 4.3 branch.Discussion----------[Mailer] Remove line breaks in email attachment contentLine breaks are not allowed in attachment content when sending over theAPI.| Q | A| ------------- | ---| Branch? | 4.3| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets |Fix#33671,Closes#32645| License | MIT| Doc PR |This is a fix for#33671. Send grid's API throws a 400 error when sending email attachments with default base64 encoding.Removing the line breaks resolves this issue.Commits-------a28a7f9 [Mailer] Remove line breaks in email attachment content
Uh oh!
There was an error while loading.Please reload this page.
Line breaks are not allowed in attachment content when sending over the
API.
This is a fix for#33671. Send grid's API throws a 400 error when sending email attachments with default base64 encoding.
Removing the line breaks resolves this issue.