I don’t know if it’s related to that, but I certainly see how this situation can come about for others as it did for you.
It would be nice to have some validation for the field’s value on the form, if that’s possible. But the mix-up shouldn’t happen even if one user did have that value entered incorrectly.
I think the problem may be in get_user_id_wit
h, if the 2nd argument ($os0
) is also passed from somewhere, then this query would be used:
SELECT `user_id`
FROM `".$wpdb->usermeta."`
WHERE (
`meta_key` = '".$wpdb->prefix."s2member_subscr_id'
OR `meta_key` = '".$wpdb->prefix."s2member_subscr_baid'
OR `meta_key` = '".$wpdb->prefix."s2member_subscr_cid'
OR `meta_key` = '".$wpdb->prefix."s2member_first_payment_txn_id'
)
AND (
`meta_value` = '".esc_sql($subscr_txn_baid_cid_id)."'
OR `meta_value` = '".esc_sql($os0)."'
)
LIMIT 1
$os0
could get the value of option_selection1
. E.g. s2member/src/includes/classes/paypal-return-in-subscr-or-wa-w-level.inc.php
$user_id = c_ws_plugin__s2member_utils_users::get_user_id_with($paypal['subscr_id'], $paypal['option_selection1'])
option_selection1
is meant for the user ID when updating an existing user, but for new users it defaults to the domain name, same as the custom value. At least that’s what I find that Jason did in s2member-pro/src/includes/classes/gateways/stripe/stripe-checkout-in.inc.php for example.
$ipn['option_name1'] = 'Referencing Customer ID';
$ipn['option_selection1'] = $old__subscr_or_wp_id;
or if a new user
$ipn['option_name1'] = 'Originating Domain';
$ipn['option_selection1'] = $_SERVER['HTTP_HOST'];
So if you now enter the domain name as the subscr CID for a user in his profile, as you described doing by mistake, maybe you can have some situations where the wrong user could be picked up by the query, because it would find a match here:
`meta_key` = '".$wpdb->prefix."s2member_subscr_cid'
`meta_value` = '".esc_sql($os0)."'
Problem is that the line that calls get_user_id_with
is not checking if it’s ‘Referencing Customer ID’ or ‘Originating Domain’, it just uses ‘option_selection1’. That should only be used when it’s actually an ID, not something else.
Of course all this happens under the rare case that you experienced, but it should be prevented. I’ll see what I can do, but I think that adding the ID check (i.e. is it an integer) in get_user_id_with
, will be the simplest solution. E.g. $os0 == (int)$os0
maybe?
if($subscr_txn_baid_cid_id && !empty($os0) && $os0 == (int)$os0)
Do you want to try that improved conditional?
Maybe I’m wrong somewhere there, but I think that it would be safer to validate that 2nd argument as an actual ID, because it shouldn’t be anything else, and although I’m still not sure if it could ever be queried when the domain name is there, it seems possible from what I see, and it shouldn’t be used.
Ideally, there would be a refactor to not use the same variable for two completely different things, but that will not happen now. I don’t know in what other places Jason expected the domain name in that field. I’ll just avoid this kind of thing in the new s2Member I want to build.