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

AddExpressionLanguageProvidersPass breaks custom routers #37042

Closed
wizhippo opened this issue Jun 1, 2020 · 2 comments
Closed

AddExpressionLanguageProvidersPass breaks custom routers #37042

wizhippo opened this issue Jun 1, 2020 · 2 comments

Comments

@wizhippo
Copy link
Contributor

wizhippo commented Jun 1, 2020

Symfony version(s) affected: 5.1

Description
Previously in 5.0 there were no services tagged with routing.expression_language_provider so the issue did not arise. There is in 5.1 so the issue can occur.

addExpressionLanguageProvider is not part of the router interface. As such AddExpressionLanguageProvidersPass should not use it if the router does not support it as the router could be replaced with a custom router that does not support the function.

$definition->addMethodCall('addExpressionLanguageProvider', [new Reference($id)]);

How to reproduce
Example ezplatform replaces the router.default with its own router which does not implement addExpressionLanguageProvider nor should it have to if it is not part of the interface.

Possible Solution
Check it router supports addExpressionLanguageProvider first.
Make addExpressionLanguageProvider part of the router interface.
Apply to router.default instead of router and stipulate route.default has to be the symfony router or extended class.

@nicolas-grekas
Copy link
Member

Would you mind sending a PR, branch 5.1?
An is_a() check might do it, isn't it?

@wizhippo
Copy link
Contributor Author

wizhippo commented Jun 1, 2020

Personally I think the pass should target router.default instead of router. This would fix the issue I am seeing. It is router that normally would be replaced and router.default is added to the chain_router which replaces router

wizhippo added a commit to wizhippo/symfony that referenced this issue Jun 1, 2020
Using a `chain_router` usually replaces the `router` and add the `router.default` to it's chain.

This would `addExpressionLanguageProvider` to the default router only as the chain router is not expected to have `addExpressionLanguageProvider` as it is not part of the router interface.

See symfony#37042
wizhippo added a commit to wizhippo/symfony that referenced this issue Jun 1, 2020
Using a `chain_router` usually replaces the `router` and add the `router.default` to it's chain.

This would `addExpressionLanguageProvider` to the default router only as the chain router is not expected to have `addExpressionLanguageProvider` as it is not part of the router interface.

See symfony#37042
@fabpot fabpot closed this as completed Jun 11, 2020
fabpot added a commit that referenced this issue Jun 11, 2020
… to router.default (wizhippo)

This PR was squashed before being merged into the 5.1 branch.

Discussion
----------

[DependencyInjection] Apply ExpressionLanguageProviderPass to router.default

| Q             | A
| ------------- | ---
| Branch?       |  5.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #37042
| License       | MIT

Using a `chain_router` usually replaces the `router` and add the `router.default` to it's chain.

This would `addExpressionLanguageProvider` to the default router only as the chain router is not expected to have `addExpressionLanguageProvider` as it is not part of the router interface.

Commits
-------

215ad1f [DependencyInjection] Apply ExpressionLanguageProviderPass to router.default
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this issue Jun 11, 2020
… to router.default (wizhippo)

This PR was squashed before being merged into the 5.1 branch.

Discussion
----------

[DependencyInjection] Apply ExpressionLanguageProviderPass to router.default

| Q             | A
| ------------- | ---
| Branch?       |  5.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix symfony/symfony#37042
| License       | MIT

Using a `chain_router` usually replaces the `router` and add the `router.default` to it's chain.

This would `addExpressionLanguageProvider` to the default router only as the chain router is not expected to have `addExpressionLanguageProvider` as it is not part of the router interface.

Commits
-------

215ad1f93d [DependencyInjection] Apply ExpressionLanguageProviderPass to router.default
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