muratgozel / MIMEText

RFC-2822, RFC-2045 and RFC-2049 compliant raw email message generator.
https://muratgozel.github.io/MIMEText/
MIT License
80 stars 35 forks source link

Adding attachments breaks it #32

Closed floschutz closed 1 year ago

floschutz commented 1 year ago

Hey, we have used this implementation for a while now for sending emails through AWS SES. It worked flawlessly until we tried adding attachments to our emails.

This is an example setup:

const mimeMessage = createMimeMessage();

mimeMessage.setSender({ name: 'Sender', addr: 'app@[...]' });
mimeMessage.setTo('rec@[...]');
mimeMessage.setSubject('no subject defined');

mimeMessage.setAttachment('test.txt', 'text/plain', Buffer.from('This is a test!').toString('base64'));

const mailRequest: SendRawEmailRequest = {
  RawMessage: {
    Data: mimeMessage.asRaw(),
  },
};

Without mimeMessage.setAttachment sending through SES works fine. But adding it results in an Invalid Base64 stream error from the AWS SDK.

The output of mimeMessage.asRaw() looks like this:

Date: Fri, 03 Feb 2023 09:10:14 +0000
From: [...] <[...]>
To: <[...]>
Message-ID: <leif09vb1nr-1675415414574@[...]>
Subject: [...]
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary=ndvh1dnwcae

--ndvh1dnwcae
Content-Type: multipart/alternative; boundary=lurrh6lqibe

--lurrh6lqibe
--ndvh1dnwcae
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: base64
Content-Disposition: attachment;filename="test.txt"

VGhpcyBpcyBhIHRlc3Qh

--lurrh6lqibe
Content-Type: text/html; charset=UTF-8

<strong>Test</strong>

--lurrh6lqibe--
--ndvh1dnwcae
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: base64
Content-Disposition: attachment;filename="test.txt"

VGhpcyBpcyBhIHRlc3Qh

--ndvh1dnwcae--

It seems like the attachment gets inserted two times. To be honest I lack the proper understanding of MIME to fully understand why the resulting output is not valid.

We already tried not transforming the attachment to Base64. We tried the included toBase64 method, but did not use it further since it is not included in the TypeScript declaration.

muratgozel commented 1 year ago

Hi @floschutz Could you try adding mimeMessage.setMessage('text/plain', 'some text.'); before mimeMessage.setAttachment ?

Alex-McLean commented 1 year ago

I have recently run into a seemingly similar issue, specifically with the attachment being included twice.

If I'm interpreting the implementation of this package correctly, I believe the issue is specific to attachments with the text/plain content type, as these attachments are then also treated as the plaintext message of the email.

In MIMEMessage.asRaw we have:

const plaintext = this.getMessageByType('text/plain')

And in MIMEMessage.getMessageByType we have

const matcher = (msg: MIMEMessageContent) => (msg.getHeader('Content-Type') as string || '').includes(type)

Finally, we can see in MIMEMessage.addAttachment that files are given a Content-Type header when added, which will include text/plain for .txt or similar attachments, and will therefore be returned by getMessageByType as plaintext in asRaw assuming there is no actual plaintext message added first.

muratgozel commented 1 year ago

Hi @Alex-McLean That might me a good find, allow me a few days to resolve this please.

muratgozel commented 1 year ago

Hi @floschutz @Alex-McLean I just run a couple of tests and these problems might be related your aws sdk implementation. The SDK accepts uint8array kind of data but your trying to send string. Have a look at the examples I created here in the readme please. Also, update the mimetext to the latest version.

Alex-McLean commented 1 year ago

Thanks for the update @muratgozel, but I'm not using AWS at all here, and the issue is reproducible entirely within the MIMEMessage class. I still think my initial diagnosis is correct, here's a MIMMEssage.spec.js test that reproduces it:

test('generates html message with a plain text attachment', () => {
    const samplePlainTextBase64 = 'ZXhhbXBsZSBwbGFpbiB0ZXh0'

    const msg = new MIMEMessage(envctx)
    msg.boundaries = {mixed: 'abcdef', alt: 'qwerty'}
    msg.setHeader('Date', 'Wed, 22 Mar 2023 23:36:33 +0000')
    msg.setHeader('Message-ID', '<oliusb0xvxc@mail.com>')
    msg.setSender('test@mail.com')
    msg.setSubject('Lorem Ipsum')
    msg.addMessage({contentType: 'text/html', data: 'hello there'})
    msg.addAttachment({
        contentType: 'text/plain',
        filename: 'sample.txt',
        data: samplePlainTextBase64
    })

    expect(msg.asRaw()).toBe('Date: Wed, 22 Mar 2023 23:36:33 +0000' + envctx.eol +
      'From: <test@mail.com>' + envctx.eol +
      'Message-ID: <oliusb0xvxc@mail.com>' + envctx.eol +
      'Subject: =?utf-8?B?TG9yZW0gSXBzdW0=?=' + envctx.eol +
      'MIME-Version: 1.0' + envctx.eol +
      'Content-Type: multipart/mixed; boundary=abcdef' + envctx.eol + envctx.eol +
      '--abcdef' + envctx.eol +
      'Content-Type: text/html; charset=UTF-8' + envctx.eol +
      'Content-Transfer-Encoding: 7bit' + envctx.eol + envctx.eol +
      'hello there' + envctx.eol + envctx.eol +
      '--abcdef' + envctx.eol +
      'Content-Type: text/plain; name="sample.txt"' + envctx.eol +
      'Content-Transfer-Encoding: base64' + envctx.eol +
      'Content-Disposition: attachment; filename="sample.txt"' + envctx.eol + envctx.eol +
      samplePlainTextBase64 + envctx.eol +
      '--abcdef--'
    )
})

This test will fail unless we add attachment checks to getMessageByType:

getMessageByType(type: string): MIMEMessageContent | undefined {
    const matcher = (msg: MIMEMessageContent) => (msg.getHeader('Content-Type') as string || '').includes(type) && !(msg.isAttachment() || msg.isInlineAttachment())
    return this.messages.some(matcher) ? this.messages.filter(matcher)[0] : undefined
}

I did try and make this into a PR, but was unsure what I was supposed to be doing with the build and the docs and such, seeing as they are all committed to the repo.

aavilla-riparian commented 1 year ago

Getting this same error and interested in what comes from the discussion.

aavilla-riparian commented 1 year ago

Any word / traction on this?

muratgozel commented 1 year ago

Hi @Alex-McLean @aavilla-riparian I have added and run three more tests:

  1. html email with plain text attachment
  2. plain text attachment only
  3. plain text content with plain text attachment

All were successful, The messages were in messages and attachments were in attachments section. Could you please provide more code samples so I can test this?

Please don't forget to update the lib to the latest version. It's v3.0.13.

Alex-McLean commented 1 year ago

I believe if you swap the order of addMessage and addAttachment in the plain text email with a plain text attachment then the test will fail. Seemingly due to this code in MIMEMessage.ts which just gets the first text/plain message when searching: this.messages.filter(matcher)[0]

aavilla-riparian commented 1 year ago

@muratgozel how did testing go based off of @Alex-McLean's feedback?

aavilla-riparian commented 1 year ago

FWIW, I monkey patched my codebase to use @Alex-McLean's snippet and it fixed my issue. Anything we can do to help push this along?

muratgozel commented 1 year ago

Hi @Alex-McLean @aavilla-riparian @floschutz Just fixed this, sorry for late response, hope everything goes well on your side. Also added a contribution guideline so you can create pull requests with confidence.