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

Class-named services cannot be used for hooks #1176

Closed
aschempp opened this issue Nov 6, 2017 · 6 comments
Closed

Class-named services cannot be used for hooks #1176

aschempp opened this issue Nov 6, 2017 · 6 comments
Assignees
Labels
Milestone

Comments

@aschempp
Copy link
Member

aschempp commented Nov 6, 2017

Due to the class_exists check in System::importStatic (and System::import) it is not possible to register a hook listener service if the class name is used as service name. Do we really need that class_exists check?

@Toflar
Copy link
Member

Toflar commented Nov 7, 2017

Nope, container->has() is enough. +1 for dropping that (as a bugfix!).

@leofeyer
Copy link
Member

leofeyer commented Nov 7, 2017

We have added the check to prevent services from accidentally overwriting existing classes, which would then no longer be loaded. So no, $container->has() is not enough.

@discordier
Copy link
Contributor

@leofeyer This won't work for native Symfony 3.3 applications anymore as the suggested service name IS the class name.
See here and here

So in the end you will have to alias an symfony service to something else to make it usable in Contao then?
I'm all up for using only $container->has().

@leofeyer leofeyer added this to the 4.4.8 milestone Nov 8, 2017
@aschempp
Copy link
Member Author

aschempp commented Nov 8, 2017

I think the only and major issue we had initially was with our Session class and Symfony's session. Because System::import('Session') would take the service instead of our class. So we gotta make sure about that …

@aschempp
Copy link
Member Author

aschempp commented Nov 8, 2017

I think the best approach is to check for a backslash in the name.

if ($container->has($name) && (false !== strpos($name, '\') || !class_exist($name)) {
}

@leofeyer leofeyer self-assigned this Nov 9, 2017
@leofeyer
Copy link
Member

leofeyer commented Nov 9, 2017

Fixed in 9361ea9.

@leofeyer leofeyer closed this as completed Nov 9, 2017
@leofeyer leofeyer modified the milestones: 4.4.8, 4.4 May 14, 2019
leofeyer pushed a commit that referenced this issue Jan 13, 2020
…1176)

Description
-----------

PR for contao/contao#1079

I think it's better to disable it for all non-Contao requests. Not just the FE ones.

Commits
-------

35868fd6 Disable MakeResponsePrivateListener for non-Contao requests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants