URLs wrapped in <> parsed as HTML when wp_mail_content_type set to text/html

[iandunn]

[16285] wrapped the password-reset URL in the e-mail sent from retrieve_password() in greater-than/less-than signs in order to prevent it from breaking when line wrapped (see #14140). That has the side-effect of causing the URL to be parsed as HTML by the mail client when the message's content-type is set to text/html via the wp_mail_content_type filter. I don't see any other places in Core where this happens.

Using wp_mail_content_type to enable HTML e-mails is a common technique documented on the Codex and across the Web. Is it considered a bad practice to enable it globally -- as opposed to adding it before calling wp_mail(), then removing it after; or just setting the content type in the $headers param of wp_mail()? If so, we should document that on the Codex and possibly in wp_mail() where the filter is applied.

Even if it is, I don't think it'd hurt to have retrieve_password() check the content-type and behave accordingly. If it's "text/html", then create a proper link; otherwise wrap the URL in greater-than/less-than signs.

If everyone agrees on that approach, I'll submit a patch.

[SergeyBiryukov]

Related/duplicate: #21095

[iandunn]

Thanks Sergey, I didn't come across that when I searched for an existing ticket. It sounds like the suggestion there was to revert [16285], and I agree that that's not a good idea.

I do still think this is a valid issue, though, because the current behavior leads to unexpected problems, and simply making retrieve_password() aware of the content-type before it constructs the message is an easy fix.

[SergeyBiryukov]

Replying to iandunn:

simply making retrieve_password() aware of the content-type before it constructs the message is an easy fix.

Yes, that makes sense to me.

[dd32]

Using wp_mail_content_type to enable HTML e-mails is a common technique documented on the Codex and across the Web. Is it considered a bad practice to enable it globally -- as opposed to adding it before calling wp_mail(), then removing it after; or just setting the content type in the $headers param of wp_mail()? If so, we should document that on the Codex and possibly in wp_mail() where the filter is applied.

IMHO, Yes, If you enable it globally, you also have to take into account the fact that others use wp_mail, and expect to pass plain text content to it, and you must therefor also process the plain text accordingly with a filter or whatever is needed.

If a core change was made here, I'd be personally for detecting the HTML mimetype in wp_mail, and running any plain text input to htmlentities(), but that of course brings up questions of how to detect the content is HTML or plain text.

[DaveWP196]

There is a problem with some webmail clients with the current message structure with the trailing ">" being treated as part of the URL. As a consequence, the user is unable to reset their password as their username has ">" appended in the reset request.

It would be probably better is the entire section was coded to send an HTML message reset request, or additional parameters were added to the 'retrieve_password_message' filter to pass the username and URL to a custom message generator so that themes can generate their own message in HTML.

[iandunn]

#18493 already exists for changing all Core e-mails over to HTML; it hasn't had any activity for 5 months, so I don't think we can count on it being committed soon, but I'm hoping that it will be eventually.

Since retrieve_password() is the only Core function that wraps URLs in <>, then I think just making it aware of the content-type is a good solution until everything is converted over to HTML.

[SergeyBiryukov]

Related: #23420

[SergeyBiryukov]

#33328 was marked as a duplicate.

[ibrythill]

#34056 was marked as a duplicate.

[SergeyBiryukov]

#34400 was marked as a duplicate.

[SergeyBiryukov]

#34970 was marked as a duplicate.

[pento]

#38613 was marked as a duplicate.

[arnoldbuzdygan]

Replying to pento:

#38613 was marked as a duplicate.

Great, pity that this bug is still there :(

[pento]

#39551 was marked as a duplicate.

[SergeyBiryukov]

Replying to iandunn:

Since retrieve_password() is the only Core function that wraps URLs in <>, then I think just making it aware of the content-type is a good solution until everything is converted over to HTML.

wp_new_user_notification() does the same​ since [33023], and, since it's a pluggable function, potentially any plugin that overrides it would use angle brackets as well.

23578.diff​ is a step in the right direction, but I don't think retrieve_password() is the right place for this fix:

• nl2br() makes the message more readable, but it would be inconsistent with other core emails if wp_mail_content_type is set to text/html globally.
• As noted above, it doesn't fix plugins that might override wp_new_user_notification() or wrap URLs in angle brackets in their own emails.

I think we should deal with it in wp_mail(), see 23578.2.diff​.

[SergeyBiryukov]

#39742 was marked as a duplicate.

[swissspidy]

#43206 was marked as a duplicate.

[SergeyBiryukov]

#43662 was marked as a duplicate.

[swissspidy]

#43918 was marked as a duplicate.

[SergeyBiryukov]

In 47086:

Mail: Remove angle brackets from password reset URL in emails sent by retrieve_password() and wp_new_user_notification().

The brackets were originally added in [16285] per W3C recommendation in ​https://www.w3.org/Addressing/URL/5.1_Wrappers.html to avoid wrapping the URL across multiple lines in plain text in older email clients.

This doesn't seem like a common issue in modern email clients, and the current implementation causes more issues than it solves. Since the URL is on a line by itself, it should not require any delimiters.

The URL in recovery mode email introduced in [44973] doesn't have angle brackets, so it's time to retire them in password reset email too if they're not used consistently.

Props donmhico, Otto42, sproutchris, iandunn, dd32, DaveWP196, sebastian.pisula, tommix, sablednah, julian.kimmig, Rahe, clayisland, arenddeboer, nicole2292, nagoke, squarecandy, eatingrules, SergeyBiryukov.
Fixes #21095, #23578, #44589.

[nick-v]

Hi,

I think closure of this ticket means that the last bullet here can be erased : ​https://developer.wordpress.org/reference/hooks/wp_mail_content_type/#more-information.

Thanks,