[HttpKernel] Normalized container classname in kernel #27029
Conversation
| Q | A |
|---|---|
| Branch? | 2.7 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #25915, #26884 |
| License | MIT |
| Doc PR |
|
I agree with @nicolas-grekas |
|
Done, but shouldn't we allow digits too? |
|
Actually, did it ever work? |
| @@ -480,7 +480,7 @@ protected function initializeBundles() | |||
| */ | |||
| protected function getContainerClass() | |||
| { | |||
| return $this->name.ucfirst($this->environment).($this->debug ? 'Debug' : '').'ProjectContainer'; | |||
| return $this->name.ucfirst(preg_replace('/[^a-zA-Z0-9_]/', '', $this->environment)).($this->debug ? 'Debug' : '').'ProjectContainer'; | |||
There was a problem hiding this comment.
the regex about valid chars in class names is broader than that
There was a problem hiding this comment.
The official regexp for class names is ^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$ (see http://php.net/manual/en/language.oop5.basic.php) so this part should be [^a-zA-Z0-9_\x7f-\xff] instead of [^a-zA-Z0-9_] ... but I don't think we should do this change. The probability of causing a clash because two weird env names have been stripped from wrong characters is negligible.
|
What are the next steps for this one? /cc @xabbuh |
|
We will need to agree on the regex to be used. It looks like @nicolas-grekas and @stof have different opinions on what would be the best we should use. |
|
This reminds me #13412 |
|
Re-reading, I'm in favor of throwing an exception, since it never worked. |
|
If we were to merge this one, I think it should be on master. |
|
I'm closing since getName has been deprecated on master meanwhile. Thanks for helping move the discussion forward! |