Unable to parse status groups in some cases

[AndrewGretton]

Hey there! It seems that Mimekit throws an exception when parsing NDR status groups for some real-world NDRs we're seeing.

Platform

• OS: Tested in Windows
• .NET Runtime: net6
• .NET Framework: not tested
• MimeKit Version: 3.4.1 (this is new code for us, so the issue may also exist in previous versions)

To Reproduce

The attached NDR is an anonymised real-world NDR we have received. Using the following code, Mimekit throws an exception:

using MimeKit;

using (var ms = new MemoryStream(Encoding.UTF8.GetBytes(File.ReadAllText("fail.txt")))) 
{
    var msg = MimeMessage.Load(ms);
    var deliveryStatus = msg.BodyParts.FirstOrDefault(p => p.ContentType.MimeType == "message/delivery-status") as MessageDeliveryStatus;
    if (deliveryStatus?.StatusGroups != null) // exception is thrown here
    {
        //
    }
}

It throws a FormatException, "failed to parse headers", at the following stack trace:

   at MimeKit.MimeParser.ParseHeaders(Byte* inbuf, CancellationToken cancellationToken)
   at MimeKit.MessageDeliveryStatus.ParseStatusGroups()
   at MimeKit.MessageDeliveryStatus.get_StatusGroups()
   at async Program.<Initialize>(?) in :line 9

fail.txt

[jstedfast]

The problem is that the content of the message/delivery-status part isn't valid syntax (i.e. it does not contain status group headers)

This is the SFSMTP program.

I'm sorry to have to inform you that the attached e-mail could not be delivered to the following destinations:

[email protected]

Reason:
E-mail blocked (Sender IP address Not Accepted (1.2.3.4 listed on dnsbl.server))

[jstedfast]

An example (from rfc3464) of what it should look like:

content-type: message/delivery-status

Reporting-MTA: dns; sun2.nsfnet-relay.ac.uk

Final-Recipient: rfc822;[email protected]
Status: 4.0.0 (unknown temporary failure)
Action: delayed

In the above example, there are 2 status groups (i.e. 2 groups of headers) as the content of the message/delivery-status MIME part.

[jstedfast]

I think the solution is to catch any exceptions when accessing the MessageDeliveryStatus.StatusGroups property and if there is an exception, just default to displaying the content of the preceding text/plain part.

Unfortunately, that means that you can't actually process the message/delivery-status content programatically (unless you can implement some sort of free-form natural language parser for the text/plain content?). If you only ever deal with this particular SMTP server, then it might be possible to use regex to extract the recipient address and the "reason" text.

I think I have to close this as invalid or wontfix because I don't know what MimeKit could do in this situation that would be useful. It certainly can't parse the content as a list of status groups.

BTW, while we're here, I would recommend changing this line of code:

var deliveryStatus = msg.BodyParts.FirstOrDefault(p => p.ContentType.MimeType == "message/delivery-status") as MessageDeliveryStatus;

to:

var deliveryStatus = msg.BodyParts.OfType<MessageDeliveryStatus>().FirstOrDefault();

That's a bit simpler expression/easier to read and I think that will be faster (but feel free to verify before using).

[AndrewGretton]

Thanks for taking a look at this issue. I can see that wontfix / invalid is likely the rational outcome. Having said that, I wonder if there's any value in making the StatusGroups property return null instead of throwing an exception? From a developer ergonomics perspective it's perhaps a bit gentler, although I guess you could argue it's a breaking change for people who are currently expecting FormatException to be thrown.

FWIW, we see a a very large (7 figure) amount of NDRs per day, and we encounter malformed NDRs of this type ~100 times per day right now, from different MTAs. Try/catch is absolutely a simple change for us to make, of course. I just wanted to have the conversation here just in case I was missing something. Thanks again!

[jstedfast]

I would consider returning null and having MimeKit catch the exception itself as an acceptable solution. Many of the properties of various classes in MimeKit already return null if the value isn't available.

I think that, if I were to put myself in the shoes of a developer trying to write an app using MimeKit, I think I'd also expect (and probably prefer) null over an exception, so I'm definitely open to this.

[jstedfast]

Okay, so taking a look at MessageDeliveryStatus, it looks like the code catches ParseException instead of FormatException, likely by mistake (I originally planned to make the MimeParser throw ParseException instead of FormatException).

Secondly, if the Content is null, then the code creates an empty StatusGroups, so I think instead of returning null, I'll do the same when the content is invalid.

[jstedfast]

I guess it turns out that this was a legit "bug" after all. Catching ParseException was pointless, it really did need to catch FormatException.