Project

General

Profile

Actions

Bug #16948

closed

bp-email-type doesn't properly deal with 'to' arrays

Added by Boone Gorges over 1 year ago. Updated over 1 year ago.

Status:
Resolved
Priority name:
Normal
Assignee:
Category name:
Registration
Target version:
Start date:
2022-10-03
Due date:
% Done:

0%

Estimated time:
Deployment actions:

Description

I experienced a fatal error in my PHP 8.x testing environment during the Commons registration process. Backtrace:

[03-Oct-2022 18:59:58 UTC] PHP Fatal error:  Uncaught TypeError: strlen(): Argument #1 ($str) must be of type string, array given in /home/bgorges/sites/xcommons/wp-includes/formatting.php:3675
Stack trace:
#0 /home/bgorges/sites/xcommons/wp-content/plugins/buddypress/bp-core/classes/class-bp-email-recipient.php(74): sanitize_email()
#1 /home/bgorges/sites/xcommons/wp-content/plugins/bp-email-type/bp-email-type.php(93): BP_Email_Recipient->__construct()
#2 /home/bgorges/sites/xcommons/wp-includes/class-wp-hook.php(309): Ray_BP_Email_Type->check_user_option()
#3 /home/bgorges/sites/xcommons/wp-includes/plugin.php(191): WP_Hook->apply_filters()
#4 /home/bgorges/sites/xcommons/wp-content/plugins/buddypress/bp-core/bp-core-functions.php(3652): apply_filters()
#5 /home/bgorges/sites/xcommons/wp-content/plugins/buddypress/bp-core/bp-core-filters.php(591): bp_send_email()
#6 /home/bgorges/sites/xcommons/wp-includes/class-wp-hook.php(307): bp_core_activation_signup_user_notification()
#7 /home/bgorges/sites/xcommons/wp-includes/plugin.php(191): WP_Hook->apply_filters()
#8 /home/bgorges/sites/xcommons/wp-includes/ms-functions.php(1066): apply_filters()
#9 /home/bgorges/sites/xcommons/wp-includes/class-wp-hook.php(307): wpmu_signup_user_notification()
#10 /home/bgorges/sites/xcommons/wp-includes/class-wp-hook.php(331): WP_Hook->apply_filters()
#11 /home/bgorges/sites/xcommons/wp-includes/plugin.php(476): WP_Hook->do_action()
#12 /home/bgorges/sites/xcommons/wp-includes/ms-functions.php(892): do_action()
#13 /home/bgorges/sites/xcommons/wp-content/plugins/buddypress/bp-members/bp-members-functions.php(1891): wpmu_signup_user()
#14 /home/bgorges/sites/xcommons/wp-content/plugins/buddypress/bp-members/screens/register.php(263): bp_core_signup_user()
#15 /home/bgorges/sites/xcommons/wp-includes/class-wp-hook.php(307): bp_core_screen_signup()
#16 /home/bgorges/sites/xcommons/wp-includes/class-wp-hook.php(331): WP_Hook->apply_filters()
#17 /home/bgorges/sites/xcommons/wp-includes/plugin.php(476): WP_Hook->do_action()
#18 /home/bgorges/sites/xcommons/wp-content/plugins/buddypress/bp-core/bp-core-dependency.php(388): do_action()
#19 /home/bgorges/sites/xcommons/wp-includes/class-wp-hook.php(307): bp_screens()
#20 /home/bgorges/sites/xcommons/wp-includes/class-wp-hook.php(331): WP_Hook->apply_filters()
#21 /home/bgorges/sites/xcommons/wp-includes/plugin.php(476): WP_Hook->do_action()
#22 /home/bgorges/sites/xcommons/wp-content/plugins/buddypress/bp-core/bp-core-dependency.php(445): do_action()
#23 /home/bgorges/sites/xcommons/wp-includes/class-wp-hook.php(307): bp_template_redirect()
#24 /home/bgorges/sites/xcommons/wp-includes/class-wp-hook.php(331): WP_Hook->apply_filters()
#25 /home/bgorges/sites/xcommons/wp-includes/plugin.php(476): WP_Hook->do_action()
#26 /home/bgorges/sites/xcommons/wp-includes/template-loader.php(13): do_action()
#27 /home/bgorges/sites/xcommons/wp-blog-header.php(19): require_once('...')
#28 /home/bgorges/sites/xcommons/index.php(17): require('...')
#29 {main}
  thrown in /home/bgorges/sites/xcommons/wp-includes/formatting.php on line 3675

After some examination, I think that the fatal error is uncovering an existing bug in the bp-email-type plugin. Here's the logic:

1. During registration, BP sends activation keys to the new user like this: https://buddypress.trac.wordpress.org/browser/tags/10.4.0/src/bp-core/bp-core-filters.php?marks=591#L564 Note that the 'to' parameter is something like:

[
  0 => [
    'test@example.com' => 'salutation'
  ]
]

2. bp_send_email() correctly handles 'to' arrays: https://buddypress.trac.wordpress.org/browser/tags/10.4.0/src/bp-core/classes/class-bp-email.php?marks=920#L902
3. But when bp-email-type hooks into the sending of the activation email (https://github.com/cuny-academic-commons/cac/blob/e991bdfea26176d9727fa03298870b09b1c33acb/wp-content/plugins/bp-email-type/bp-email-type.php#L35), it doesn't account for arrays: https://github.com/cuny-academic-commons/cac/blob/e991bdfea26176d9727fa03298870b09b1c33acb/wp-content/plugins/bp-email-type/bp-email-type.php#L92
4. BP_Email_Recipient runs the value of $to through sanitize_email(), which in turn does a strlen check. In PHP 7, this fails silently. In PHP 8, it throws a fatal error.

The PHP 7 behavior has never bothered us, since it just ends up hitting the bail clause here anyway https://github.com/cuny-academic-commons/cac/blob/e991bdfea26176d9727fa03298870b09b1c33acb/wp-content/plugins/bp-email-type/bp-email-type.php#L95 But it feels like a bug anyway. I think we should be looping through if 'to' is an array. Ray, does this seem right to you? If so, could you make the change and test?

Actions

Also available in: Atom PDF