[Routing][AnnotationClassLoader] fix utf-8 encoding in default route name

[przemyslaw-bogusz]

Q	A
Branch?	3.4
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
License	MIT

If controller, or one of its methods, contain a unicode character and you run:

./bin/console debug:router

you get this:

This PR fixes it into this:

[Tobion]

I don't see why the Ä got scrambled. strtolower should just not lowercase it but keep it as-is. See https://3v4l.org/scUGN
Also this fix could be considered a BC break because it might change the route names of existing apps.

[nicolas-grekas]

I don't see why the Ä got scrambled

I'd suspect because strtolower is locale-sensitive.

We could have something like:

$name = str_replace('\\', '_', $class->name).'_'.$method->name;
$name = \function_exists('mb_strtolower') && preg_match('//u', $name) ? mb_strtolower($name, 'UTF-8') : strtolower($name);

[przemyslaw-bogusz]

@Tobion That's very interesting. From my experience, strtolower breaks a string with unicode characters. I guess it indeed depends on locale.

As for the BC break - keeping in mind that each bug fix can be considered a BC break - this method is used only when a route has no name. So, it could potentially be a problem for someone, who uses routing but does not name the routes.

[przemyslaw-bogusz]

@nicolas-grekas As I understand, you want to check if the string contains a unicode character? preg_match('//u', $name) returns true even on an empty string - https://3v4l.org/VLNrp. In my experience, comparing strlen to mb_strlen is a good way to do it. But couldn't we just check if mb_strtolower exists and apply it?

[nicolas-grekas]

But couldn't we just check if mb_strtolower exists and apply it?

that would break if any other encoding is used - mb requires knowing the charset before using it.

[przemyslaw-bogusz]

preg_match('//u', $name) as a way to validate encoding - I haven't thought about that one. Motto for this week: 'Learning while contributing'.

[nicolas-grekas]

LGTM, could you add a test case please?

[nicolas-grekas]

The tests are failing on appveyor. Windows doesn't like unicode names... We should find a test case that doesn't rely on the filesystem willingness :)

[przemyslaw-bogusz]

I've changed the name of the fixture class and now all tests pass. By the way, I think there might be a few more similar unicode issues hidden inside the Symfony code, e.g. method underscore of Container.

[fabpot]

Thank you @przemyslaw-bogusz.