Skip to content
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.

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

[HttpKernel][Bug] ServiceValueResolver fails if controller namespace starts with a backslash in routing. #26772

Closed
mathieutu opened this issue Apr 3, 2018 · 2 comments

Comments

@mathieutu
Copy link
Contributor

mathieutu commented Apr 3, 2018

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 4.0.

Hi,
I've seen a problem when configuring my routes:

Controller "App\Controllers\Foo" requires that you provide a value for the "$foo" argument. Either the argument is nullable and no null value has been provided, no default value has been provided or because there is a non optional argument after this one.

# config/routes.yaml
foo:
    methods: POST
    path: /foo
    controller: \App\Controllers\Foo
// src/Controllers/Foo.php
class Foo
{
    public function __invoke(FooService $foo, Request $request)
    {
        // ...
    }
}

The services.yaml is properly configured: Foo is tagged as a 'controller.service_arguments' and FooService is correctly autowired, and can be seen in php bin/console debug:autowiring.

The service locator is correctly generated in container cache.

When debugging, I've seen that in the \Symfony\Component\HttpKernel\Controller\ArgumentResolver\ServiceValueResolver::supports method, the controller passed to the route ("\App\Controllers\Foo") is searched in container factories (["App\Controllers\Foo" => (...)]) and not found because of the first backslash of the namespace.

The router find the proper controller with this backslash, so in my opinion the ServiceValueResolver should not fail at this point.

The following PR fixes this issue: #26773

Thanks for you work.

@mathieutu mathieutu changed the title [HttpKernel][Bug] ServiceValueResolver fail if controller namespace start with a backslash in routing. [HttpKernel][Bug] ServiceValueResolver fails if controller namespace starts with a backslash in routing. Apr 3, 2018
@xabbuh
Copy link
Member

xabbuh commented Apr 3, 2018

The failure is excepted and I would not change that. Otherwise, this would be the only part of the framework where you could reference a service by something that looks like its ID. I think we should rather improve the error handling here to make it more clear what is going wrong.

@mathieutu
Copy link
Contributor Author

mathieutu commented Apr 3, 2018

@xabbuh The problem is that in the routing you actually can register the controller with this backslash. And if you don't try to inject services directly in method, it will work perfectly.

It fails only when you want to inject the service in the controller method.

If the failure is normal, it has to fail much earlier and always, during the routing with a Controller not found, and not during the argument resolving phase just for some cases.

nicolas-grekas added a commit that referenced this issue Apr 14, 2018
…namespace starts with a backslash in routing (mathieutu)

This PR was submitted for the 4.0 branch but it was squashed and merged into the 3.4 branch instead (closes #26773).

Discussion
----------

[HttpKernel] Make ServiceValueResolver work if controller namespace starts with a backslash in routing

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #26772
| License       | MIT

Hi folks,

This PR fixes #26772:
>Controller "App\Controllers\Foo" requires that you provide a value for the "$foo" argument. Either the argument is nullable and no null value has been provided, no default value has been provided or because there is a non optional argument after this one.

Thank you for your work!

PS: This is my first (of many planned!) PR to Symfony, so please let me know if I did something wrong.

Commits
-------

3b47441 [HttpKernel] Make ServiceValueResolver work if controller namespace starts with a backslash in routing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants