FIX: Class names referenced in Config should be case-insensitive #3381
Conversation
The config system maps top-level key name to a PHP class. PHP classes are case-insensitive, but without this patch, the Config system was case-sensitive on this variable. I think this should be considered a bug. The following patch makes the "class" key case-insensitive, but leave the "name" key, as case-sensitive, mirroring the semantics of PHP classes and static variables, respectively. Arguably, the entire config system could be case-insensitive, but I haven't done that in this patch.
|
I'd be keen to get @hafriedlander input on this, since he is God of The Config System. |
|
+1 for this @hafriedlander I think this is a bug fix and can stay against 3.1 |
|
How do you feel about using a case-fixing mechanism instead of strtolower? See https://github.com/micmania1/silverstripe-framework/blob/replace-static-manifest/core/ClassInfo.php#L163 for @micmania1 's solution. Is there a place we could strategically place this in the code so that we have to do as few case corrections as possible? Places such as get_called_class() automatically normalise class casing, so there are certainly times it's not necessary. I personally feel that storing the config in the correct case is going to deviate the least amount from what the current behaviour is, and is less likely to introduce regressions. |
|
The issue with my solution is that it only resolves class names - not made up configuration in yaml. I'd be able to remove my fix if it was agreed that this is the best way to go. |
|
fixes #3378 |
|
|
|
Maybe this would be better in 3.2 instead, just because of the regression risk issue that Damian pointed out. |
|
very much related: #3949 |
|
I think this is a patch and should stay in 3.1 myself. Under what circumstances could this cause a problem to existing code? |
|
Superceded by new config api, which stores all per-class config with a lowercase key https://github.com/silverstripe/silverstripe-config/blob/1.0/src/Collections/MemoryConfigCollection.php#L82 |
The config system maps top-level key name to a PHP class. PHP classes are case-insensitive,
but without this patch, the Config system was case-sensitive on this variable. I think this
should be considered a bug.
The following patch makes the "class" key case-insensitive, but leave the "name" key, as
case-sensitive, mirroring the semantics of PHP classes and static variables, respectively.
Arguably, the entire config system could be case-insensitive, but I haven't done that in
this patch.