Skip to content

[DependencyInjection] Container::has doesn't handle ids with backslashes #9801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
FOODy opened this issue Dec 17, 2013 · 1 comment
Closed

[DependencyInjection] Container::has doesn't handle ids with backslashes #9801

FOODy opened this issue Dec 17, 2013 · 1 comment
Labels
DependencyInjection

Comments

@FOODy
Copy link

FOODy commented Dec 17, 2013

The has-method doesn't handle service ids with backslashes. (e30a7d0)

It could be easily fixed with an additional isset($this->methodMap[$id]) check:

    public function has($id)
    {
        $id = strtolower($id);

        return isset($this->services[$id])
            || array_key_exists($id, $this->services)
            || isset($this->aliases[$id])
            || isset($this->methodMap[$id])
            || method_exists($this, 'get'.strtr($id, array('_' => '', '.' => '_')).'Service')
        ;
    }

or a modified method_exists-line:

    public function has($id)
    {
        $id = strtolower($id);

        return isset($this->services[$id])
            || array_key_exists($id, $this->services)
            || isset($this->aliases[$id])
            || method_exists($this, 'get'.strtr($id, array('_' => '', '\\' => '_', '.' => '_')).'Service')
        ;
    }

Best regards!
@cordoval
Copy link
Contributor

ping @fabpot is this worth implementation or was it decided against supporting \ on service ids ?

@jakzal jakzal mentioned this issue Dec 20, 2013
Merged
fabpot added a commit that referenced this issue Dec 21, 2013
@fabpot
bug #9834 [DependencyInjection] Fixed support for backslashes in serv…
1ecfd44 
…ice ids. (jakzal)

This PR was merged into the 2.3 branch.

Discussion
----------

[DependencyInjection] Fixed support for backslashes in service ids.

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #9801
| License       | MIT
| Doc PR        |

This change is needed for consistency with `camelize()` which is used in [`ProxyDumper`](https://github.com/symfony/symfony/blob/2.3/src/Symfony/Bridge/ProxyManager/LazyProxy/PhpDumper/ProxyDumper.php#L69) and [`PhpDumper`](https://github.com/symfony/DependencyInjection/blob/2.3/Dumper/PhpDumper.php#L1275).

Either this PR needs to be merged for consistency or #9610 rolled back (if we don't want to support backslashes in service ids).

Anyone could tell me why we're not using the `camelize()` method internally in the `Container`?

Commits
-------

c6f210b [DependencyInjection] Fixed support for backslashes in service ids.
@fabpot fabpot closed this as completed Dec 21, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DependencyInjection
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@fabpot @FOODy @cordoval