Fix user_is_email_unique() on case-sensitive DB #1899
Conversation
When the mantis_user_table's email column is case-sensitive, the user_is_email_unique() function incorrectly returned true if the given e-mail only differed by case (e.g. test.user@example.com vs Test.User@example.com) Fixes #32451
- Case insensitive user cache search - Use DbQuery::sql_ilike() for database search Fixes #32451
| @@ -288,18 +291,20 @@ function user_is_email_unique( $p_email, $p_user_id = null ) { | |||
| } | |||
|
|
|||
| $p_email = trim( $p_email ); | |||
| // Escape SQL LIKE pattern chars to ensure exact match | |||
| $p_email = preg_replace( '/([%_])/', '\\\\$1', $p_email ); | |||
There was a problem hiding this comment.
The escaping for SQL LIKE should be part of the implementation for $t_query->sql_ilike function. In this PR it is done before each call.
There was a problem hiding this comment.
The escaping for SQL LIKE should be part of the implementation for $t_query->sql_ilike function
No we don't want that, the replacement has to be performed by the caller.
If we did it in DbQuery::sql_ilike(), then the function would no longer interpret LIKE wildcards % or _, effectively turning it into a case-insensitive = operator (which basically what I'm going after here).
| self::$user_id = user_get_id_by_cookie( $t_cookie ); | ||
| } | ||
|
|
||
| public static function tearDownAfterClass() { |
There was a problem hiding this comment.
Should you call parent::tearDownAfterClass() similar to calling parent::setUpBeforeClass() above?
There was a problem hiding this comment.
Technically yes I guess, but the method is not defined in the parent, so it would just be unnecessary overhead
|
|
||
| $t_query = new DbQuery; | ||
| $t_query->sql( 'SELECT * FROM {user} WHERE ' | ||
| // Case-insensitive like |
There was a problem hiding this comment.
Is there a reason the fetching from cache is configurable for case insensitive vs. not, and here it is always case insensitive?
There was a problem hiding this comment.
I'm not sure I understand what you're asking.
When searching a user Id by e-mail, we always want the search to be case insensitive, this is what this PR is changing.
The user cache contains other data, which is looked-up in a case-sensitive manner. So user_search_cache()'s new $p_case_sensitive parameter defaults to true ensuring unchanged behavior for existing usages, while allowing insensitive searches for email.
Does this answer your question ?
|
@vboctor without further feedback from you, I assume you're satisfied with my replies to your review comments and plan to merge this tonight or tomorrow. |
Fixes #32451
This PR follows #1898 and @atrol's remark questioning the target version for #32451 being 2.25.8.
This is splitting the original PR, to target the bug fix at master-2.25 branch, while the enhancement will be merged into master branch only