feat/encoders & feat/related-attachments

benedikt-schaber commented 1 year ago

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: logo

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 commented 1 year ago

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

benedikt-schaber commented 1 year ago

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 commented 1 year ago

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

mathe42 commented 1 year ago

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 commented 1 year ago

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: "a@a.de",
    html: '<p><b>Follow us on</b></p><p><a href=""><img src="cid:logo"></a></p>',
    subject: "aaa",
    from: "a@a.de",
    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 commented 1 year ago

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 commented 5 months ago

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.

EC-Nordbund / denomailer