[HttpKernel] Container class name should start with letter or underscore #20750
Conversation
| @@ -479,7 +479,12 @@ protected function initializeBundles() | |||
| */ | |||
| protected function getContainerClass() | |||
| { | |||
| return $this->name.ucfirst($this->environment).($this->debug ? 'Debug' : '').'ProjectContainer'; | |||
| $name = $this->name.ucfirst($this->environment).($this->debug ? 'Debug' : '').'ProjectContainer'; | |||
| if (!preg_match('/^[a-zA-Z_\x7f-\xff]/', $name)) { // class name must start with a letter or underscore | |||
There was a problem hiding this comment.
Instead, what about always adding a static prefix (like Symfony)? That would avoid executing this regex.
There was a problem hiding this comment.
wouldn't it be somehow considered as BC break? the default name (e.g. appDevDebugContainer) is used by e.g. symfony plugin for phpstorm?
There was a problem hiding this comment.
To avoid the regex, it can be replaced by the following:
if(false === stripos($name[0], 'abcdefghijklmnopqrstuvwxyz_'))There was a problem hiding this comment.
@jzawadzki what about static prefix app then?
| return '_'.$name; | ||
| } | ||
|
|
||
| return $name; |
There was a problem hiding this comment.
to be consistent with the method name getContainerClass(), what do you think about to rename this one to $class ?
|
Note that the regex will be executed twice (at least) per request. I would like to avoid that. |
|
Would it be acceptable to leave public function getName()
{
if (null === $this->name) {
$dirName = preg_replace('/[^a-zA-Z0-9_]+/', '', basename($this->rootDir));
$this->name = false !== strpos($dirName[0], '0123456789') ? '_'.$dirName : $dirName;
}
return $this->name;
} |
|
this Pr is a partial fix only. The dumped classes for the cached router also use the kernel name as prefix, and so suffer from the same issue, which is not fixed here. |
| $kernel->__construct('dev', false); | ||
|
|
||
| $kernel->boot(); | ||
| $this->assertInstanceOf('Symfony\Component\DependencyInjection\ContainerInterface', $kernel->getContainer()); |
There was a problem hiding this comment.
would be much better to write the test in a way which does not trigger a fatal error on failure, as this forbids running other tests and to get the proper PHPUnit reporting.
|
Closing in favor of #21883 |
…arting with a number (fabpot) This PR was merged into the 2.7 branch. Discussion ---------- [HttpKernel] fix Kernel name when stored in a directory starting with a number | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20489 | License | MIT | Doc PR | - replaces #20750 Commits ------- f244eb8 [HttpKernel] fixed Kernel name when stored in a directory starting with a number
Hi all,
this PR tries to fix (rather rare) issue when generated Container Class name is starting with number (which is the case when kernel name [base name of root dir] starts with one).
Not sure if my solution is correct - it's the easiest I could think off: it just adds underscore in front of the generated class name.