justin_ruch1180 wrote:
WHEW.. This is complicated for me...:(
well justin_ruch1180 did you registered just for posting that ? Actually, this sub forum is for developpers and about technical issue, so it's perfectly normal if you don't understand : it's not for you ^^
Offline
I noticed [Bugtracker] ticket 1835 is now fixed, so I cant reply there anymore.
However, I have a problem with the solution as it is applied in [Subversion] r10860. The solution make a query to retrieve every user from the database. While this solves the problem indeed, it's also a huge performance issue IMHO.
I suggest to change the query into this:
$q = pwg_query(" SELECT ".$conf['user_fields']['username']." AS username FROM `".USERS_TABLE."` WHERE `".$conf['user_fields']['username']."` = \"".pwg_db_real_escape_string($username_lo)."\"; ");
In order to only retrieve the usernames we are interested in.
With this change I would be happy, even though the test for ambiguity can be rewritten as well.
-Tim
Offline
the next time I'll close some time after ^^
You have right : the code isn't optimize at all
Offline
I agree the code can be improved.
I would write it:
/** * For test on username case sensitivity * * @param : $username typed in by user for identification * * @return : $username found in database or same as parameter if ambiguous * */ function search_case_username($username) { global $conf; $esc_username = pwg_db_real_escape_string(strtolower($username)); $q = pwg_query(" SELECT `".$conf['user_fields']['username']."` AS username FROM `".USERS_TABLE."` WHERE LOWER(`".$conf['user_fields']['username']."`) = \"".$esc_username."\"; "); $r = pwg_db_fetch_assoc($q)); if(count($r) == 1) { return $r[0]['username']; }else{ return $username; } }
(Added some missing "`" in the query.)
The only thing is, that it needs that MySQL LOWER() function works the same way as PHP strtolower() function. If it is the case (which is quite likely I think), then it's perfect.
Indeed the code I had proposed is not optimized; what I can say is that, for [extension by Eric] LCAS, I tested some iterative PHP code cycling on all the usernames of a set of more than 2000 user, it was staying below 15 s of processing time, thus was it not crashing the code.
But of course whe should always prefer an optimized code (for LCAS it is not possible).
Offline
Hi,
I updated my code in [Bugtracker] ticket 1835, there was another thing with count() and pwg_db_fetch_assoc($q) I messed up.
Yes, I rely on LOWER and str_to_lower to do the same thing. I don't know if they do...
But we can make sure to rely on LOWER only:
$q = pwg_query(" SELECT `".$conf['user_fields']['username']."` AS username FROM `".USERS_TABLE."` WHERE LOWER(`".$conf['user_fields']['username']."`) = LOWER(\"".pwg_db_real_escape_string($username)."\"); ");
The other option is to do it backwards ;).
Query all the users that match the hashed password, and go look for case ambiguities there... Then you can rely on str_to_lower only. But that would freak every-bodies mind out.
-Tim
Offline
the problem is for special uppercase :
quoting from http://www.php.net/manual/fr/function.strtolower.php :
<?php strtolower("mĄkA"); ?>
will return: mĄka;
the best solution - use mb_strtolower()
<?php mb_strtolower("mĄkA",'UTF-8'); ?>
will return: mąka
LOWER use the current character set so I think it's better
Offline
So, bringing it together (using LOWER) it would become:
/** * For test on username case sensitivity * * @param : $username typed in by user for identification * * @return : $username found in database * */ function search_case_username($username) { global $conf; $q = pwg_query(" SELECT `".$conf['user_fields']['username']."` AS username FROM `".USERS_TABLE."` WHERE LOWER(`".$conf['user_fields']['username']."`) = LOWER(\"".pwg_db_real_escape_string($username)."\"); "); if(pwg_db_num_rows($q) == 1) { $row = pwg_db_fetch_assoc($q); return $row['username']; }else{ return $username; } }
Offline
stim wrote:
I updated my code in [Bugtracker] ticket 1835, there was another thing with count() and pwg_db_fetch_assoc($q) I messed up.
Yes, it seems better this way :-) .
stim wrote:
Yes, I rely on LOWER and str_to_lower to do the same thing. I don't know if they do...
But we can make sure to rely on LOWER only:Code:
$q = pwg_query(" SELECT `".$conf['user_fields']['username']."` AS username FROM `".USERS_TABLE."` WHERE LOWER(`".$conf['user_fields']['username']."`) = LOWER(\"".pwg_db_real_escape_string($username)."\"); ");
This one seems fine to me.
stim wrote:
The other option is to do it backwards ;).
Query all the users that match the hashed password, and go look for case ambiguities there... Then you can rely on str_to_lower only. But that would freak every-bodies mind out.
Yes :-D ! I prefer previous solution -- but we'll see what will be chosen at the end; I'm not part of the team.
flop25 wrote:
the problem is for special uppercase
Indeed! That's one of the reason for [extension by Eric] LCAS to exist.
flop25 wrote:
the best solution - use mb_strtolower()
It does be quite a solution; yet mb_strtolower() is provided by PHP extension mbstring, thus is there a risk that on some servers this extension is not active. In this case mb_strtolower() generates a notice. For LCAS Whiler proposed a code allowing to get free from mbstring:
/** * Split a multi-byte string in a array of each of its characters. * Many thanks to Whiler ;-) in * http://fr.piwigo.org/forum/viewtopic.php?pid=162907#p162907 * * @param string s: string to split * * @return : array of each character of s */ function LCAS_mbStringToArray($s) { return preg_split('//u', $s, -1, PREG_SPLIT_NO_EMPTY); }
Offline
@luc that's why using LOWER only is better because it's multi-byte safe.
Offline
flop25 wrote:
@luc that's why using LOWER only is better because it's multi-byte safe.
OK, perfect, then :-)) .
Offline
@all : we can't use LOWEr because of the compatibility between the type data base : Postgresql/MySQL/SQLite
Moreover it's a feature which is false by default, and concern gallery with a huge amount of users : it's too rare !
So the first initial commit will stay
Offline
Lower is actually supported in all three.
flop25 wrote:
it's too rare !
You just made sure of that. With this performance issue, I guess no-one who notices it will use this.
I'm almost inclined to go for the backward way, if LOWER is out of the question. :-S
But this commit just takes away all scalability. If it is really that rare that this performance decrease is to be permitted, I don't believe it should be configurable.
-Tim
Offline
stim wrote:
Lower is actually supported in all three.
flop25 wrote:
it's too rare !
You just made sure of that. With this performance issue, I guess no-one who notices it will use this.
I'm almost inclined to go for the backward way, if LOWER is out of the question. :-S
But this commit just takes away all scalability. If it is really that rare that this performance decrease is to be permitted, I don't believe it should be configurable.
-Tim
I don't catch everything you mean, and what you intend to do if the code stays as on last commit.
@both: I'm not skilled enough in DBs to be able to answer about the LOWER() function in the different DB languages. Can you both give links here showing the reason why you say the LOWER() function can/cannot be used?
Anyway what I can say, is that personally speaking, I would trust the code committed by flop25. I don't beleive it would generate issues.
But I'm OK to change my mind, if it is necessary :-) !
Offline
Lower in sqlite:
http://www.sqlite.org/lang_corefunc.html
mysql
http://dev.mysql.com/doc/refman/5.0/en/ … tion_lower
postgres:
http://developer.postgresql.org/pgdocs/ … tring.html
General:
http://oreilly.com/catalog/sqlnut/chapt … html#27402
I mean to say that I strongly disagree with this solution because it is not scalable at all. Therefore, I don't like to settle with the last commit. Because: if it is not scalable, it's not a actually a feature of the software, since (power)users will be obliged to turn it off eventually, as their traffic grows(*).
I can understand that it might be undesirable to rely on 'LOWER' (if there exists considerable doubt about its performance). However, that shouldn't be an excuse not to make piwigo scalable.
Therefore I (jokingly) suggested to apply the 'backward' method I shared earlier. (Since, although it is backward in every sense of the word, it is scalable and can solve the issue.)
(*) I don't want to be harsh or anything. And I do see how this might never become an issue for the average user. But it's just bad quality of software to leave it unscalable like this, and I don't believe it's something that can't be solved.
I was very happy to find a community/album that fitted my needs (I've looked for it several times but never found anything good), and I'd like to continue to be of good quality if I'm going to use it. :)
Last edited by stim (2011-05-13 20:45:49)
Offline
thx for the doc. I read and didn't see any problem -except one for PostgreSQL but it's 10 years old ...-
I'll do some test with mysql and special character with LOWER.
Offline