MimePart: quote an encoded display name only when it could be re-parsed - #107
Merged
Conversation
Member
|
Thanks |
dg
pushed a commit
that referenced
this pull request
Aug 18, 2026
…ed (#107) whenever it contained anything outside RFC 2822 atext. Every accented letter is outside atext, so every non-ASCII name was quoted, and because the quotes land inside the base64 payload, clients showed them as part of the name: "Objednávky domena.cz" instead of Objednávky domena.cz. Per RFC 2047 §6.2 an encoded-word is decoded only after the field is parsed, so quotes there are not syntax; they only guard against a receiver that decodes first and re-parses the result (the Gmail failure behind #102). The encoded path therefore quotes only names holding a character such a receiver could turn into another address (,;:<>@"\); diacritics and dots are encoded bare. The literal path keeps the atext test, where the quotes are real RFC 5322 delimiters.
dg
pushed a commit
that referenced
this pull request
Aug 18, 2026
…ed (#107) whenever it contained anything outside RFC 2822 atext. Every accented letter is outside atext, so every non-ASCII name was quoted, and because the quotes land inside the base64 payload, clients showed them as part of the name: "Objednávky domena.cz" instead of Objednávky domena.cz. Per RFC 2047 §6.2 an encoded-word is decoded only after the field is parsed, so quotes there are not syntax; they only guard against a receiver that decodes first and re-parses the result (the Gmail failure behind #102). The encoded path therefore quotes only names holding a character such a receiver could turn into another address (,;:<>@"\); diacritics and dots are encoded bare. The literal path keeps the atext test, where the quotes are real RFC 5322 delimiters.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
#102 fixed a real failure – a comma in a display name broke DKIM verification at Gmail – by wrapping the phrase in a
quoted-stringbeforeiconv_mime_encode(). That part works and this PR keeps it.The side effect is the predicate:
$escapefires on any character outside RFC 2822atext, and every diacritic is outsideatext. So a name that contains nothing dangerous at all is quoted too, and because the quoting happens before encoding, the quotes land inside the base64 payload and decode as part of the name:Every sender or recipient name with diacritics now reaches the recipient with visible quotation marks around it.
$mail->getFrom()still returns the name unquoted, so the wire format and the API disagree.Solution
Keep the quoting on the encoded path, but only for characters that can actually restructure an address list:
,;:<>@"\. A diacritic or a dot cannot, so those names are encoded bare:The literal (non-encoded) path keeps the original
atexttest unchanged: there the name really is emitted as-is, the quotes are RFC 5322 delimiters, and dropping them would be a syntax error.Worth being explicit about the reasoning, because it is the part #102 got slightly wrong. Per RFC 2047 §6.2:
So in a conforming parser a comma inside base64 cannot split an address – at parse time it is not a comma. The quoting in #102 is therefore not protection against a spec-compliant receiver; it is protection against one that decodes first and re-parses the result. That is a real class of receiver, worth defending against, but it only justifies quoting characters that such a receiver could misparse into a different address structure. Diacritics are not in that set, and paying for them with a visibly mangled display name in every conforming client is a bad trade.
Scope decisions worth a second opinion
.amongspecials, so a strict reading would keep quotingdomena.cz. It cannot restructure an address, and including it would leave the original complaint unfixed, so it is treated as harmless here. Say the word if you would rather have it in.(...)as a comment and hide part of the name, but it cannot end up with a different set of addresses. Happy to add them if you consider hidden text the greater evil.Tests
tests/Mail/Mail.email.encodedName.phptis new and pins both paths: an ASCII name outsideatextstill becomes a realquoted-string, a plain diacritic name and a dotted name encode bare, and a comma, a quote or an embedded<addr>keep the quoting.Three existing expectations change, all of them names with nothing dangerous in them:
Mail.email.phpt"Žluťoučký kůň"→Žluťoučký kůňMail.email.multiple.phpt,Mail.longLines.phpt"Řehoř Řízek"→Řehoř Řízek,"Luboš Smažák"→Luboš SmažákThe two fixtures that motivated #102 are asserted unchanged:
Mail.headers.002.phpt's"Kdo uteče, obědvá"(comma) andMail.email.phpt's"Žluťoučký \"kůň\""(quote). Both now carry a comment saying why they keep the quotes.docs/internals.mdgains a bullet on the two quoting tests and why they differ.Verification
All four CI jobs run locally on PHP 8.4: