search

jordan-wright / email

Robust and flexible email library for Go
MIT License
2.65k stars 321 forks source link

Support for inline attachments with NewEmailFromReader #119

Closed tarmo-randma closed 4 years ago

tarmo-randma commented 4 years ago

in v4.0.0 when using NewEmailFromReader to create an email, attachments are only created if Content-Disposition is 'attachment'. This PR adds support for inline attachments as well.

Links to issue #82 just like PR #89

The failing test during build was there before this change already, assumed it is a known issue

jordan-wright commented 4 years ago

Hi there!

I'll have to investigate this. Your change to:

if cd == "attachment" || cd == "inline" {

is what broke the existing test. I'll need to take a look and see what's the expected behavior there.

tarmo-randma commented 4 years ago

Sorry I missed that it worked before.

I looked at the failing test and I must say I am stumped. Sure there is an inline attachment in the test message there but I cannot make out WHY the test needs to behave in the way it behaves. Does not seem logical to me. I hope you will find some time to check it out in detail. Thanks for your work on this lib btw

tarmo-randma commented 4 years ago

So the TestMultipartNoContentType contained an inline attachment with no filename part and required this part to be parsed as the actual e-mail body. I do not know if this is expected behavior according to the standards but it does not matter for this pull request I think.

I updated the PR to only parse inline attachments if they also have a filename component defined in their Content-Disposition header. This allows the existing tests to pass and also allows supporting inline attachments. Hopefully it is acceptable now.

jordan-wright commented 4 years ago

This LGTM - thanks for sending in and being flexible with adjusting the PR. 💚