WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 7 months ago

#25014 closed defect (bug) (fixed)

class-phpmailer.php Version 5.2.4 Qmail Bug

Reported by: JaxWeather Owned by: nacin
Milestone: 3.7 Priority: normal
Severity: normal Version: 3.6
Component: External Libraries Keywords: has-patch
Focuses: Cc:

Description

class-phpmailer.php version 5.2.4 which is included with WordPress 3.6 has an apparent bug with mail systems using Qmail.
http://sourceforge.net/p/phpmailer/patches/97/

This was fixed in Version 5.2.5 based on changelog
https://github.com/Synchro/PHPMailer/blob/master/changelog.md
"Fixes for qmail when sending via mail()"

Installing phpmailer.php version 5.2.6 allowed mail() to function normally.

Attachments (4)

25014.patch (54.7 KB) - added by c3mdigital 8 months ago.
Upgrade phpmailer to latest version 5.2.6
25014.2.patch (72.3 KB) - added by bpetty 8 months ago.
25014-3.6.patch (557 bytes) - added by bpetty 8 months ago.
25014.3.patch (263.1 KB) - added by MattyRob 7 months ago.

Download all attachments as: .zip

Change History (20)

comment:1 c3mdigital8 months ago

  • Component changed from Mail to External Libraries
  • Keywords has-patch added

c3mdigital8 months ago

Upgrade phpmailer to latest version 5.2.6

comment:2 SergeyBiryukov8 months ago

  • Milestone changed from Awaiting Review to 3.7

comment:3 markoheijnen8 months ago

#25056 was marked as a duplicate.

comment:4 bpetty8 months ago

Some quick notes:

  1. Version 5.2.6 includes the new "extras/class.html2text.php" functionality that is used when calling PHPMailer::MsgHTML(..., $advanced = true) - which is a new parameter that defaults to false, and is never used from PHPMailer directly, so we should still be fine not including this extra file just as we don't any other extra files.
  1. Since we rename files, this is one change we always need to make to PHPMailer from upstream when updating: just replacing "class.smtp.php" with "class-smtp.php" in "class-phpmailer.php". I've done this in the updated patch.
  1. Speaking of "class-smtp.php", the update to this file was missing from the first patch. I've included it's update with this now too.

bpetty8 months ago

bpetty8 months ago

comment:5 bpetty8 months ago

For 3.6, I'd suggest only applying the quickfix for this rather than updating PHPMailer. I have attached the backported fix as 25014-3.6.patch

comment:6 bpetty8 months ago

  • Cc bpetty added

comment:9 MattyRob7 months ago

  • Version changed from 3.6 to trunk

PHPMailer 5.2.7 is out new and employs an AutoLoader to call the SMTP class, that probably needs including in any future patch to 5.2.7.

I've included a patch here for review but appreciate it may need a new ticket.

MattyRob7 months ago

comment:10 SergeyBiryukov7 months ago

  • Version changed from trunk to 3.6

comment:11 follow-up: bpetty7 months ago

5.2.7 was released 8 days ago, and there's already been one confirmed bug related to the new autoloader in that version that has been fixed since (with no 5.2.8 yet of course), and another related autoloader report that seems to indicate that backwards compat may have been broken for simply including PHPMailer the way we currently do (and how some 3rd party WP libraries/plugins/code might include it too). What versions of PHP have you tested this with?

I'm hesitant about leaping over to 5.2.7 just yet here, especially not with 3.7 this close.

comment:12 in reply to: ↑ 11 MattyRob7 months ago

Replying to bpetty:

I'm hesitant about leaping over to 5.2.7 just yet here, especially not with 3.7 this close.

I've tested only on PHP 5.3.26.

I quite agree that it's probably a step too far for 3.7 because of the auto-loader and the need to include an additional file. The PHPMailer and SMTP classes just need dropped in and renamed to the WordPress convention and a '.' needs replaced with a '-' in the AutoLoader which might make future updates smoother.

comment:13 follow-up: bpetty7 months ago

Yeah, it would be nice to use the original filename for class.smtp.php, and I think this should probably all be done at some point with a full move into a subfolder separating PHPMailer from the rest of WordPress using it's original library structure as long as we can work out back compat solutions as necessary. That should be a future ticket though.

You might notice that in PHPMailer 5.2.7, we still couldn't just exclude the autoloader, and replace the two files we do use since the classes themselves have been modified to rely on the autoloader too (I'm not sure why they do this): https://github.com/PHPMailer/PHPMailer/blob/v5.2.7/class.phpmailer.php#L573-L576

comment:14 in reply to: ↑ 13 MattyRob7 months ago

Replying to bpetty:

Yeah, it would be nice to use the original filename for class.smtp.php, and I think this should probably all be done at some point with a full move into a subfolder separating PHPMailer from the rest of WordPress using it's original library structure as long as we can work out back compat solutions as necessary. That should be a future ticket though.

Perhaps we can patch to 5.2.6 for the 3.7 Release of WordPress and then create a new ticket to migrate to a drop in approach for PHPMailer in 3.8. The existing class-phpmail.php file could be amended to be a wrapper fro the PHPMailer class files stored in a sub-folder. We can then just drop in the most recent / stable release of PHPMailer into a sub-folder in wp-includes.

As an added benefit that might just make updates in the future easier to manage and apply. We'd just have to be sure there are no direct calls to the mailer or smtp classes from the core code or any popular and current plugins.

comment:15 nacin7 months ago

Let's use a new ticket for updating PHPMailer.

comment:16 nacin7 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 25682:

Backport a fix for qmail from PHPMailer upstream.

props bpetty.
fixes #25014.

Note: See TracTickets for help on using tickets.