Bug: WP_User_Query returns wrong user details

[ak.singla]

Hello,

I am using the below code to fetch the user from the database.

    $user_query_args = array(
        'search'         => 'abc@email.com',
        'search_columns' => ['user_email', 'ID']
    );
    $user_query = new WP_User_Query($user_query_args);

My Findings:

If the email does not start with numeric digits'
Case 1:

• Email: abc@email.com
• User exists with above email
• Works fine.

If the email starts with numeric digits'
Case 2:

• Email: 123abc@email.com
• User exists with the above email
• Work fine.

Case 3: (INVALID RESULTS)

• Email: 123abc@email.com
• User DOES NOT exist with the above email
• Does not work as intended and returns the user data of the user with ID: 123, instead of returning 0 results

[johnbillion]

Thank you for the report @aksingla

[psykro]

I just tested this, and I'm not 100% sure it's a WordPress bug. The query being generated in case 3 would look like this

SELECT SQL_CALC_FOUND_ROWS wp_users.ID
FROM wp_users
WHERE 1=1 AND (user_email LIKE '123abc@email.com' OR ID = '123abc@email.com')
ORDER BY user_login ASC

If you run it, it will result in user ID 123 being returned.

You can test this by running the following SQL query in a MySQL database tool

SELECT * FROM `wp_users` WHERE id = '123abc@email.com';

And if there is a user with id 123, it will be returned.

However, if you update the search columns to not include the ID field

<?php
'search_columns' => ['user_email']

Then it will only perform the query based on the email field

[dd32]

Replying to psykro:

user_email LIKE '123abc@email.com' OR ID = '123abc@email.com'

If you run it, it will result in user ID 123 being returned.

It seems unexpected that we'd be looking up non-numeric data in a numeric field, so it's probably worthwhile considering this a WordPress bug, even if it's a plugin explicitly asking for it, and ultimately just SQL casting a numeric-like-string to integers, which is something that even PHP does ( intval( '123abc@email.com' ) === 123 - but is_numeric( '123abc@email.com' ) === false ) and WordPress has had to work around in other areas of code before.

'search_columns' => ['user_email', 'ID']

search_columns being explicitly set to search within ID does make this a little plugin-implementation specific though, WordPress only does this when the search field is explicitly numeric.

A better check for a plugin would be something like this:

   $search = 'abc@email.com';
   $user_query_args = array(
        'search'         => $search,
        'search_columns' => ( is_numeric( $search ) ? ['user_email', 'ID'] : [ 'user_email' ] )
    );
    $user_query = new WP_User_Query($user_query_args);

This isn't a 6.1 regression though from what I can see, but I'd be supportive of defensive checks added here somewhat similar to ​https://github.com/WordPress/wordpress-develop/compare/trunk...fix/57731-search-id-with-string

[ak.singla]

Replying to dd32:

'search_columns' => ['user_email', 'ID']

search_columns being explicitly set to search within ID does make this a little plugin-implementation specific though, WordPress only does this when the search field is explicitly numeric.

Referring to search_columns (parameter, type: array), the user can use any combination from the available options.

	search_columns (array) – List of database table columns to matches the search string across multiple columns.
	‘ID‘ – Search by user id.
	‘user_login‘ – Search by user login.
	‘user_nicename‘ – Search by user nicename.
	‘user_email‘ – Search by user email.
	‘user_url‘ – Search by user url.

In File: wp-includes/class-wp-user-query.php, WordPress does check for the type of the search value, but only when the "search_columns" are not explicitly provided in the arguments.

	$search_columns = array();
	if ( $qv['search_columns'] ) {
		$search_columns = array_intersect( $qv['search_columns'], array( 'ID', 'user_login', 'user_email', 'user_url', 'user_nicename', 'display_name' ) );
	}
	if ( ! $search_columns ) {
		if ( false !== strpos( $search, '@' ) ) {
			$search_columns = array( 'user_email' );
		} elseif ( is_numeric( $search ) ) {
			$search_columns = array( 'user_login', 'ID' );
		} elseif ( preg_match( '|^https?://|', $search ) && ! ( is_multisite() && wp_is_large_network( 'users' ) ) ) {
			$search_columns = array( 'user_url' );
		} else {
			$search_columns = array( 'user_login', 'user_url', 'user_email', 'user_nicename', 'display_name' );
		}
	}

There seems to be a need for an added check before creating the SQL statement.

A better check for a plugin would be something like this:

   $search = 'abc@email.com';
   $user_query_args = array(
        'search'         => $search,
        'search_columns' => ( is_numeric( $search ) ? ['user_email', 'ID'] : [ 'user_email' ] )
    );
    $user_query = new WP_User_Query($user_query_args);

Yes, I have already used something similar to change search_columns based on the search term data type when we found this issue with one of our users having "999xxxxx@…" in his email ID.

[riteshtailor]

Replying to dd32:

search_columns being explicitly set to search within ID does make this a little plugin-implementation specific though, WordPress only does this when the search field is explicitly numeric.

A better check for a plugin would be something like this:

   $search = 'abc@email.com';
   $user_query_args = array(
        'search'         => $search,
        'search_columns' => ( is_numeric( $search ) ? ['user_email', 'ID'] : [ 'user_email' ] )
    );
    $user_query = new WP_User_Query($user_query_args);

I agrees with the comment (y)

[ironprogrammer]

During test triage today it was noted that this ticket has testing info (keyword added), and is pending further discussion and a possible patch to move it forward. Thanks, everyone, for your input thus far!