Framework: Mailgun API error with dirty data and cc/bcc

[sebsel]

• Laravel Version: 5.5.4 (but reproduced current 5.7)
• PHP Version: 7.1.20
• Database Driver & Version: [unrelated]

Description:

In our project, we got some dirty data, and the Mailgun API does not accept all our emails. Turns out this only becomes a problem when adding cc or bcc's to the mail.

I've identified two problems: one with an opening ( that is not closed, and one with an e-mail in the name field. Once you add one of those names to the 'to' field, and add a cc or bcc, you will get an exception:

GuzzleHttp\Exception\ClientException with message 'Client error: `POST https://api.mailgun.net/v3/[DOMAIN]/messages.mime` resulted in a `400 BAD REQUEST` response:
{
  "message": "'to' parameter is not a valid address. please check documentation"
}
'

I have opened a ticket at Mailgun to ask their opinion, but my guess it that they would suggest adding "" around the name. (See below)

Steps To Reproduce:

In a tinker-session with Mailgun setup, one could run the following lines:

$email1 = "[email protected]";
$email2 = "[email protected]";

// Example with opening an ( and not closing it
Mail::send('welcome', [], function($m) use ($email1, $email2) { $m->to($email1, 'something ( end'); $m->bcc($email2); });

// Example with email inside of name
Mail::send('welcome', [], function($m) use ($email1, $email2) { $m->to($email1, '[email protected] Real Name'); $m->bcc($email2); });

Possible Solutions:

I think the issue is located here:

    // Illuminate/Mail/Transport/MailgunTransport
    protected function getTo(Swift_Mime_SimpleMessage $message)
    {
        return collect($this->allContacts($message))->map(function ($display, $address) {
            return $display ? $display." <{$address}>" : $address;
        })->values()->implode(',');
    }

As you can see, this function does not add quotes around the name, but concats all the addresses together. The examples of the steps to reproduce result in the following strings, which are not accepted by the API:

• something ( end <[email protected]>,[email protected]
• [email protected] Real Name <[email protected]>,[email protected]

I think it would be best to add quotes:

• "something ( end" <[email protected]>,[email protected]
• "[email protected] Real Name" <[email protected]>,[email protected]

Also interesting to note that this issue only seems to arise when adding more recipients to the mail. Just a single user is fine, which makes this kind of an edge case.

[sebsel]

Hm, I now see that there are no quotes mentioned in this document: https://tools.ietf.org/html/rfc2822#section-3.4 It should be fine without them.

Let's just wait for Mailgun to come back at this.

[driesvints]

Both examples can be prevented by proper validation in the application itself. Just provide the correct input?

[sebsel]

I agree that the data should be cleaner, though, in this particular application, we do not own that data. This is the correct input.

Apart from this particular application, I don't think the above values should result in an API failure. But I'm also asking Mailgun about it, because it's their API that's rejecting it. I'll update when I have a stronger case :)

[ahmed-alaa]

@sebsel Hi, did you get any updates from Mailgun regarding this case?

[sebsel]

I did, but unfortunately, I switched jobs, so I don't have access to my e-mail history.

Of the top of my head, the solution as presented by Mailgun was to define the to field multiple times in the request (instead of relying on commas).

See the example taken from their API docs:

curl -s --user 'api:YOUR_API_KEY' \
    https://api.mailgun.net/v3/YOUR_DOMAIN_NAME/messages \
    -F from='Excited User <mailgun@YOUR_DOMAIN_NAME>' \
    -F to=YOU@YOUR_DOMAIN_NAME \
    -F [email protected] \
    -F subject='Hello' \
    -F text='Testing some Mailgun awesomeness!'

They showed me output and examples of tests with something ( end <[email protected]>,[email protected] and [email protected] Real Name <[email protected]>,[email protected] as input, and it seemed that this was the solution. Unfortunately, I never came around to update here, or to fix our app.

Thanks for the reminder :)

@driesvints Is this enough info to consider a reopening? I don't feel like it's Laravel's job to catch up with Mailguns API, but since the driver is in the core of the framework, you kind of have to in a way.

[ahmed-alaa]

@sebsel Thanks for sharing the info.

The solution that applied to our problem was putting the name with "" to handle such an issue, but of course it was more related to the driver of MailgunTransport

Illuminate/Mail/Transport/MailgunTransport.php // MailgunTransport->getTo()