@mdecimus Can you provide feedback on this PR? If this solution is unsatisfactory, can the docs be extended to explain in more detail in what cases InlineBinary should be expected over Binary?

Hi,

I'm sorry but I don't have time to review this at the moment, I need to make sure this change does not break Stalwart Mail Server and I don't have time unfortunately.

We've taken a second look and our suggested solution was defective. The fix we propose now is much more conservative and its only effect is to recategorize some parts that are PartType::Binary into PartType::InlineBinary.

We've added a helper diff that shows the location of the email parts whose type is affected by this change to simplify review and reviewed the changes to tests manually. The affected attachments follow the blueprint of #70 and are referenced inline in the HTML, but are PartType::Binary which seems wrong, at the very least unexpected.

To speed up review we've attempted to assess the impact on mail-auth, mail-server, mail-builder and mail-send.

Nearly all mentions occur in match arms where both PartType::Binary and PartType::InlineBinary are handled the same, the following command returns all mentions where this is not the case:

rg ' PartType::(Inline)?Binary' mail-auth mail-builder mail-send mail-server \
| rg -v 'PartType::Binary.*\|.*PartType::InlineBinary|PartType::InlineBinary.*\|.*PartType::Binary'

This narrows the list down to few files that must be checked manually

mail-server/crates/jmap/src/email/ingest.rs:                        part.body = PartType::Binary(Cow::from(&[][..]));
mail-server/crates/jmap/src/email/parse.rs:                        PartType::Binary(_) | PartType::Message(_) => true,
mail-server/crates/jmap/src/email/metadata.rs:            _ => PartType::Binary(Cow::Borrowed(&[])),
mail-server/crates/jmap/src/email/metadata.rs:  MetadataPartType::Binary => PartType::Binary(bytes),
mail-server/crates/jmap/src/email/metadata.rs:  MetadataPartType::InlineBinary => PartType::InlineBinary(bytes),
mail-server/crates/jmap/src/email/metadata.rs:            PartType::Binary(contents) => (contents.len(), MetadataPartType::Binary)
mail-server/crates/jmap/src/email/metadata.rs:            PartType::InlineBinary(contents) => {
mail-server/crates/jmap/src/email/index.rs:    PartType::Binary(_) | PartType::Message(_) if !has_attachments => {

Of these cases, the only case that seems to incur any significant change is to fix the indexing, which as far as we can tell currently will index a HTML message with inline images with Property::HasAttachment, which is also unexpected to us.