WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#16014 closed defect (bug) (fixed)

Search wildcards should only affect multisite

Reported by: nacin Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version:
Component: Users Keywords: regression has-patch
Focuses: Cc:

Description

Wildcard restrictions to get_search_sql() should only affect multisite. Right now they affect wp-admin/users.php.

3:15	nacin	we need an implicit_wildcard flag in WP_User_Query or something
3:16	nacin	and it only does trailing wildcards, which is probably a change
3:17	nacin	good for scaling a network
3:17	nacin	poor for wp-admin/users
3:19	nacin	basically, we'd need to allow get_search_sql() to do leading wildcards
3:19	nacin	and offer an implicit wildcard through users.php

Attachments (4)

16014.diff (1.7 KB) - added by nacin 4 years ago.
16014.2.diff (1.7 KB) - added by SergeyBiryukov 4 years ago.
16014.3.diff (1.7 KB) - added by SergeyBiryukov 4 years ago.
16014.4.diff (2.8 KB) - added by SergeyBiryukov 4 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 @westi4 years ago

Why must we do this for 3.1?

This sounds like something that can wait for 3.2

comment:2 @nacin4 years ago

  • Keywords regression added

comment:3 @nacin4 years ago

Caused in #15170.

@nacin4 years ago

comment:4 @nacin4 years ago

Crude patch attached. Not at all tested.

comment:5 @SergeyBiryukov4 years ago

Took me too long to figure out. The patch seems to be working.

Searching for admin on a single site:

WHERE 1=1 AND (user_login LIKE '%admin%' OR user_nicename LIKE '%admin%') 

On Multisite:

WHERE 1=1 AND (user_login LIKE 'admin' OR user_nicename LIKE 'admin') 

However, searching for admin* on a single site results in “No matching users were found”:

WHERE 1=1 AND (user_login LIKE '%admin%' OR user_nicename LIKE '%admin*%') 

Which is different from 3.0. On Multisite, it's OK:

WHERE 1=1 AND (user_login LIKE 'admin' OR user_nicename LIKE 'admin%') 

Is this correct? Perhaps $search = trim($search, '*') is needed for a single site too?

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

comment:6 @nacin4 years ago

I agree with adding a trim.

@SergeyBiryukov4 years ago

comment:8 @ocean904 years ago

  • Keywords has-patch added

comment:9 @ryan4 years ago

Should we be checking is_multisite() or is_network_admin(). I think these searches are fine as long as they are bounded to a particular blog as they are in wp-admin/users.php.

comment:10 @markjaquith4 years ago

Ryan, inclined to say is_network_admin().

comment:11 @nacin4 years ago

Agreed.

@SergeyBiryukov4 years ago

comment:12 @SergeyBiryukov4 years ago

Replaced with is_network_admin().

comment:13 @westi4 years ago

Why are we burying the logic deep in the searching code rather than where it is called.

A plugin may want to do something using the searching code but allowing the banned search types and will now have to duplicate the code.

comment:14 @SergeyBiryukov4 years ago

Perhaps we should add wide as a parameter of WP_User_Query?

comment:15 @ryan4 years ago

Exposing wild = 'trailing|leading|both' in WP_User_Query sounds good. Let the callers decide what they want. Or, handle * in search strings down in get_search_sql() and skip the args, leaving it up to callers to add leading and trailing *.

comment:16 @ryan4 years ago

WP_User_Query::prepare_query() trims leading and trailing asterisks and passes wild = trailing | leading | both to get_search_sql() accordingly. WP_Users_List_Table::prepare_items() always adds leading and trailing asterisks to the search term. WP_MS_Users_List_Table::prepare_items() allows user to pass a trailing asterisk in the search and strips all other asterisks.

@SergeyBiryukov4 years ago

comment:17 @SergeyBiryukov4 years ago

Updated the patch for the latest suggestion.

comment:18 @ryan4 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [17189]) Default to leading and trailing wildcards for site user searches. Require explicit trailing wildcard asterisk request for network user searches. Disallow leading wildcards for network user searches. Move wildcard policy up the stake, allowing more flexibility in WP_User_Query. Props SergeyBiryukov. fixes #16014

comment:19 @ryan4 years ago

s/stake/stack/

Note: See TracTickets for help on using tickets.