System.Net.Mail.MailAddress.ToString() issue with embedded double quotes in DisplayName

[J0nKn1ght]

Description

If a MailAddress instance is created with a DisplayName that includes embedded double quote characters, the ToString method doesn't properly escape the embedded double quotes.

The ToString method in the repository is currently:

        /// <summary>
        /// this returns the full address with quoted display name.
        /// i.e. "some email address display name" &lt;user@host&gt;
        /// if displayname is not provided then this returns only user@host (no angle brackets)
        /// </summary>
        public override string ToString()
        {
            if (string.IsNullOrEmpty(DisplayName))
            {
                return Address;
            }
            else
            {
                return "\"" + DisplayName + "\" " + SmtpAddress;
            }
        }

link to above source

If the DisplayName property includes double quotes, then these are not escaped properly before surrounding the string with additional double quotes.
For example, if the display name is 'Henry "The Fonz" Winkler', when the ToString method is called, the resulting string would be:
\"Henry \"The Fonz\" Winkler\" <someemail@somedomain.com>
but should be:
\"Henry \\\"The Fonz\\\" Winkler\" <someemail@somedomain.com>

Configuration

Tested on .Net Core v2.2, but the source in this repository looks like it is still an issue in the latest version.

Regression?

Not known if this worked correctly on previous versions of .Net Framework / .Net Core

Other information

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

[scalablecory]

ToString() is not intended to return a serialized string, merely one that is human-readable. I don't know that I would want to escape the inner quotes here, as it would sacrifice readability.

[J0nKn1ght]

OK, thanks. So is there no built in way to produce a full mail address string which can be re-parsed? I've added an extension method to build the elements up and escape double quotes in the DisplayName property, but it seemed like there should be a method in the framework to produce a string which can be re-parsed using the MailAddress constructor (or anything else that accepts an RFC 2822 format email address).
Happy to carry on using my extension method if you don't think it would be useful.

[karelz]

Triage: We already allow proper parsing (added in #907). Adding proper serialization as mirroring functionality makes sense. Using ToString is probably best.
Looks like fairly straightforward change -- any interest to contribute a PR @J0nKn1ght?
cc @MarcoRossignoli

[J0nKn1ght]

Hi @karelz - Sorry, I've been busy with project deadlines. As you say, it's a simple change, so I could look at doing a PR. Do you think that it's acceptable to always return a mail address from ToString with escaped embedded double quotes? Can you envisage any circumstances where that could break existing code that uses ToString?

[jaymin93]

I was also checking for the issue you opened but when I add double quotes string the values I am getting back the same way so can you explain. Why you need this actually I seen your expected output but it's escaping the already escaped string not the original one so what display name currently returns that seems perfect for me

[jaymin93]

And in your extension method are you replacing only double quotes or any other special char can you show extension method ?

[J0nKn1ght]

@jaymin93 So, if you have a variable (say 'displayName') that contains:

Henry "The Fonz" Winkler

and you instantiate a new MailAddress with:

var mailAddress = new MailAddress("hw@yopmail.com", displayName);

if you call the ToString() method and view the result it contains this:

"Henry "The Fonz" Winkler" <hw@yopmail.com>

i.e. there are four double quote characters, and the ones around 'The Fonz' are not escaped (to a higher level than the surrounding ones). Therefore, anything trying to parse the mail address will match the first double quote with the one before 'The...', not the one after 'Winkler'. To correctly parse the mail address, the double quotes around 'The Fonz' must be escaped.

We're actually getting the error when passing the value from ToString to the constructor of MailAddress again, which should parse it correctly, but I suspect that anything trying to parse an RFC 2822 mail address will have the same problem. There's information on page 9 of the RFC 2822 document about quoted pairs, and an example on page 41 of a mail address containing double quote characters, showing that they have to be escaped.

The extension method that I'm using is this:

        public static string ToSafeString(this MailAddress mailAddress)
        {
            if (!string.IsNullOrEmpty(mailAddress.DisplayName))
                return $"\"{mailAddress.DisplayName.Replace("\"", "\\\"")}\" <{mailAddress.Address}>";
            else
                return mailAddress.Address;
        }

As the C# string contains double quote characters that are already escaped, the extension method replaces any embedded double quotes with two additional escape characters, so that the escape character is itself escaped.

Does that make sense?

[jaymin93]

ok now i got what your concern is all about , and i will be happy to add pr for same if you do not mind . so should i go ahead and add pr ?

@karelz i am interested adding pr for same , should i create overload of tostring( bool escape) or should i add replace logic in the existing to string ?