search

smiley22 / S22.Imap

A free, easy-to-use and well-documented .NET library component for communicating with IMAP servers
http://smiley22.github.com/S22.Imap/Documentation/
MIT License
210 stars 112 forks source link

added check to see if body part has a name and disposition is unknown, b... #106

Open zkestenbaum opened 9 years ago

zkestenbaum commented 9 years ago

...efore putting it in the body. This fixes a problem where an email that has an empty body and an attachment (where the content disposition is not set) would have the attachment appear in the message body. With this change, the attachment correctly appears as an attachment.

NiKiZe commented 9 years ago

+1 the only thing I'm missing is unit tests for this. (and also results for if everything passes as it is now)

Would it be reasonable to assume attachment if it has a name? Or does bodyparts come with names sometime as well?

zkestenbaum commented 9 years ago

Just added a unit test for this. All unit tests pass currently. if I revert the change I made, then this new unit test fails.

I think it is a reasonable assumption, especially since the code is already making that assumption (a few lines below this spot). I think it was an oversight to not include this check in this particular spot. // If the part has a name it most likely is an attachment and it should go into the // Attachments collection. bool hasName = part.Parameters.ContainsKey("name");

jstedfast commented 9 years ago

The name/filename parameters can be set on any MIME part, whether it is meant to be shown inline as the body of the message or as an attachment.

The only way to tell if a MIME part is meant to be shown as an attachment or not is to look at the Content-Disposition value ("attachment" vs "inline").

It seems that this is what his patch does, so it looks correct to me, although it could probably be simplified to just part.Disposition.Type == ContentDispositionType.Inline instead of !(part.Disposition.Type == ContentDispositionType.Attachment || part.Disposition.Type == ContentDispositionType.Unknown)