Apostrophe in member name

[nilshoerrmann]

We have a newly created user that's called Director's uncut. While front-end creation worked perfectly, it's neither possible to log into the front-end nor to filter by this name in the backend:

1. The front-end login just fails with the message that the user could not be found, no logs.
2. The backend logs an MySQL syntax error.

The SQL error reads

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 's uncut')

and highlights the following line:

`t104`.value IN ('Director's uncut')

This is obviously invalid.

We are using latest integration with latest Symphony (2.6.11).
Any ideas how to fix this?

/cc @brendo

[nilshoerrmann]

Regarding the first issue – the missing member from the front-end – there seems to be a mismatch between the handles:

The Member: Username field uses this SQL:

SELECT `entry_id` FROM `tbl_entries_data_104` WHERE `handle` = 'director-s-uncut' LIMIT 1

The handle stored in the database is directors-uncut.

[nilshoerrmann]

This is how the handle is built when creating an user:

Lang::createHandle(trim($data))

This is how it's created when trying to login:

Lang::createHandle($username)

trim() shouldn't remove an apostrophe, should it?

[nilshoerrmann]

The only thing I can think of that might result in different apostrophe handling, are different transliteration. I deactivated the German localisation and recreated the user with the same effect – so that shouldn't be the cause.

[nitriques]

@nilshoerrmann add this line here

$data = array_map(array('MySQL', 'cleanValue'), $data);

Same fix would be needed here as well.

The $data array is imploded without being properly escaped. BTW, this makes it possible to do SQL injection...

[nitriques]

trim() shouldn't remove an apostrophe, should it?

No, only " \t\n\r\0\x0B"

[nilshoerrmann]

Any idea regarding the different handle generation?

[nilshoerrmann]

@brendo and @michael-e: What's the reason behind using the handle to fetch the member at all?

$member_id = Symphony::Database()->fetchVar('entry_id', 0, sprintf(
	$this->get('id'), Lang::createHandle($username)
));

See: https://github.com/symphonycms/members/blob/master/fields/field.memberusername.php#L84-L87

Wouldn't that possibly lead to member mismatches because different names might result in identical handles? Why doesn't members use the actual provided $username and compares it to the stored value?

[brendo]

Wouldn't that possibly lead to member mismatches because different names might result in identical handles? Why doesn't members use the actual provided $username and compares it to the stored value?

No it shouldn't because we check this when creating a new username.

@nitriques is correct, the underlying Identity class should be escaping for SQL injection.

What's the reason behind using the handle to fetch the member at all?

We often prefer handle because generally it's shorter (perf) and free from punctuation.

That said, I'm a bit unsure how the handle could be two different things, director-s-cut and directors-cut!

[nilshoerrmann]

Okay, that's good to know.

Still, I don't understand why Members created different handles for the same user. This happens with localisation extensions disabled so there shouldn't be any differences in the transliteration settings. I was thinking that maybe one part uses General::createHandle (without transliteration handling) while the other users Lang::createHandle (with transliteration handling). But I couldn't find anything related in the codebase.

[nilshoerrmann]

Summary

1. Backend filtering can be fixed by escaping the values correctly as mentioned by @nitriques and @brendo.
2. There is still an unknown bug that causes inconsistent handle creation (member creation versus member discovery later on).

[nitriques]

Backend filtering can be fixed by escaping the values correctly as mentioned b

I've fixed this.

There is still an unknown bug that causes inconsistent handle creation

It's not a bug. Lang::createHandle() apply transliterations on the current lang. If you create the user using a certain lang, the front will always be 'en' for users not logged in Symphony.

There are sadly no nice way to

1. Make it retrocompatible OR
2. Use the user's language

Let's continue this in #288