Update PHPMailer to 6.x

[Synchro]

PHPMailer 5.2 has now been deprecated and ​PHPMailer 6.0 has been released. Minimum requirements have been increased to PHP 5.5 and it now has a namespace, which will require changes in WP's use of it.

[netweb]

Related: #40472 Update PHPMailer to 5.2.25

cc @MattyRob

[Synchro]

You may find this useful : ​https://stackoverflow.com/questions/45940509/how-should-i-upgrade-from-phpmailer-5-2-to-6-0

[johnbillion]

As the WordPress project is a long way from requiring PHP 5.5 as a minimum, updating to PHPMailer 6 isn't on the cards, unless somebody wants to maintain a fork that's compatible with PHP 5.2.

Closing as maybelater. We'll get there eventually.

[ocean90]

#45276 was marked as a duplicate.

[SergeyBiryukov]

Reopening. As of ​WordPress 5.2, PHP 5.6.20+ is required, so if that was the remaining limitation in upgrading, it shouldn't be a blocker anymore.

[SergeyBiryukov]

#45822 was marked as a duplicate.

[SergeyBiryukov]

41750.diff​ seems to work in my testing, bumping PHPMailer to the ​latest release, 6.0.7 at the moment.

Per the ​Stack Overflow thread and ​upgrading notes, the biggest change for us is probably the introduction of the PHPMailer\PHPMailer\PHPMailer namespace:

The reason this class ends up with this "triple name" is because it's the PHPMailer class, in the PHPMailer project, owned by the PHPMailer organisation. This allows it to be differentiated from other forks of PHPMailer, other projects by the PHPMailer organisation, and other classes within the project.

With enough testing and a dev note, I think this can still make it into 5.3.

[desrosj]

41750.2.diff​ updates the MockPHPMailer class to extend the PHPMailer class at the correct new namespace.

Also, it looks like PHPMailer now includes its own Exception class that is used by default. The only thing that class does is wrap exception messages in HTML. 41750.2.diff​ adds a preceding \ to all Exception occurrences to use the globally namespaced Exception instead.

There are still some unit test failures that need to be investigated.

[Synchro]

FWIW, I'm, very happy to see this update going ahead, thanks for getting it into WP! Is there anything you need help with?

[SergeyBiryukov]

#48072 was marked as a duplicate.

[davidbaumwald]

@desrosj Is this something that make it in for Beta 1 of version 5.3 tomorrow, or do the test failures still need investigating?

[desrosj]

I think we should play this one safe and punt. @SergeyBiryukov if you feel otherwise, feel free to move it back.

[Synchro]

I'm not sure when your deadline is for 5.3, but there are some reasonably important bug fixes in the pipeline for PHPMailer.

[jrf]

Replying to Synchro:

I'm not sure when your deadline is for 5.3, but there are some reasonably important bug fixes in the pipeline for PHPMailer.

@Synchro Hi Marcus, sorry that nobody replied to you before, but WP 5.3 has already gone into feature freeze.

An upgrade like this will need plenty of testing as the impact could be large if not done 100% correctly, not just for WP itself, but also for plugins/themes using the functionality provided via WP, so should preferably be made early in a release cycle.

[nasiralamreeki]

WordPress really needs to take more seriously updating external libraries. Years between updating libraries from upstream is unacceptable. There’s tremendous benefits to getting these libraries updated faster.

We can’t wait for theme and plugin developers forever to test. Sometimes we have to push the web forward by pushing ahead with changes even if they break themes and plugins as this in turn will push developers to update their code.

We pushed Gutenberg ahead even though most major themes and plugins weren’t ready for it.

[davidbaumwald]

The latest patch no longer applies cleanly, so it needs-refresh. @desrosj given that, is this doable for 5.4?

[SergeyBiryukov]

#49298 was marked as a duplicate.

[donmhico]

PHPMailer 6.1.4

[donmhico]

41750.3.diff integrates PHPMailer 6.1.4 . ​https://github.com/PHPMailer/PHPMailer/tree/v6.1.4. As @davidbaumwald mentioned, there are failing unit tests that needs more investigating. I'll try to look more into it, but anyone please feel free to jump in.

[ayeshrajans]

The tests fail because the PHPMailer/Exception class is not loaded. This is new from the current version, so we need to manually require this class in addition to the class-phpmailer.php file.

This patch takes a slightly new approach:

1. Instead of updating src/wp-includes/class-smtp.php and class-phpmailer.php files, this patch creates a new directory src/wp-includes/PHPMailer/ and adds verbatim PHPMailer.php, SMTP.php, and POP3.php classes. There is a new exception class at Exception.php, which is included as well.

2. To maintain BC, existing src/wp-includes/class-smtp.php, class-phpmailer.php, and class-pop3.php files require the new files.

3. PHPMailer uses the PHPMailer\PHPMailer\Exception as the exception class. This is updated in pluggable.php file. We use Fully-Qualified Class Names (FQCN) here because using FQCN doesn't pollute the header of pluggable.php class.

4. In this patch, all tests related to PHPMailer and SMTP are passing. However, the new POP3.php class is not exactly an easy migrate, nor I have personally done it before. Help welcome.

5. It also updated the mail mockup file to use the new file paths.

So far, this patch fixes all but one test related to mail-fetching from ./wp-mail.php file.

This will break the phpcs.xml ignore list because PHPMailer is in a new path. This can be fixed with a patch similar to that updates the phpcs.xml file. It is also attached to this ticket.

Future plans:

1. We will need more test coverage because right now, all PHPMailer-thrown exceptions are silently ignored. This is a bad practice, and ideally should be handled in a layer above.

2. The POP3 functionality doesn't appear to have enough tests.

CI results: ​https://travis-ci.org/Ayesh/wordpress-develop/jobs/647470540

[ayeshrajans]

@Synchro you are the author of the library :) Please provide any input you have, specially might be helpful for the POP3 usage. Thank you.

[desrosj]

If we are going to move the library into its own directory, then we should consider requiring it with Composer and using the grunt build process to copy the files into the correct location.

The old files should also include a _deprecated_file call to alert developers the files have moved.

[desrosj]

Hit submit too soon.

This would be the first library we include this way. We would need to confirm that the build server has Composer installed and configured correctly.

It would make updating the library going forward much easier, though.

[ayeshrajans]

It would be lovely if we can get the build server involved to fetch library. We might as well be able to incorporate composer autoloader (with composer.json config.vendor-dir set to src/wp-includes or another way.

To be honest, I think this would derail this issue a bit outside because the majority of the work would be composer/grunt changes as opposed to modifying WordPress core to adapt to PHPMailer 6.

Composer integration is something I personally would love to see in WP, and I would gladly dedicate a few hours every week to volunteer on this ♥️.

[Synchro]

@ayeshrajans your approach sounds pretty sensible to me.

You only need to use PHPMailer\PHPMailer\Exception if you're explicitly using exceptions (they are turned off by default). You do still need to have the Exception class loaded as even with exceptions disabled it is used internally. So long as it's disabled, you shouldn't have any uncaught exceptions. Do you have examples where this is a problem?

Frankly I feel a little unwell at the thought that some might still be using the POP3 library! Nobody should be using POP-before-SMTP for auth any more - the last time I used it was about 30 years ago. Indeed it doesn't have any test coverage - it was originally a third party library that was donated to the project, and it's a difficult thing to test as it requires coordination across protocols. Do you have any visibility on whether it's being used at all?

Composer, at least in a build system, would seem essential, but WP obviously has a lot of BC issues to consider, and I know very little about the WP codebase.

[ayeshrajans]

Thanks for you reply @Synchro .

You only need to use PHPMailer\PHPMailer\Exception if you're explicitly using exceptions (they are turned off by default). You do still need to have the Exception class loaded as even with exceptions disabled it is used internally. So long as it's disabled, you shouldn't have any uncaught exceptions. Do you have examples where this is a problem?

The way WordPress deals with the exceptions is by, well, ignoring them. For example, some tests fail where it tries to set an invalid email address, and the expected exception is not the same as PHPMailer 6 throws. As of now, all PHPMailer-specific exceptions are abstracted. Fatal errors are thrown with WP_Error which throws a WPDieException. As of now, the exceptions are closed within wp_mail function.

Frankly I feel a little unwell at the thought that some might still be using the POP3 library! Nobody should be using POP-before-SMTP for auth any more - the last time I used it was about 30 years ago. Indeed it doesn't have any test coverage - it was originally a third party library that was donated to the project, and it's a difficult thing to test as it requires coordination across protocols. Do you have any visibility on whether it's being used at all?

Thank you - I honestly doubt if there is a meaningful real-life usage of this mail-fetch feature. I'd be overjoyed if this feature was moved to a contributed plugin and kept WP core clean.

Composer, at least in a build system, would seem essential, but WP obviously has a lot of BC issues to consider, and I know very little about the WP codebase.

Every file in wp-includes is included on-demand with hardcoded file names, and WP does not necessarily follow PSR-4 or PSR-0 naming conventions either. Realistically, we have a long way to go to support composer fully throughout WordPress.

[Synchro]

If you don't enable exceptions in PHPMailer, they shouldn't leak out because they are either not thrown (for example ​here, where the function returns false instead) or are caught internally (like ​here).

Are you saying that because you have not loaded PHPMailer's own exception class, it actually dies with an unknown class error, as opposed to throwing an exception that's uncaught?

[ayeshrajans]

Hi @Synchro, yes the test failures were from the undeclared PHPMailer\PHPMailer\Exception class. ​CI log.

Even before this patch, WordPress ​Opts-in to exceptions (with new PHPMailer( true )).

So this patch preserves this behavior, and catches the new PHPMailer\PHPMailer\Exception. I think this is safer because because we get the strict validation from PHPMailer, and any unhandled exceptions from WordPress side will rightfully bubble up.

[ayeshrajans]

I opened #49386 to receive feedback from core maintainers about deprecating Post via email feature, which WordPress uses the POP3 class for.

[ayeshrajans]

The tests pass now!

• It does not try to change anything with POP3. The class-pop3.php file does not appear to be related to the PHPMailer library we have (not 5.2, not 6.0, it might even be unrelated). So the class-pop3.php file remains unchanged.

• Further, wp-mail.php file is not changed either. It will continue to use the class-pop3.php file which we don't change.

• Composer is not incorporate, because I believe we should have discuss it further as to how to implement it properly. As of now, we copy the current PHPMailer 6.1 release verbatime on files SMTP.php, PHPMailer.php, and Exception.php to src/wp-includes/PHPMailer directory.

The failing test from the last patch was not due to POP3 class. It does not look like "Post via email" feature even has tests.

The failing test was because a strpos call on a sent email (mocked) does not contain proper headers. It did, but it was mime encoded which resulted in the strpos call from failing. This patch fixes that test too, but manually decoding the mime-encoded headers.

[ayeshrajans]

​https://travis-ci.org/Ayesh/wordpress-develop/builds/647882845

[davidbaumwald]

This ticket is marked early for soak time, and we're past that point for 5.4. with Beta 1 landing tomorrow. This is being moved to Future Release. If any maintainer or committer feels this can be included in 5.4 or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

Attempting to update PHPMailer to the latest version (currently 6.1.5).

Trac ticket:

[desrosj]

Thanks for working on this, everyone!

I looked at the latest patch, and I think that this approach should work fine. However, I uploaded 41750.4.diff​ with a few small changes.

Some minor adjustments:

• Changed the version notes to 5.5.0.
• Updated the PHPMailer library to ​which was released on March 14, 2020.
• Changed the wording a bit on the deprecated file calls and copied the same sentence into the inline documentation to be more consistent.

Updating the library did introduce a test error to Tests_Mail::test_wp_mail_custom_boundaries, PHPMailer\PHPMailer\Exception: Invalid header name or value. I have not yet ​gotten to investigate that, but it should be a result of one of the changes in 6.1.5.

Lastly and most importantly, I am concerned that 41750.6.patch​ is not quite there from a backwards compatibility standpoint and would break too many sites. Even if we get the word out to plugin developers and they update their plugins, there is no guarantee that site owners actually update their plugins.

I did some exploring of the plugin directory to try and validate this and found ​612 plugins that call new PHPMailer(). Only 11 had 50k+ installs, and there were a small handful of plugins that were bundling their own, updated versions of PHPMailer.

41750.4.diff​ re-adds a PHPMailer class in the original class-phpmailer.php file and only contains a __construct() method that will return an instance of the new, correctly namespaced class and trigger a deprecated function notice. The idea here is to prevent fatal errors when calling new PHPMailer() the "old" way (even though that way was discouraged and technically incorrect). I'm trying to think of any "gotchas" with this approach.

[desrosj]

Sorry, that last approach in 41750.4.diff​ doesn't actually work.

41750.5.diff​ uses class_alias() instead, which does work as intended. 41750.5.diff​ also merges the changes from 41750-phpcs-fixes.patch​ into a single patch, and makes some PHPCS changes to the suggested changes.

The failing test still remains: ​https://github.com/WordPress/wordpress-develop/pull/242/checks?check_run_id=627685741

Props to @wpscholar for testing and refining the latest patch with me.

[ayeshrajans]

Thanks for the patches, and I think we are almost ready for this 🎉.

I took @desrosj's [41750.5.diff] applied on clean master. The exception that caused the test to fail is because PHPMailer rightfully rejected the header with a new line character (\n) in the custom header.

$phpmailer->addCustomHeader( sprintf( "Content-Type: %s;\n\t boundary=\"%s\"", $content_type, $boundary ) );

The content-type header does not need this new line or the tab, so I removed these characters and . ​tests now pass.

(more comments to follow)

[ayeshrajans]

Updated full patch, after applying the new-line character fixes: ​https://github.com/Ayesh/wordpress-develop/commit/9af56987a639a6c1d6c7f648d292379be54b6aca

[ayeshrajans]

Please see 41750-8.patch for updated patch including few more improvements (​patch list compared to 41750-7-obsolete.patch):

1. Simplify $phpmailer->addCustomHeader to use single quotes to the double quotes don't need to be escaped:

$phpmailer->addCustomHeader( sprintf( 'Content-Type: %s; boundary="%s"', $content_type, $boundary ) );

as opposed to:

$phpmailer->addCustomHeader( sprintf( "Content-Type: %s; boundary=\"%s\"", $content_type, $boundary ) );

2. Add exception handling for other $phpmailer->addCustomHeader calls. This brings back the old behavior of silently handling invalid values. $phpmailer->addAttachment already has similar exception handling.

3. Use ::class constant in the deprecated class-phpmailer.php file. PHP 5.5 supports this, and can make the class names clickable in IDEs and helps static analyzers as well.

Tests: ​passing

[Synchro]

Nice to see this is progressing! While I agree with the quoting, that's a very strange example to use: if you're setting content-type headers and boundaries via addCustomHeader, you're using PHPMailer wrong!

[ayeshrajans]

if you're setting content-type headers and boundaries via addCustomHeader, you're using PHPMailer wrong!

This put me off too, seeing WordPress meticulously going through content-type header. I suppose this is an old artifact because even if I go as far as 10000 commits, that code is still there.

[ayeshrajans]

Related: #15448

[desrosj]

In 47902:

General: Continuing to work towards a passing PHP Compatibility scan.

This is a final pass to fix PHP compatibiilty issues in the codebase with code changes or adding phpcs:ignore comments.

With this change, all PHP compatibility warnings and errors without specific tickets have been addressed (see #49810 and #41750).

Props desrosj, johnbillion, jrf.
See #49922.

[desrosj]

Thanks @ayeshrajans!

41750.6.diff​ is an updated patch that includes ​PHPMailer 6.1.6, which was a security update.

I think this is in a really good spot. I plan on committing this at some point this week (hopefully with another review from an additional seasoned contributor), and drafting a call for testing for Make Core/Make Plugins.

@ayeshrajans Let's open new tickets for all of the "using PHPMailer wrong" type issues that are found.

[ayeshrajans]

w00t w00t, thank you!
I started to watch upstream releases of PHPMailer, and will try to submit patches to WordPress to keep WordPress's copy up to date, until we eventually get Composer support. 41750.6.diff looks good to me. Thank you.

I will also open a ticket for PHPMailer usage patterns too. Thank you, I'm excited to see this finally seeing the light 🎉.

[Synchro]

It's great to see this happening! No major changes are expected in PHPMailer in the near future, 6.1.6 should be good to go.

[desrosj]

Moving into the 5.5 milestone.

[desrosj]

In 48033:

External Libraries: Upgrade PHPMailer to version 6.1.6.

Now that WordPress Core supports PHP >= 5.6, the PHPMailer library can be updated to the latest version.

The PHPMailer files now reside in a new directory, wp-includes/PHPMailer. These files are copied verbatim from the library upstream and will make updating in the future easier. For backwards compatibility, the old files will remain and trigger deprecated file warnings.

The PHPMailer class is also now under the PHPMailer\PHPMailer\PHPMailer namespace. The PHPMailer class in the global namespace has been aliased for a seamless transition.

This upgrade also clears up a handful of PHP compatibility issues detailed in #49922.

For a full list of changes, see the PHPMailer GitHub: ​https://github.com/PHPMailer/PHPMailer/compare/v5.2.27...v6.1.6.

Props Synchro, SergeyBiryukov, desrosj, donmhico, ayeshrajans.
Fixes #41750.

[desrosj]

Reopening this for a developer note. I plan to tackle next week, but if someone else would like to create a draft, please feel free! It should focus on the high level changes in the library that plugins and site owners may need to be aware of.

[Synchro]

That's great news!

You may find the upgrading guide a useful starting point: ​https://github.com/PHPMailer/PHPMailer/blob/master/UPGRADING.md

and also this question & answer I wrote:
​https://stackoverflow.com/questions/45940509/how-should-i-upgrade-from-phpmailer-5-2-to-6-0

[ayeshrajans]

Thanks for committing this. We finally have PHPMailer 6!
I opened #50374 to make further improvements to how we are using PHPMailer.

[SergeyBiryukov]

In 48035:

External Libraries: Add class aliases for phpmailerException and SMTP to deprecated files to account for the new namespace.

This ensures backward compatibility with plugins using the old class-phpmailer.php or class-smtp.php files.

Follow-up to [48033].

Props Otto42.
Fixes #50379. See #41750.

[SergeyBiryukov]

Related: #50377, #50380.

[SergeyBiryukov]

In 48036:

Coding Standards: Exclude the whole PHPMailer directory from WPCS checks, for consistency with other external libraries in their own directory.

Follow-up to [48033].

See #41750.

[desrosj]

In 48058:

Build/Test Tools: Prevent double /s when including PHPMailer files.

ABSPATH includes a trailing slash already.

Follow up of [48033].
Props desrosj.
See #50377, #41750.

[desrosj]

Dev note was just published for this: ​https://make.wordpress.org/core/2020/07/01/external-library-updates-in-wordpress-5-5-call-for-testing/

I also compiled a list of plugins all plugins with active installs that appear to be utilizing the library in their code: ​https://docs.google.com/spreadsheets/d/1bYVY3mea2lpv-DA8Ic71v2QJtrRyMKY1GW2JpPGCOC4/edit?usp=sharing

I created this list based off the following three searches:

• ​https://wpdirectory.net/search/01EC5AQHXB3AYV3MPGFTF1H23B
• ​https://wpdirectory.net/search/01EC5AVG3B1VBY61C98Q9ATTSR
• ​https://wpdirectory.net/search/01EC5AWJ934M90R5ADHPGRTFAT

I sent a draft of an email to the plugin team for them to relay to those plugin authors as they feel appropriate.

[SergeyBiryukov]

In 48443:

Tests: Ignore EOL differences in email tests using multiline string assertions.

Unix vs. Windows EOL style mismatches can cause misleading failures in tests using the heredoc syntax (<<<) or multiline strings as the expected result.

Follow-up to [46612], [48033].

Props davidbaumwald.
See #31432, #41750.

[SergeyBiryukov]

In 48530:

Mail: Make sure the PHPMailer class is only required once if a plugin requires wp-includes/class-phpmailer.php directly.

Follow-up to [48033].

Props david.binda.
Fixes #50716. See #41750.

[SergeyBiryukov]

In 50022:

Mail: Make sure the SMTP class is only required once if a plugin requires wp-includes/class-smtp.php directly.

Follow-up to [48033], [48530].

Props oellin, greatsaltlake, audrasjb.
Fixes #52369. See #50716, #41750.