Bug #16948
closedbp-email-type doesn't properly deal with 'to' arrays
0%
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?