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?