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

No way to set the `Content-Disposition` header on an attachment to `inline` #31

Closed LadyAelita closed 1 year ago

LadyAelita commented 1 year ago

Sometimes, one wants to have an attachment with Content-Disposition set to inline, so that the files don't show up in the "Attachments" view in the e-mail client. Unfortunately, it's currently hard-coded to only ever be attachment for attachments. The moreHeaders optional param of .setAttachment() doesn't help here, because the Content-Disposition header is overriden anyways.

The only way to do it at the moment is as follows:

msg.setAttachment('something.jpg', 'image/jpeg', contentBase64, { 'Content-ID': 'something.jpg' );
const attachment = msg.getAttachments().at(-1);
attachment.setHeader('Content-Disposition', 'inline;name=something.jpg');
attachment.isAttachment = () => true;

That's pretty unwieldy, especially when using TypeScript. Is there a chance such functionality could be added?

muratgozel commented 1 year ago

Hi @Unelith This is a completely valid issue, thanks for bringing this up, I will have a look at this shortly.

nbouvrette commented 1 year ago

I think the following fix could be simple. Changing the code here: https://github.com/muratgozel/MIMEText/blob/5741465e8594cea8ac5c1287001fc45f344f90f5/src/MIMEMessage.js#L119

From:

const headers = Object.assign({}, moreHeaders, {
      'Content-Type': `${type}; charset=UTF-8`,
      'Content-Transfer-Encoding': 'base64',
      'Content-Disposition': `attachment;filename="${filename}"`
    })

To:

const headers = Object.assign({}, {
      'Content-Type': `${type}; charset=UTF-8`,
      'Content-Transfer-Encoding': 'base64',
      'Content-Disposition': `attachment;filename="${filename}"`
    }, moreHeaders)

This way you can overwrite headers if you want to and you know what you are doing.

I'm willing to do a PR if this helps as this is blocking some tests I am doing right now.

muratgozel commented 1 year ago

Hi @nbouvrette this approach looks good, send a PR please 🙏

nbouvrette commented 1 year ago

Sorry I ended up completely changing my approach and no longer use the library so I'll have to pass on the PR at this time.

muratgozel commented 1 year ago

Closing this issue as of the later versions of this library already supports this.