There is code in the UserSearch plugin that is attempting to make it possible to search for users using * wildcards:
$keys = $this->keywords;
// Replace wildcards with MySQL/PostgreSQL wildcards.
$keys = preg_replace('!\*+!', '%', $keys);
However, this doesn't work (I've verified this in a manual test by searching for "jho*n" on a site with user "jhodgdon" defined), because later on the whole keyword query is surrounded by % so you are looking for a substring, and the % added above is escaped:
$query->condition('name', '%' . $this->database->escapeLike($keys) . '%', 'LIKE')
D7 has the same problem [verified]. The code there is in user.module :: user_search_execute():
$keys = preg_replace('!\*+!', '%', $keys);
...
$query->condition('name', '%' . db_like($keys) . '%', 'LIKE')
D6 does not have the problem [verified], because there the code (in user_search() in user.module) is:
$keys = preg_replace('!\*+!', '%', $keys);
...
$result = pager_query("SELECT name, uid, mail FROM {users} WHERE LOWER(name) LIKE LOWER('%%%s%%') OR LOWER(mail) LIKE LOWER('%%%s%%')", 15, 0, NULL, $keys, $keys);
so there is no escaping of the % in D6.
Comments
Comment #1
jhodgdonComment #2
jhodgdonI just tested d6 and d7 and verified this problem exists in D7 but it works fine in D6. So this is a regression from Drupal 6. Should actually be easy to fix, and easy to add a test for.
Comment #3
jhodgdonWas easy to fix. Here's a test-only patch that should have two failures on the new wildcard search lines, and a patch with also the code fix. Works locally...
Comment #5
jhodgdonTest-only patch failed as intended, fix + test patch passed, let the reviews begin, please! :)
Comment #6
mgiffordI can conform that your patch works when searching users search/user?keys=ad*m
I hoped that it would also work when filtering on the admin side admin/people?user=ad*m&role=All&permission=All&status=All
I just searched on "ad*m" as it was always there and was easiest.
EDIT: Looks good to me!
Comment #7
jhodgdonThe admin page is a view. It's not using the UserSearch plugin, and has nothing to do with this patch. This patch fixes a regression in Search.
So so you think this is RTBC?
Comment #8
mgiffordThanks for the clarification about the admin page. And yes..
Comment #9
jhodgdonUnassigning to avoid "jhodgdon is committing this" confusion in the RTBC queue.
Comment #10
alexpottCommitted 5385c22 and pushed to 8.0.x. Thanks!
Comment #13
pietmarcus CreditAttribution: pietmarcus as a volunteer commentedI'm not sure if backports are still accepted for 7.x, but I'll backport it anyway, if only for learning how to help with development of Drupal
Comment #14
jhodgdonBackports for bug fixes are definitely still being accepted for Drupal 7, and in some cases, even feature releases. Thanks!
Comment #15
pietmarcus CreditAttribution: pietmarcus as a volunteer commentedAdded a test-only patch (which will fail) and the patch with the test and fixed code.
Comment #17
pietmarcus CreditAttribution: pietmarcus as a volunteer commentedTest only patch failed as intended, fix passed. Please review.
Comment #18
jhodgdonThanks! This looks good.
The test-only patch had some weird unrelated failures, so I'm going to retest it after I submit this comment.
There's also a very small problem with the patch:
Extra space at the end of this line. You should be able to set up your text editor to either delete or highlight end-of-line spaces to avoid this.
So please make a new patch. You don't need to make a new test-only patch. Thanks!
Comment #20
jhodgdonUgh. That retest of the test-only patch was even worse. I'll try again. I was not aware the testing platform was so unstable in D7!
Comment #22
pietmarcus CreditAttribution: pietmarcus as a volunteer commentedUgh, I thought I had my IDE set up correctly. Sorry for that. Settings are fixed, and so is the patch. Thanks for the headsup!
Comment #23
pietmarcus CreditAttribution: pietmarcus as a volunteer commentedI'm on a roll here... Please disregard patch #22, it has some local settings to make sure PHP can finish on my machine. This is the correct one.
Comment #24
jhodgdonThis looks good now, thanks!
However I just noticed that the test in your patch is not as comprehensive as the test in the D8 patch, which tested wildcards in email for admins (as yours does) but also wildcards in user name (which yours doesn't). Could you add that test too (and a new test-only patch)? Thanks!
Comment #25
pietmarcus CreditAttribution: pietmarcus as a volunteer commentedNew test added. Passed locally (with patch).
Comment #28
pietmarcus CreditAttribution: pietmarcus as a volunteer commentedFailed tests have nothing to do with my fixes. Resubmitting for testing
Comment #29
jhodgdonYeah, the D7 testing system seems to be a bit unpredictable. I wouldn't bother to resubmit again if you see test failures. They will eventually sort themselves out, hopefully anyway.
Anyway, the latest patch looks good to me. Thanks for the quick work on this! Hopefully it will get committed to D7 sometime soon.
Comment #30
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedCommitted to 7.x - thanks!
Fixed on commit: