Jump to content


Issue information

  • #035743

  • Confirmed - Minimal Impact

  • 3.2.3

  • -

  • 0 - None Assigned


Issue Confirmations

  • Yes (0)No (0)
Photo

strtolower character

Posted by Jason H on 06 February 2012 - 10:44 PM

I've got my locales set right.. using a UTF8 locale.. But..

User had a username of Úmarth

Well, when they tried to add them to a PM Conversation, it would fail, no such user.. Normally, when I see this, it's because the members_l_username/display_name are actually uppercase, or the locale is wrong.. Whatever... Simple fix..

Well, this one.. the members_l tables were 'right', in that they were lowercase.. However.. var dumping strtolower($name) in messengerFunctions.php in this..

			if ( ! in_array( strtolower( $name ), array_keys( $inviteArrayByName ) ) )
			{
				$doesNotExist[] = $name;
			}

Returned Úmarth

Now, I registered that name on my forum.. Both the members_l fields stored the name as Úmarth, not lower case.

Is that.. Just some super funky character? Because, no matter the locale, I couldn't get it to strtolower properly.

hrmmm.... if i use this function on that character precisely I get a lowercase counterpart ú with mb_* installed in php.... seems more appropriate.. :hmm:
if ( ! in_array( IPSText::mbstrtolower( $name ), array_keys( $inviteArrayByName ) ) )
			{
				$doesNotExist[] = $name;
			}


changed status to: Awaiting Feedback

Jason: pass up the ticket to T2 please.

Marcher: we can't use mb_* or it would break in all the other areas where we use strtolower, there's a FFFV report about that but it requires to change it everywhere.


Not to mention not everyone has mb* functions


View Postbfarber, on 07 February 2012 - 10:03 AM, said:

Not to mention not everyone has mb* functions
....confused.... core.php IPSText::mbstrtolower($text) :twitch:
.... it does have a fallback to strtolower in non-enabled environments.... and I would think any bug-fixing done for the bug mentioned here re strtolower itself would need to be applied to that function in that scenario anyway... :sweat:


Ah, ok, that's different from what I was talking about (and you're right - that method takes this into account).  However, as Giuseppe mentioned, there are many areas of the software that expect simply strtolower() values in this field, which may not be the same as mb_strtolower(), so we can't just up and change it.

Long term I would prefer to use the SQL LOWER() function on the field, as MySQL can take into charsets itself and we can rely on consistent values regardless of PHP configuration.


I can't get the ticket back.. Apparently it was replied to and someone else closed it.. So.. It's not in my recent replies, and I couldn't even tell you the site name now. Though.. I can duplicate the problem on my server in a way..

Most likely, they were set to a non-UTF8 locale to start with, and I (or someone) had to run a LOWER(name) on the members_l_ fields.. Which 'fixed' them.. but, broke this specific user who has that character as the first letter of their name uppercase.

I did test with mb_strtolower, and.. As expected, that worked, but.. I didn't leave that in place because it's not really a valid 'fix'.. I wound up setting the members_l_ tables to use the uppercase letter. Not a 'fix', but, it'll work for them for now. Just.. Create a member with that name. Should be simple enough to duplicate.


changed status to: Cannot Reproduce

Posted Image

Works fine locally. Probably a charset thing - if it happens again, send it up.


Based on your screenshot, I imagine you registered the user and tried to add them to a PM.. yes.. that does work, as I explained above, if you register that user on your forum, the members_l_ tables are stored with the name as Úmarth, which would then match the strtolower, because, when a user registers, the username is sent through strtolower.. So, what you have, and what your testing proves, is two wrongs making a right..

This, is a better way to look at what's happening.

http://screencast.com/t/uIMtqQE09FX

And.. Actually, I should add more to that.. Take the above, and add this into it..

http://screencast.com/t/IEQ5nO0F

That.. Doesn't PERFECTLY show it, because of the way my database collation/charset is setup, but.. It does, in a way.

running Úmarth through strtolower and through the MySQL LOWER command produces two different results.

Now.. On a flat out test like you did.. It'll work. There's a difference between working and being right.. This is not right, but it does work. I can take a car with a plugged up catalytic converter, bust the catalyst out, and put it back on.. It'll WORK, so far as the car as a whole.. but, the emissions system won't work, and if you live somewhere with emissions testing, they'll tell you that it's not RIGHT.

The only two places that this will come in as a problem is on an upgrade from IPB2 where a user with that "U" character existed on IPB2.

When you hit the function in mysql_updates_3.php of upg_22005

$SQL[] = "UPDATE members SET members_l_display_name=LOWER(members_display_name);";
$SQL[] = "UPDATE members SET members_l_username=LOWER(name);";

That's going to set it as umarth (With the right lower case u character which i'm not searching for) and when you try to send a PM to that user in IPB3.. It's going to fail, because strtolower will return it as Úmarth

OR.. If you get a site that has been using the default en_US, or maybe it_IT or de_DE charsets, and a user with that name registers. The way to fix that is to set the charset to en_US.utf8 (or whatever) and then run the above MySQL commands to lower case the "L" fields in members. If you do that.. ANY user with Ú in their username will be unable to be added to a PM conversation.

Now, if you want to say "Ok, yes, it happens, but.. BFD.." I'm fine with that. I mean, this is a pretty dang rare scenario that's going to happen most likely. But, this needs to at least be understood, because, if it happens with ONE character.. Who's to say it doesn't or won't happen with OTHER characters?


changed status to: Confirmed - FAO Matt

Right, I'm with you now...

My thinking is to re-evaluate how we handle stuff like this in the next major update, but I'll pass over to Matt so he can make a call.


changed status to: Confirmed - Minimal Impact

Yes, I agree. We need to either allow MySQL exclusively use LOWER() or exclusively use a MB safe strtolower method.

I'm reluctant to do it now, though so it's something we can add in for 3.4 I think.






1 user(s) are reading this issue

0 members, 1 guests, 0 anonymous users