Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#6970 closed defect (bug) (fixed)

Email API causes fatal error on WP <= v4.2.6

Reported by: DJPaul Owned by: djpaul
Milestone: 2.5.2 Priority: high
Severity: normal Version:
Component: Emails Keywords:
Cc:

Description

Via https://buddypress.org/support/topic/attempting-to-register/#post-251544

However, after filling in the required information and clicking ‘complete signup’ they are given this message:
Warning: require_once(extras/class.html2text.php): failed to open stream: No such file or directory in /public_html/wp-includes/class-phpmailer.php on line 2826
Fatal error: require_once(): Failed opening required ‘extras/class.html2text.php’ (include_path=’.:/usr/lib/php:/usr/local/lib/php’) in /public_html/wp-includes/class-phpmailer.php on line 2826

Relevant core change was https://core.trac.wordpress.org/ticket/25560, see the commit note. Looks like this was a WordPress bug that was only discovered/fixed relatively recently. Goes to show how long everyone's put up with wp_mail(), I suppose.

Change History (7)

#1 @DJPaul
6 years ago

  • Component changed from API to API - Emails
  • Priority changed from normal to high

#2 @DJPaul
6 years ago

This is down to a change in PHPMailer->html2text(). A "use advanced HTML2Text conversion" toggle was a bool, but in these later versions, it was changed to a callable, which we are setting to wp_strip_all_tags().

Last edited 6 years ago by DJPaul (previous) (diff)

#3 @DJPaul
6 years ago

We're using $phpmailer->msgHTML() to set Body and AltBody properties. AltBody is what is set to the end result of this html2text() call.

The default escaping for AltBody looks like

html_entity_decode(
        trim(strip_tags(preg_replace('/<(head|title|style|script)[^>]*>.*?<\/\\1>/si', '', $html))),
        ENT_QUOTES,
        $this->CharSet
);

...which looks vastly insufficient.

What I suggest is only set the aforementioned toggle to wp_strip_all_tags only if on WP >= 4.3, and otherwise, after the msgHTML, explicitly set the AltBody property to $this->normalizeBreaks( wp_strip_all_tags( $message ) );.

#4 @DJPaul
6 years ago

I wrote the above without looking at the code. Turns out, we already always set AltBody for HTML emails to the plaintext version of the email, so we can just remove the wp_strip_all_tags parameter because it's not useful.

This is hard to catch in unit tests because we mock the email delivery class so that we don't send emails -- and that's exactly where this problematic code is.

#5 @DJPaul
6 years ago

r10668 r10669 are relevant because those are tests I added to try to catch this.

#6 @djpaul
6 years ago

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

In 10670:

Emails: fix fatal errors when sending emails with old versions of WordPress (<4.3).

An upstream change in WordPress #WP25560 updated the version of
PHPMailer bundled with WordPress, from version 5.2.4 to 5.2.7. Prior to
that change, it was also impossible to use PHPMailer directly (outside
of wp_mail()) due to how PHPMailer was/is bundled with WordPress. The
PHPMailer change that concerns us is in PHPMailer->html2text(). A
parameter to use "advanced HTML-to-text conversion" was a bool, but in
the later version, it was changed to a callable parameter.

All of the above, combined with PHP's handling of truthy variables,
meant we were triggering the PHPMailer bug, responsible for the fatal
errors, on sites running old version of WordPress.

Fortunately, the fix is simple. We can remove our custom HTML-to-text
conversion parameter as it doesn't actually matter because we always
explicitly set the plain text version of the email content, after
PHPMailer runs its built-in HTML-to-text conversion function,
regardless of whether BuddyPress sends emails in plain text or HTML.

Fixes #6970

#7 @djpaul
6 years ago

In 10671:

Emails: fix fatal errors when sending emails with old versions of WordPress (<4.3).

An upstream change in WordPress #WP25560 updated the version of PHPMailer bundled with WordPress, from version 5.2.4 to 5.2.7. Prior to that change, it was also impossible to use PHPMailer directly (outside of wp_mail()) due to how PHPMailer was/is bundled with WordPress. The PHPMailer change that concerns us is in PHPMailer->html2text(). A parameter to use "advanced HTML-to-text conversion" was a bool, but in the later version, it was changed to a callable parameter.

All of the above, combined with PHP's handling of truthy variables, meant we were triggering the PHPMailer bug, responsible for the fatal errors, on sites running old version of WordPress.

Fortunately, the fix is simple. We can remove our custom HTML-to-text conversion parameter as it doesn't actually matter because we always explicitly set the plain text version of the email content, after PHPMailer runs its built-in HTML-to-text conversion function, regardless of whether BuddyPress sends emails in plain text or HTML.

Fixes #6970 (2.5 branch)

Note: See TracTickets for help on using tickets.