andrewdavey / postal

Email sending for asp.net mvc using the view engine system to render emails.
http://aboutcode.net/postal
MIT License
536 stars 168 forks source link

An invalid character was found in the mail header: ';'. #158

Open trailmax opened 8 years ago

trailmax commented 8 years ago

I've discovered an interesting bug. Today in production logs I've found the following exception:

System.FormatException: An invalid character was found in the mail header: ';'. at System.Net.Mail.MailAddressParser.ParseLocalPart(String data, Int32& index, Boolean expectAngleBracket, Boolean expectMultipleAddresses) at System.Net.Mail.MailAddressParser.ParseAddress(String data, Boolean expectMultipleAddresses, Int32& index) at System.Net.Mail.MailAddressParser.ParseMultipleAddresses(String data) at System.Net.Mail.MailAddressCollection.ParseValue(String addresses)

I've tracked the issue to an email with an apostrophe. Something like `Mr.O'Neil@example.com' when using a strongly typed email:

public class TestEmail : Postal.Email
{
    public string To { get; set; }
    public int Name { get; set; }
}

@model TestEmail
To: @Model.Name <@Model.To>
Subject: Fail Test

Hello world

var email = new TestEmail(){
    To = "Mr.O'Neil@example.com",
    Name = "Mr O'Neil",
}
email.Send(); // this will explode with the exception

The problem tracks down to RazorEngine doing encoding of all the input. So Mr.O'Neil@example.com is getting translated into Mr.O&#39;Neil@example.com. You see where the problem comes from? When trying to add this value to MailMessage.To - the above exception is thrown.

Now I've attempted to replace @Model.To with @Html.Raw(Model.To), but Razor Engine fails with message "HtmlHelper is not available".

Solution I've found is to use RawString in cshtml template:

To: @(new RawString(Model.To))

But this adds mental overhead and can easily be forgotten when adding new templates. Any ideas how to improve?

dudeinthemoon42 commented 8 years ago

Hey! Thanks for posting this bug. I actually ran into the same problem today, with the exact same input scenario. I had a series of email strings from a database query coming through and being sent, and the app hit this exception complaining about the input string. The input that triggered the exception was something like O'Connor.Kristen@example.com, and I was immediately suspicious of the apostrophe. I searched and found your post, and now it makes sense. RazorEngine is definitely trying to encode the apostrophe / HTML-escape it.

I ended up trying both @Html.aw(Model.To.Email) and your work around, @(new RawString(Model.To)). The first one did not compile (complained about an assembly reference), and the second didn't work initially either. I ended up finding where RawString is implemented (it implements IEncodedString in RazorEngine.Text), and then added @using RazorEngine.Text in order to get the context for RawString. IntelliSense was then able to highlight it and recognize it. I reran the input query and the emails went through.

I've gone ahead and started replacing all of my views with this RawString workaround. I can't say that I have any ideas on how to improve on this solution, but I just wanted to thank you for posting and give a heads-up that users might also need to include the using statement I wrote above to get it to work.

trailmax commented 8 years ago

I suspect there is no easy work-around for this issue. I sense the fundamental problem here - email headers should not be a part of Razor template at all, but this is a paradigm shift for this project and unlikely it'll be changed.

andrewdavey commented 8 years ago

When using RazorEngine the Html helper is not available. You can use @Raw(Model.SomeProp) instead.

trailmax commented 8 years ago

We are using RazorGenerator.MsBuild - it compiles all the *.cshtml files and @Raw() trips the compile process, so that is a no-go unfortunately. RawString works just the same. The problem is having to remember to do it in every email template for every header that might have special characters. But I don't think there will be a work-around to that, other than some integration test that will go through every email template and verify that RawString is applied.

petrosmm commented 7 years ago

Just an FYI for anyone else still looking. @(new RawString(Model.To)) did not work. I ended up using @Html.Raw(Model.To).

cavprime commented 7 years ago

For any fellow sufferers for whom the Raw and RawString didn't work: replace your semicolons with commas.

pcbbc commented 6 years ago

Just encountered this issue effecting the subject line. Isn't it easier to decode the HLML entites back into plain text in Postal's EmailParser? Can anyone see any issue with the following:

        void AssignEmailHeaderToMailMessage(string key, string value, MailMessage message)
        {
            value = HttpUtility.HtmlDecode(value);

Expect a similar fix is required for the text body, if a plain-text view is specified. Haven't found where to fix that yet...

Edit - I think this should do it:

        AlternateView CreateAlternativeView(Email email, string alternativeViewName)
        {
            ...
            if (contentType.StartsWith("text/plain", StringComparison.OrdinalIgnoreCase))
            {
                body = WebUtility.HtmlDecode(body);
            }

            var stream = CreateStreamOfBody(body);

And for messages with only a text body and no AlternateViews:

        void InitializeMailMessage(MailMessage message, string emailViewOutput, Email email)
        {
            ...
                    else
                    {
                        message.IsBodyHtml = messageBody.StartsWith("<");
                        if (!message.IsBodyHtml)
                        {
                            messageBody = HttpUtility.HtmlDecode(messageBody);
                        }
                        message.Body = messageBody;
                    }
Jhonust commented 9 months ago

Thanks for sharing! Identifying and addressing bugs is crucial for maintaining system reliability. Have you tried reviewing recent changes that might have caused this issue? Working apartments collaboratively to debug can lead to a swift resolution.