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

Malformed Content-Type header #102

Closed
ivanghisleni opened this issue Dec 31, 2019 · 8 comments
Closed

Malformed Content-Type header #102

ivanghisleni opened this issue Dec 31, 2019 · 8 comments
Labels

Comments

@ivanghisleni
Copy link

Hi,

We found some emails with malformed Content-Type header that generate crash:

In PartBuilder.php line 228:

   [Symfony\Component\Debug\Exception\UndefinedMethodException]
   Attempted to call an undefined method named "getValueFor" of class "ZBateso
   n\MailMimeParser\Header\GenericHeader".
   Did you mean to call "getValue"?


 Exception trace:
   at /var/www/esva/backend/vendor/zbateson/mail-mime-parser/src/Message/Part/PartBuilder.php:228
  ZBateson\MailMimeParser\Message\Part\PartBuilder->getMimeBoundary() at /var/www/esva/backend/vendor/zbateson/mail-mime-parser/src/Message/Part/PartBuilder.php:265
  ZBateson\MailMimeParser\Message\Part\PartBuilder->setEndBoundaryFound() at /var/www/esva/backend/vendor/zbateson/mail-mime-parser/src/Message/MessageParser.php:130
  ZBateson\MailMimeParser\Message\MessageParser->findContentBoundary() at /var/www/esva/backend/vendor/zbateson/mail-mime-parser/src/Message/MessageParser.php:187
  ZBateson\MailMimeParser\Message\MessageParser->readPartContent() at /var/www/esva/backend/vendor/zbateson/mail-mime-parser/src/Message/MessageParser.php:219
  ZBateson\MailMimeParser\Message\MessageParser->readPart() at /var/www/esva/backend/vendor/zbateson/mail-mime-parser/src/Message/MessageParser.php:238
  ZBateson\MailMimeParser\Message\MessageParser->read() at /var/www/esva/backend/vendor/zbateson/mail-mime-parser/src/Message/MessageParser.php:65
  ZBateson\MailMimeParser\Message\MessageParser->parse() at /var/www/esva/backend/vendor/zbateson/mail-mime-parser/src/MailMimeParser.php:70
  ZBateson\MailMimeParser\MailMimeParser->parse() at /var/www/esva/backend/src/LibraEsva/Archiver/Mail.php:193

Here the sample of portion of the EML that cause the issue:

="Content-Type: multipart/mixed;Boundary="USER_UNKNOWN"

The fix could be easy adding an extra check on contentType into getMimeBoundary():

/**
     * Returns the parsed boundary parameter of the Content-Type header if set
     * for a multipart message part.
     * 
     * @return string
     */
    public function getMimeBoundary()
    {
        if ($this->mimeBoundary === false) {
            $this->mimeBoundary = null;
            $contentType = $this->getContentType();
            if ($contentType !== null && $contentType instanceof ParameterHeader) {
                $this->mimeBoundary = $contentType->getValueFor('boundary');
            }
        }
        return $this->mimeBoundary;
    }

Does it make sense to create a pull request for fixing it?

@travisaustin
Copy link

I'm seeing this, as well.

@zbateson zbateson added the bug label Jan 2, 2020
@zbateson
Copy link
Owner

zbateson commented Jan 2, 2020

Interesting, thanks for reporting.

It looks to me like the bug is that somehow HeaderFactory isn't returning a ParameterHeader for the Content-Type header, and that should be addressed rather than checking if the returned value is a ParameterHeader (the type should be a guarantee).

I'm confused about what's causing that in your example. Do you mean it's exactly like you pasted it with the leading =" or what's wrong with the header in your example that you think is causing this?

@travisaustin
Copy link

travisaustin commented Jan 2, 2020

For me, the issue is occurring for a message that has a header called "contenttype".

I recognize that the contenttype header is not valid or correct, but I'd prefer the header be ignored instead of causing a fatal PHP error. :-)

Here's a complete message that causes this issue:

From: Test <test@test.com>
To: Test <test@test.com>
Subject: Test Message
Date: Thu, 2 Jan 2020 14:31:21 +0000
Message-ID: <test@test.com>
contenttype: text/html
Content-Type: multipart/alternative;
	boundary="_123_boundary_"
MIME-Version: 1.0

--_123_boundary_
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable

Plaintext version

--_123_boundary_
Content-Type: text/html; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable

<html>
<head>
</head>
<body>
<p>HTML version</p>
</body>
</html>

--_123_boundary_--

@ivanghisleni
Copy link
Author

ivanghisleni commented Jan 4, 2020 via email

@zbateson
Copy link
Owner

zbateson commented Jan 6, 2020

Thanks @travisaustin and @ivanghisleni -- both of those would be an issue but for slightly different reasons.

Unfortunately @travisaustin -- yours is a little more complicated. The reason is, in trying to make something like what's happening with @ivanghisleni work, I ended up treating both a header with a name like "contenttype" to be equal to one with a name like "content-type", and the fix for that will be a little more complicated.

I'll try get something out for both if possible, I'll need to investigate a bit more, but certainly the @ivanghisleni issue can be addressed more quickly.

All the best.

zbateson added a commit that referenced this issue Jan 9, 2020
- Only use normalized header names when exact matches aren't
  available
- Use normalized header naming both when determining the class to
  use for a header, and for finding headers
@zbateson
Copy link
Owner

zbateson commented Jan 9, 2020

Fix for both released in 1.2.0

@zbateson zbateson closed this as completed Jan 9, 2020
@ivanghisleni
Copy link
Author

ivanghisleni commented Jan 9, 2020 via email

@travisaustin
Copy link

@zbateson - Thank you for the fast turnaround for this!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants