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] Fix attachment content encoding for SendgridTransport#32645
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
| } | ||
| /** | ||
| * @internal |
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.
it doesn't make sense to me to have an internal method for cross component uses
if we think this is a needed API, we should commit to it
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.
I'm against making this public, that leaks internal information I don't want to expose.
| $filename =$headers->getHeaderParameter('Content-Disposition','filename'); | ||
| $disposition =$headers->getHeaderBody('Content-Disposition'); | ||
| // Sendgrid does not accept line breaks for base64 encoded attachments | ||
| $content ='base64' ===$attachment->getEncoding() ?base64_encode($attachment->getBody()) :$attachment->bodyToString(); |
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 looks like bypassing the encoding abstraction
can't we instead decide for the encoding earlier? (a line-break-less variant of base64)
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.
We can have a specific base64 encoder, but it can’t be done earlier since the email is not aware of the transport. The transport would have to set the encoder on each attachment (DataPart instance) at send time
…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.
Using the SendgridTransport (HTTP), emails cannot be sent as soon as they have attachments.
This is due to Sendgrid not supporting line breaks in base64 encoded content (encoding is considered invalid, which results in a 400 response from the Sendgrid API).
It has been reported multiple times in sendgrid official integrations, and the answer was always "please open a PR to document this in our troubleshooting section, strict base64 is required".
See limitation documented herehttps://github.com/sendgrid/sendgrid-ruby/blob/master/USE_CASES.md#adding-attachments and a bug reportsendgrid/sendgrid-php#272.