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

Double period issue with SES's SendRawEmail and dot-stuffing #501

Closed
robertmiles3 opened this Issue Dec 2, 2016 · 7 comments

Comments

Projects
None yet
6 participants
@robertmiles3

robertmiles3 commented Dec 2, 2016

To send an email through the SES SDK with an attachment, you must use the SendRawEmail function (can we please make this more developer-friendly in a future version??). In order to do so, you either hand-code a MIME message or rather convert a System.Net.Mail.MailMessage object to a MemoryStream. This has all been working fine, but I've encountered an issue. After much digging, I've been able to find the problem and replicate it, and it seems to be a by-product of SMTP dot-stuffing.

The issue is that in certain scenarios, when the MailMessage is converted to a raw MIME message, if a period in the message body is wrapped to the beginning of a line in the raw message then it is dot-stuffed (properly I assume). However, it doesn't seem to be handled either inside the SDK or on the SES side of things because the email comes through with double periods. This can be replicated via the following example console app code...

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Net.Mail;
using System.Reflection;
using System.Text;
using Amazon;
using Amazon.Runtime;
using Amazon.SimpleEmail;
using Amazon.SimpleEmail.Model;
using MimeKit;

namespace SESMimeIssue
{
    class Program
    {
        const string _SESAccessKey = "foo";
        const string _SESSecretKey = "bar";
        const string _fromAddress = "noreply@bar.com";
        const string _toAddress = "foo@bar.com";

        static void Main(string[] args)
        {
            var to = new List<string> { _toAddress };
            var subject = "TEST MESSAGE";
            var message = $"This is a carefully crafted HTML email message body such that you should see a single period right here ->. However, you'll see <strong>two periods</strong> in the email instead of one period like originally given in the code.";

            var body = $"<br />Hello,<br /><br />{message}<br /><br />Sincerely,<br /><br />Your Tester";

            var attachmentPath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Test.txt");
            var result = SendEmail(to, subject, body, new List<string> { attachmentPath }, isHtml: true);
            Console.WriteLine(result ? "Successfully sent message" : "Failed to send message");

            if (Debugger.IsAttached)
            {
                Console.WriteLine("Press any key to continue...");
                Console.ReadLine();
            }
        }

        private static bool SendEmail(List<string> to, string subject, string body, List<string> attachmentFilePaths = null, bool isHtml = false)
        {
            var message = new MailMessage
            {
                From = new MailAddress(_fromAddress),
                Subject = subject,
                Body = body,
                IsBodyHtml = isHtml
            };

            foreach (var address in to)
            {
                message.To.Add(address.Trim());
            }

            if (attachmentFilePaths?.Any() == true)
            {
                foreach (var filePath in attachmentFilePaths)
                {
                    message.Attachments.Add(new Attachment(filePath));
                }
            }

            try
            {
                var creds = new BasicAWSCredentials(_SESAccessKey, _SESSecretKey);
                using (var client = new AmazonSimpleEmailServiceClient(creds, RegionEndpoint.USEast1))
                {
                    // Using this DOESN'T work as it doubles the period
                    var request = new SendRawEmailRequest { RawMessage = new RawMessage { Data = ConvertMailMessageToMemoryStream(message) } };
                    // Using this DOES work
                    //var request = new SendRawEmailRequest { RawMessage = new RawMessage { Data = ConvertMailMessageToMemoryStreamViaMimeKit(message) } };
                    Console.WriteLine($"RawMessage.Data...\r\n\r\n{Encoding.ASCII.GetString(request.RawMessage.Data.ToArray())}");

                    client.SendRawEmail(request);
                    return true;
                }
            }
            catch (Exception ex)
            {
                Console.WriteLine($"AmazonSESHelper.SendEmail => Exception: {ex.Message}");
            }

            return false;
        }

        // From http://stackoverflow.com/questions/29532152/create-mime-mail-with-attachment-for-aws-ses-c-sharp/29533336#29533336
        private static MemoryStream ConvertMailMessageToMemoryStream(MailMessage message)
        {
            var stream = new MemoryStream();
            var assembly = typeof(SmtpClient).Assembly;
            var mailWriterType = assembly.GetType("System.Net.Mail.MailWriter");

            var mailWriterConstructor = mailWriterType.GetConstructor(BindingFlags.Instance | BindingFlags.NonPublic, null, new[] { typeof(Stream) }, null);
            var mailWriter = mailWriterConstructor.Invoke(new object[] { stream });

            var sendMethod = typeof(MailMessage).GetMethod("Send", BindingFlags.Instance | BindingFlags.NonPublic);
            sendMethod.Invoke(message, BindingFlags.Instance | BindingFlags.NonPublic, null, new[] { mailWriter, true, true }, null);

            var closeMethod = mailWriter.GetType().GetMethod("Close", BindingFlags.Instance | BindingFlags.NonPublic);
            closeMethod.Invoke(mailWriter, BindingFlags.Instance | BindingFlags.NonPublic, null, new object[] { }, null);

            return stream;
        }

        private static MemoryStream ConvertMailMessageToMemoryStreamViaMimeKit(MailMessage message)
        {
            var stream = new MemoryStream();
            var mimeMessage = MimeMessage.CreateFromMailMessage(message);
            mimeMessage.WriteTo(stream);
            return stream;
        }
    }
}

The raw MIME message that gets outputted is...

X-Sender: noreply@bar.com
X-Receiver: foo@bar.com
MIME-Version: 1.0
From: noreply@bar.com
To: foo@bar.com
Date: 2 Dec 2016 11:45:36 -0500
Subject: TEST MESSAGE
Content-Type: text/html; charset=us-ascii
Content-Transfer-Encoding: quoted-printable

<br />Hello,<br /><br />This is a carefully crafted HTML email me=
ssage body such that you should see a single period right here ->=
.. However, you'll see <strong>two periods</strong> in the email i=
nstead of one period like originally given in the code.<br /><br =
/>Sincerely,<br /><br />Your Tester

You can see the dot-stuffing (again, assuming that's proper per SMTP), but it doesn't get un-done after the send as can be seen when I receive the email...

duy9e

I can get this to work if I add in @jstedfast's MimeKit (which is great) but that adds another dependency in all our apps, which is not my favorite thing to do.

I'm not sure whose problem this actually is...is it the SDK? is it on the SES receiving side? is the System.Net.Mail.MailWriter not supposed to dot-stuff?

@jstedfast

This comment has been minimized.

jstedfast commented Dec 2, 2016

The problem is that System.Net.Mail.MailWriter expects to send the message over SMTP which requires byte-stuffing the message as it gets serialized for transport over the TCP socket.

MIME messages saved to disk should not have the byte-stuffing.

There's 2 ways that SES can deal with this / make this easier:

  1. Add a bool property on RawMessage that specifies that the message data has already been byte-stuffed.
  2. Add a RawMessage or SendRawEmail() API that takes a System.Net.Mail.MailMessage object so that it can handle this more easily for developers using System.Net.Mail to construct their messages.

I'm not sure which would be easier for them to do.

@robertmiles3

This comment has been minimized.

robertmiles3 commented Dec 5, 2016

Agreed @jstedfast. Thanks for taking the time to give some input. As for your suggestions...

  1. Add a bool property on RawMessage that specifies that the message data has already been byte-stuffed.

I think this one would just confuse developers would have no idea what byte-stuffing/dot-stuffing is (I didn't before this issue). In addition, it would force developers who do know to remember to set this.

  1. Add a RawMessage or SendRawEmail() API that takes a System.Net.Mail.MailMessage object so that it can handle this more easily for developers using System.Net.Mail to construct their messages.

I think this is the way to go and really wish this is what would have happened in the first place. Here was my initial developer experience with SES:

  1. I need to send an email with an attachment. Oh, I can't do that with the normal SendEmail and have to use SendRawEmail. Ok then.
  2. [Googles]...hmm, looks like SendRawEmail needs a MemoryStream with a raw MIME message that I have no clue how to concoct. Looks like my options are:
    a. I can add in a library like MimeKit to convert a MailMessage to a MemoryStream. Works great, but adds a dependency to all our apps that email. Not cool.
    b. I can add all this reflection jazz that some SO user posted. Seems to work, let's roll with that.

Of course, only come to find later this bug, which would be near impossible to know up front given the mess with reflection and the fact that it will only happen when a period lands on a wrapping break.

All that to say, if SendRawEmail had something like this, it would be very natural for a C# developer who is used to a MailMessage.

@robertmiles3

This comment has been minimized.

robertmiles3 commented Dec 6, 2016

@sstevenkang I see the label, but do you have any thoughts or ETA on this? On my end, I have to move on this fairly quickly to rectify this issue and decide to either add in MimeKit or abandon SES.

@PavelSafronov

This comment has been minimized.

Contributor

PavelSafronov commented Dec 6, 2016

It's very unlikely that this will be updated on the service side quickly, if at all. The approach I would recommend at this time is to add a dependency on MimeKit.

@robertmiles3

This comment has been minimized.

robertmiles3 commented Mar 7, 2017

@normj Does the closing of this (and the "Service Issue" tag) mean that this was fixed on the service side of things?

@normj

This comment has been minimized.

Member

normj commented Mar 7, 2017

Sorry I should have added a comment when I closed the issue. I was just trying to clear up issues that the SDK team can handle. Service issues are best handled through the service forums or AWS support.

@ThomasArdal

This comment has been minimized.

ThomasArdal commented Dec 7, 2018

Just had to send an email with attachment using aws-sdk-net and found this thread. I totally agree that there should be a better way of doing this through the client. I'm using templated emails everywhere, why having to use the raw methods when needing to send attachments, seems odd. I know that the client is mostly auto-generated, but overloads for SendEmail etc. could be written. Behind the cover, these would simply use the raw methods, hiding what goes on.

Don't expect anything, just wanted to add my thoughts 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment