Fix e-mail URI escaping recipients

[Cheesebaron]

Description of Change

Escaping the recipient e-mail addresses, leads to the character @ being escaped and creates an invalid URL.

So [email protected] becomes john%40myhost.com which isn't containing a valid e-mail.

On iOS this fails when translating the URL to NSUrl.

Issues Fixed

Fixes #12010

[ghost]

Hey there @Cheesebaron! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[Cheesebaron]

Hold your horses, just reading RFC 2368, there are even more issues with this method for creating the URI. I guess no one ever tested it?

Everything is delimited using ? this is wrong. Only the first parameter should be formatted like that.

[Cheesebaron]

I've changed the implementation, with inspiration from https://codereview.stackexchange.com/a/129600, to properly generate a RFC 2368 mailto URL.

So previously there were several issues.

• To, CC, BCC where all URL escaped, which is not correct if they are not part of a parameter, so only if you use to=someone%40somewhere.net you can escape, but if using raw mailto:someone%40somewhere.net it won't work
• The To recipients were delimited incorrectly for the format chosen

This has now been corrected and proper URLs are emitted. Perhaps someone should add some implementation to the e-mail tests ;)

[Cheesebaron]

Also find it a bit weird why MailMessage is not used instead of rolling your own? Any particular reason for this?

[rmarinho]

Maybe because this code was legacy ? Coming from Essentials?

Any thoughts @mattleibow ?

[jfversluis]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 2 pipeline(s).

[jfversluis]

@Cheesebaron just an FYI we didn't forget about this one! Matt had some off time and trying to figure out the history here if there was a good reason for it. Otherwise I think this is a great improvement. Thank you for that! I'll make sure we get to it shortly :)

[Cheesebaron]

@Cheesebaron just an FYI we didn't forget about this one! Matt had some off time and trying to figure out the history here if there was a good reason for it. Otherwise I think this is a great improvement. Thank you for that! I'll make sure we get to it shortly :)

Yep no worries. I have a workaround in my App for this which works, so I am not in a rush to get this in.

[mattleibow]

The lack of MailMessage may have been a result of UWP... But also, does that support the construct of parameters to a uri? And then it may also pull in related code making the linker less effective.

But, this code appears to only be used on iOS, so if this looks good if it works.

[mattleibow]

Also:

Perhaps someone should add some implementation to the e-mail tests

Thanks for starting the work on the tests! ;)

[Cheesebaron]

Thanks for starting the work on the tests! ;)

🙈

I was having trouble even building the solution, but got that to work yesterday. I can have a look at adding some of those. There are no tests at all for any of this e-mail stuff to begin with 😂

[azure-pipelines[bot]]

Azure Pipelines successfully started running 2 pipeline(s).

[mattleibow]

Amazing!

There are no tests at all for any of this e-mail stuff

No idea what you mean! 😉 All I see are wonderful tests in this PR 😉 Perfect example of a PR and tests to go along with it!

Thanks a big bunch!

[BioTurboNick]

Is there a way to work around this until the fix is released? I now realize that I've been seeing this crash in AppCenter because people needing help weren't able to contact me...

[Cheesebaron]

Yeah, you can do anything you want in your code. To work around this I made a small service like so:

public class IosEmailService : IEmailService
{
    public Task SendMessage(EmailMessage message)
    {
        if (MFMailComposeViewController.CanSendMail)
            return Email.ComposeAsync(message);

        return ComposeWithUrl(message);
    }

    private static Task ComposeWithUrl(EmailMessage message)
    {
        var url = GetMailToUri(message);
        var nsurl = NSUrl.FromString(url);
        return Launcher.Default.OpenAsync(nsurl!);
    }

    private static string GetMailToUri(EmailMessage message) =>
        "mailto:?" + string.Join("&", Parameters(message));

    private static IEnumerable<string> Parameters(EmailMessage message)
    {
        if (message.To?.Any() == true)
            yield return "to=" + Recipients(message.To);

        if (message.Cc?.Any() == true)
            yield return "cc=" + Recipients(message.Cc);

        if (message.Bcc?.Any() == true)
            yield return "bcc=" + Recipients(message.Bcc);

        if (!string.IsNullOrWhiteSpace(message.Subject))
            yield return "subject=" + Uri.EscapeDataString(message.Subject);

        if (!string.IsNullOrWhiteSpace(message.Body))
            yield return "body=" + Uri.EscapeDataString(message.Body);
    }

    private static string Recipients(IEnumerable<string> addresses) =>
        string.Join(",", addresses.Select(Uri.EscapeDataString));
}

Calling SendMessage on my own service instead of calling MAUI Essentials directly.

[ederbond]

Hi @Cheesebaron , could you show us what's on your Email.ComposeAsync(message) on line 6 of your code above? Who is that Emailclass?

[Cheesebaron]

Hi @Cheesebaron , could you show us what's on your Email.ComposeAsync(message) on line 6 of your code above? Who is that Emailclass?

That is just MAUI Essentials

[ederbond]

Backport it to .NET 7.

[hartez]

/backport to net7.0

[github-actions[bot]]

Started backporting to net7.0: https://github.com/dotnet/maui/actions/runs/5070650052