Escape quotes and backslashes in MailAddress display name when encoding SMTP headers#128979
Escape quotes and backslashes in MailAddress display name when encoding SMTP headers#128979Copilot wants to merge 6 commits into
Conversation
Co-authored-by: MihaZupan <25307628+MihaZupan@users.noreply.github.com>
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
|
@copilot please add support for escaping |
Co-authored-by: mrek-msft <188900745+mrek-msft@users.noreply.github.com>
Done. Both Note This response was generated by GitHub Copilot. |
|
@copilot, please move |
Co-authored-by: mrek-msft <188900745+mrek-msft@users.noreply.github.com>
Done in 41e68cd. Extracted into a private static Note This response was generated by GitHub Copilot. |
| // The literal '\' in the input is RFC 5322-escaped as '\\' in the | ||
| // resulting quoted-string when allowUnicode permits the quoted-string | ||
| // form (i.e. no Q-encoding). |
There was a problem hiding this comment.
@copilot remove this comment and add " to test input above (+ escaped into expected output below).
There was a problem hiding this comment.
Done in 52b71be. Added " to the input string (between ! and #), updated the allowUnicode=true expectation to include \", updated the Q-encoded expectation to include =22 (which shifts the line folds), and removed the comment.
Note
This response was generated by GitHub Copilot.
| [MemberData(nameof(DisplayNamesWithSpecialChars))] | ||
| public void EncodeSingleMailAddress_WithDisplayNameContainingSpecials_ShouldEscapeAsQuotedPairs(string displayName, string expectedEncodedDisplayName) | ||
| { | ||
| // Regression test for https://github.com/dotnet/runtime/issues/52439: |
Co-authored-by: mrek-msft <188900745+mrek-msft@users.noreply.github.com>
Co-authored-by: mrek-msft <188900745+mrek-msft@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes RFC quoted-string escaping for MailAddress display names by ensuring embedded " and \ characters are emitted as quoted-pairs in both the SMTP header encoding path (MailAddress.Encode) and the string rendering path (MailAddress.ToString).
Changes:
- Add a shared escaping helper in
MailAddressand use it from bothToString()and the ASCII /allowUnicodebranch ofEncode(...). - Update and extend unit tests to cover display names containing quotes, backslashes, and combinations thereof for both single and multiple address encoding.
- Adjust the existing “range of chars” test expectations for the
allowUnicode=trueoutput to reflect the new escaping behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/System.Net.Mail/src/System/Net/Mail/MailAddress.cs | Escapes \ and " in quoted display names for both ToString() and the ASCII/allowUnicode Encode path via a shared helper. |
| src/libraries/System.Net.Mail/tests/Unit/MailAddressTests/MailAddressEncodeTest.cs | Adds theory-based regression coverage for escaping in MailAddress.Encode, MailAddressCollection.Encode, and ToString, and updates a pre-existing expectation. |
| src/libraries/System.Net.Mail/tests/Unit/MailAddressTests/MailAddressDisplayNameTest.cs | Extends display-name test data to include backslash cases and updates expected ToString() escaping accordingly. |
MailAddress.Encode— the pathSmtpClientuses to writeFrom/To/Cc/etc. headers — wraps the display name in"…"but does not escape RFC 5322 quoted-string specials ("and\), producing an invalid quoted-string and corrupting the header when aDisplayNamelikeHenry "The Fonz" WinklerorC:\path\to\fileis used.MailAddress.ToString()previously escaped"but not\; only the SMTP send path was missed when quote support was added in #36752.Changes
MailAddress.Encode(ASCII /allowUnicode=truebranch): escape embedded\as\\and"as\", producing a valid RFC 5322 quoted-string. The Q-encoded (non-ASCII) branch is unaffected since encoded-words don't use quoted-string syntax.MailAddress.ToString: apply the same\→\\escaping alongside the existing"→\"escaping, so both rendering paths are consistent.\is escaped first, then", otherwise the newly inserted backslashes would themselves get double-escaped.[Theory]-based regression tests inMailAddressEncodeTest(with aDisplayNamesWithSpecialCharsmember-data source) covering quote-only, backslash-only, and combined cases for bothMailAddress.EncodeandMailAddressCollection.Encode; extendMailAddressDisplayNameTest.MailAddressTestDataQuoteswith backslash variants and fix the helper that computes the expectedToString()to apply both replacements. The pre-existing range-of-chars test was updated to expect the now-escaped[\\]substring in itsallowUnicode=trueassertion (the Q-encoded path is unchanged).Before vs. after for
new MailAddress("test@example.com", "Henry \"The Fonz\" Winkler").Encode(0, false):Before vs. after for
new MailAddress("test@example.com", "C:\\path\\to\\file").Encode(0, false):