feat/encoders & feat/related-attachments

[benedikt-schaber]

Hello. I feel I need to prefix this PR with a little justification: This PR has multiple parts to it and most of them should have been discussed in an Issue beforehand. But, since we needed these features fast I sadly did not have the time to do so before beginning development. I hope it can still be useful and understand that it may need reworking.

The goal of this PR was to allow users to embed images. This necessitated or at least encouraged multiple other changes.

feat/related-attachments

Implements Content-Type: multiple/related for mimeContent.
To quote Wikipedia:

One common usage of this subtype is to send a web page complete with images in a single message. The root part would contain the HTML document, and use image tags to reference images stored in the latter parts.

Minimal Example

await client.send({
   html: '<p><b>Follow us on</b></p><p><a href=""><img src="cid:logo"></a></p>',
   relatedAttachments: [
   	{
   		content: logoB64,
   		filename: "logo",
   		encoding: "base64",
   		contentType: "image/png",
   		contentID: "logo",
   		contentDisposition: "inline",
   	}
   ],
});

Result:

Implementation

Added an optional field relatedAttachments on Content. Also added an optional field relatedAttachments on SendConfig that populates the relatedAttachments field of the html mime entry upon being resolved.

Then in SMTPClient.send check if there are relatedAttachments and if so use the encodeRelated encoder to first encode the mimeContent itself and then the attachments inside a Content-Type: multipart/related wrapper.

feat/encoders

Since we also need to encode Content and Attachment inside the related wrapper the functions have been refactored from the send method into the encode folder. Additionally the unique boundary calculation has also been refactored here and now uses matchAll instead of replace.

[mathe42]

Thanks for this PR it looks proising!

But I request some changes to line this up with my coding style and revert some thing as I intended them...

Some general notes:

1. Don't use writeCMD. Make more methods inside the SMTPClient class
2. In the config resolver the goal is that all keys exists with a good value so you don't have to do that inside the client. There are multiple instances of that. I commented on one of them

This was just a quick review in 5min and doesn't include a review of the new relatedAttachments implementation...

[mathe42]

I also have to read through some specs to see if this implementation is correct etc. Will do this evening (UTC+1).

[benedikt-schaber]

The good:

• all respective functions are now member functions of SMTPClient
• writeCMD has subsequently been banished from the code
• now config modifications are done in config/mail

The potentially bad:

• introduced the ResolvedContent type
• renamed resolveContent resolveMessage
• created a new resolveContent function

The funny:

• I had way to much fun git rebase -i-ing and fear at some point did not improve the commit history anymore

[mathe42]

I allways squash merge so no "nice" history needed.

[mathe42]

I had a quick look and it looks quite good!

I will need some time to test it and read some spec. I might add some changes on top of yours and do some renaming etc. where needed. So expect a merge in the next few days.

[mathe42]

I had a look at the code and it looks realy good! But I don't know if we need related-attachments:

I pushed a fix to feat/attachment-content-id-fix with that you can run:

import { SMTPClient } from "./mod.ts";

const transport = new SMTPClient({
    connection: {
        hostname: "sandbox.smtp.mailtrap.io",
        port: 25,
        tls: false,
        auth: {
            username: "xxx",
            password: "xxx"
        }
    },
    debug: {
        log: true,
        noStartTLS: true,
        allowUnsecure: true
    }
})



transport.send({
    to: "[email protected]",
    html: '<p><b>Follow us on</b></p><p><a href=""><img src="cid:logo"></a></p>',
    subject: "aaa",
    from: "[email protected]",
    attachments: [
        {
            filename: "test.png",
            content: Deno.readFileSync("./test/e2e/attachments/image.png"),
            encoding: "binary",
            contentType: "image/png",
            contentID: "logo"
        }
    ]
})

And it works like you would expect!

What is the difference?

(My point is only about the related attachments the rest of the code I want to merge soon 👍 )

[benedikt-schaber]

I quickly tested your example code and it did not work in all clients, most notably it did not work in Thunderbird. I can look into why it did not work further if requested.

One remark to feat/attachment-content-id-fix:
There is a small bug, Content-ID is now written twice in client.ts
Attachments would if embedded be preferably inline in most cases I think, the user should perhaps at least be given the option.

Actual differences
Assuming the issues with Thunderbird (and other clients) can be resolved, I think the only difference would be to users displaying the email as plain text or rich text. Here with multipart/related inline attachments should not display, whilst clients may display inline attachments following the message. I am not confident making these statements though whilst the behavior differs from client to client.

PS: I also noticed a missing newline in my code to be spec compliant, I will fix it later

[benedikt-schaber]

Sorry for my inactivity on this matter,
I should finally have some time for some open source work again. If you wish I could resume work on this issue.
However, with nodemailer being available on deno and the following uncertain future of this repository I currently do not think that introducing these features would be beneficial.
If you agree I would thus propose to close this pull request.