Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline content with a filename parameter is not detected as an attachment #125

Closed
ChristopheB opened this issue Jul 30, 2015 · 2 comments
Closed

Comments

@ChristopheB
Copy link

An inline content with a filename parameter is currently not detected as an attachment.

As an example, a full body with a Content-Disposition: inline; filename="filename.ext":

This is a message in Mime Format.  If you see this, your mail reader does not support this format.

--=_dc003190f003830c0d438d07c275e942
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

--=_dc003190f003830c0d438d07c275e942
Content-Type: text/plain
Content-Transfer-Encoding: base64
Content-Disposition: inline; filename="test.txt"

VGhpcyBpcyBhIHRleHQgZm9yIHRlc3RpbmcgaW5saW5lIGF0dGFjaG1lbnRz
--=_dc003190f003830c0d438d07c275e942--

Every email client that I have tested against this displays a file "test.txt" as an attachment. Opening the email displays this text inline, decoded from base64: This is a text for testing inline attachments. This has been tested on Gmail, Outlook, Outlook webmail, and Roundcube.

I guess MailParser should also handle this.

So I wanted to add this particular case, but to tell you the truth, I had quite a hard time trying to figure out this part, and to guess every case (line 299):

// detect if this is an attachment or a text node (some agents use inline dispositions for text)
if (textContent && (!this._currentNode.meta.contentDisposition || this._currentNode.meta.contentDisposition == "inline")) {
    this._currentNode.attachment = false;
} else if ((!textContent || ["attachment", "inline"].indexOf(this._currentNode.meta.contentDisposition) >= 0) && !this._currentNode.meta.mimeMultipart) {
    this._currentNode.attachment = true;
}

From what I understand, it could be refactored to this:

this._currentNode.attachment = false;

if (!this._currentNode.meta.mimeMultipart && (!textContent || this._currentNode.meta.contentDisposition == "attachment")) {
    this._currentNode.attachment = true;
}

Or to be totally explicit, to this:

this._currentNode.attachment = false;

if (!this._currentNode.meta.mimeMultipart) {

    if (!textContent) {
        this._currentNode.attachment = true;
    }

    if (this._currentNode.meta.contentDisposition == "attachment") {
        this._currentNode.attachment = true;
    }
}

And now, if we want to handle our inline attachment, here we go:

this._currentNode.attachment = false;

if (!this._currentNode.meta.mimeMultipart) {

    if (!textContent) {
        this._currentNode.attachment = true;
    }

    if (this._currentNode.meta.contentDisposition == "attachment") {
        this._currentNode.attachment = true;
    }

    if (this._currentNode.meta.contentDisposition == "inline" && this._currentNode.meta.fileName) {
       this._currentNode.attachment = true;
    }
}

I'm not really sure this covers every weird case scenario, so I didn't make a pull request, but I can push one if it can help.

Thanks for your time :)

@jstedfast
Copy link

The logic used by most mail clients for determining if something is an attachment or not is far more complicated than just "does it have a filename parameter?"

Typically, what mail clients will do is render the message and any MIME parts left over (i.e. not referenced from an HTML body) are then shown as attachments.

However, this is very mail-client specific.

IMHO, a library should not treat anything as an attachment unless it explicitly has a Content-Disposition value of "attachment" (or anything other than "inline" if you want to be more accepting).

@andris9
Copy link
Member

andris9 commented Dec 17, 2015

Agree, this is mail client specific and inlined text/plain seems more like something that is not an attachment and should be rendered in place. As a comparison, OSX Mail App does not treat it as an attachment.

@andris9 andris9 closed this as completed Dec 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants