What do I have to do in documentations?

I think we have to be a bit careful about allowing overrides of everything. For example as a hard thing I don't think the password handlers should be overrideable for security reasons.

I think that certain other things like the applications and session (things that traditionally had a Jfactory static method) also probably shouldn't either.

Obviously most the factories probably should be overrideable on the other hand. Don't think it's quite as easy as we're making it out to be I'm afraid!

@wilsonge

1. why do you think that proper use of a DI-container, where you can exchange one dependency for another, would be dangerous for security? Can you give some example or direction of what kind of scenario you have in mind for such a malicious use?
2. do you think that other frameworks (on any platform) that don't have this odd "protected"-feature in their DI-containers are all insecure?
3. what's the use of a DI-container if you use it to store some immutable "globals"?
4. in extensions usually a clone of Joomla's "central" DI-container is used, so when you exchange a dependency for another, it is usually locally scoped. If it would not be possible to locally override some dependency in the DI-container provided by Joomla, a developer can better use another DI-container or directly inject the dependency. "Protecting" developers to easily use the Joomla DI-container might not be the best strategy to help developers to make better extensions.
5. It is the responsibility of the extension builder to build secure extensions. Those extensions won't be more secure by limiting the use of Joomla's DI-container. Are such limitations not a bit paternalistic? I don't see what that contributes to security.

I couldn't have worded it better myself. Seriously I couldn't have :) Far from it even. But I completely agree with all of the above.

Tbh, the side effects of switching already used objects are endless.
Example, one of my extensions replaces the Joomla Database handler (I tested the bad and ugly way but decided to use the proper way) and adds additional functionality, this happens in the earliest possible state.
if now another extensions developer thinks it's a good idea to add a function to the db handler, because he or she doesn't want to have it in it's own component, and replaces the db object too then we have a conflict. my or the other extension will fail.

it's something different on Factories used in the local container of the component. there you can't expect that the joomla own factory is used.

From my point of view it must be really clear why you want to remove the lock and why and if you don't have a better way to do it.

Other example, if a shared instance in the container get replaced, old copies used in extensions are still the old object while new access to the container object will be the new object which could have extremely negative side effects on joomla (core).

So to make it should please proof that it doesn't work different for each use case.

@HLeithner

1. Replacing a key in the DI-container means: replacing it with another implementation of the same interface. If you replace it with an implementation of another interface (as you describe), you are not doing it correctly.
2. Some uses of the DI-container in Joomla's core violate the Liskov Substitution Principle. That is a bug. For instance: the Application has the Joomla\Application\CMSApplicationInterface as a key in the DIC. That means, that no other methods than listed in the interface should be used in the places where this dependency is resolved by the DI-container. Unfortunately that is not the case: in all kinds of models in core extensions (like com_content), methods are used that are not part of the application interface. That's why you get errors when using a core extension model from for instance a CLI-application. Locking the bug behind some odd "protect"-flag only hides the bug, it is not solving it. We better clean this mess instead of preventing developers to properly use the DIC: stick to the interface!
3. if you want to add a method to some dependent object, then you are using another interface. Hence you need to use another key to register your dependency in the DIC. The original interface is now a dependency of your new interface. For those cases I agree that it is not necessary to replace a key in the DIC: better add a new one. We should educate developers on that (and provide proper examples of it in core). But that is something else than preventing the use of another implementation of an interface in the DIC.
4. one of the use cases of a DI-container is: replacing the implementations with mocks to be able to test your software. A "protect"-flag prevents that. Why would you even use a DI-container for things like that? Might be easier to go back to something like global $mainframe ;-) . Maybe we should make some nice examples of how to test your custom software in Joomla, using the DI-container.

In some of my extensions I use Doctrine ORM instead of Joomla's database handler. That is not conforming the Joomla database interface, so I don't replace that interface. I use them side by side, Joomla's db and Doctrine. No problem for any other extension.

I think that the original problem that led to this issue and PR should not be solved by replacing the protected-flag, but by adding another key to the DI-container. But I still think, for other cases (testing amongst them), that we should get rid of that odd protected-flag, for it only protects developers from properly using the DIC. That double message might be a bit confusing. I owe you some concrete examples, both of a better solution of the original problem and proper examples of why we better drop that protected-flag in the DI-container (to be continued). This will need some research.

@HLeithner

1. Replacing a key in the DI-container means: replacing it with another implementation of the same interface. If you replace it with an implementation of another interface (as you describe), you are not doing it correctly.

If I replace the implementation with my own implementation of the same interface with different functionality and someone else does the same my installation is broken, in worse case beyond repair.

2. Some uses of the DI-container in Joomla's core violate the Liskov Substitution Principle. That is a bug. For instance: the Application has the Joomla\Application\CMSApplicationInterface as a key in the DIC. That means, that no other methods than listed in the interface should be used in the places where this dependency is resolved by the DI-container. Unfortunately that is not the case: in all kinds of models in core extensions (like com_content), methods are used that are not part of the application interface. That's why you get errors when using a core extension model from for instance a CLI-application. Locking the bug behind some odd "protect"-flag only hides the bug, it is not solving it. We better clean this mess instead of preventing developers to properly use the DIC: stick to the interface!

You know joomla is old and extension developers and agencies are complaining that we are too fast in our translation (lol took 7 years for a major version which is basically the same was before with some cleanups)

3. if you want to add a method to some dependent object, then you are using another interface. Hence you need to use another key to register your dependency in the DIC. The original interface is now a dependency of your new interface. For those cases I agree that it is not necessary to replace a key in the DIC: better add a new one. We should educate developers on that (and provide proper examples of it in core). But that is something else than preventing the use of another implementation of an interface in the DIC.

If I need replace functionality which can't be done by other ways I have to replace existing keys with my own implementation if other components should use them (active or automatically). Since this could make a clash I skipped this and used another approach.

4. one of the use cases of a DI-container is: replacing the implementations with mocks to be able to test your software. A "protect"-flag prevents that. Why would you even use a DI-container for things like that? Might be easier to go back to something like global $mainframe ;-) . Maybe we should make some nice examples of how to test your custom software in Joomla, using the DI-container.

yes but you don't boot joomla and then swap the parts of the container, instead you fill the container with the things you need before they are protected.

In some of my extensions I use Doctrine ORM instead of Joomla's database handler. That is not conforming the Joomla database interface, so I don't replace that interface. I use them side by side, Joomla's db and Doctrine. No problem for any other extension.

So no need to remove the protection because you found a better solution, as you mentioned earlier, modifying the main container is not much more then changing a $globals variable.

I think that the original problem that led to this issue and PR should not be solved by replacing the protected-flag, but by adding another key to the DI-container. But I still think, for other cases (testing amongst them), that we should get rid of that odd protected-flag, for it only protects developers from properly using the DIC. That double message might be a bit confusing. I owe you some concrete examples, both of a better solution of the original problem and proper examples of why we better drop that protected-flag in the DI-container (to be continued). This will need some research.

If you find a use case where removing the protection is the only way to solve it, I'm happy to here about it.

I think the DIC implementation of a CMS like Joomla which is highly dynamical compared to a laravel or symfony application needs to make a "safe playground" for all extension developers and for the site own who installs 30 extensions and expect that not 2 extensions kills each other.

If I replace the implementation with my own implementation of the same interface with different functionality and someone else does the same my installation is broken, in worse case beyond repair.

different functionallity == different interface.

If I need replace functionality which can't be done by other ways I have to replace existing keys with my own implementation if other components should use them (active or automatically).

replacing functionallity == changing the interface. Another implementation should preserve the interface. Otherwise it is another interface (and should not be with the original interface as a key in the DIC).

yes but you don't boot joomla and then swap the parts of the container, instead you fill the container with the things you need before they are protected.

In Joomla extensions we use a clone of the DI-container, so we never swap objects in the original container, but only in the local clone. And that indeed is something else than filling the container with different objects. Would be interesting to create some nice examples of both approaches.

I think the DIC implementation of a CMS like Joomla which is highly dynamical compared to a laravel or symfony application needs to make a "safe playground" for all extension developers and for the site own who installs 30 extensions and expect that not 2 extensions kills each other.

I totally agree. But I think a lot can be improved for that on the current use of Joomla's DI-container in the core. It is in my agenda to try to help with that.

Well, in ideal world that will work.
In personal App it will work.

However in CMS the issue a bit different, as already was pointed we cannot guarantee that an overriden depenency will not break the core. That why we have to protect items in "Global container".

I think would help if "Local container" still allows to override, but only in local scope.
Or we have to remove some globals, so extensions will define them locally.
Or unprotect only specific items (mostrly factories).

I think would help if "Local container" still allows to override, but only in local scope.

Wel, that is the case in a Joomla extension: you only override something locally, in the clone of the DI-container that is only used locally. The protected-flag however prevents even that!

@HLeithner
yes but you don't boot joomla and then swap the parts of the container, instead you fill the container with the things you need before they are protected.

I don't understand 100% what you want to say here. Could you explain this sentence a bit?

Do I understand it well, that you think that it would be better to fill the container with your own things before the core protected objects are registered? So that the own objects are (globally) used instead of the core protected objects? And that would be better than replacing a core object in the clone of the DI-container that is only used locally in your extension? I probably misunderstand that, for it looks obvious to me that overriding core things globally is more dangerous for the whole installation than only locally in the cloned container in your own extension.

Well, in ideal world that will work. In personal App it will work.

However in CMS the issue a bit different, as already was pointed we cannot guarantee that an overriden depenency will not break the core. That why we have to protect items in "Global container".

1. if an overriden dependency in the (always!) cloned container of an extension, that is only used in that extension, would break some core functionallity, then there is an architectural flaw.
2. a service locator misuse of a DI-container, to get some globals everywhere, is nowadays mostly seen as an anti-pattern. We must try to compartmentalise our software as much as possible, so one extension doesn't effect another. In Joomla a compromise is used between some global use and keeping things local by always cloning the central DI-container in every extension. So when you use your own implementation instead of the core extension, it will only be used locally.
3. Joomla doesn't only have distributed extensions, but extensions also very often custom made for just one company and a very specific domain. In fact, that is more my kind of business than the distibuted extensions. Joomla is a very nice "platform" to build custom applications on. I know that is not the main focus of the current Joomla maintainers, but it is also a part of Joomla. Some misunderstandings in this discussions could also come from the focus on the CMS and distibuted extensions. But even then: I haven't seen yet an example where it would do any harm to remove that protected flag.
4. there still seems to be some misunderstanding about what happens if you override a key of a core object in the DI-container as intended: you only override the implementation, but the interface stays the same.

Not removing that protected flag might be more dangerous: if someone then wants to override some core behaviour, then we do it globally (for instance using the Obix class extender plugin). Much more powerful, but not restricted to the local clone of the DI-container that every extension makes.

I can imagine the following compromise if anyone would be worried that some extension might not only override a local clone of the DI-container that is normally used in every container. For some developer might also do the override in the $container->parent. What if we would just standard remove all protected flags only in the clone? That is also the intended use, to only do overrides in the local clone of the extension and people that are afraid it would influence some parent behaviour can be happy, because we just keep that odd protection-flag for their peace of mind.

This discussion has reached a depth and a level of knowledge about the joomla core where I unfortunately can no longer follow. It seems to me that my change proposal cannot be accepted in this way. However, I do not understand the concept and the implementation of DI containers in joomla well enough to make another, better change proposal.

As extension developer I simply became aware that, contrary to the promises, overwriting containers in components does not work after all. In my current project I would like to overwrite the FormFactory, which should be possible from my point of view but isn't at the moment.

Therefore my proposition is to unprotect all insensitive containers like Joomla\CMS\Form\FormFactoryInterface here with this PR and find a clean solution for the sensitive one afterwards. Maybe @HermanPeeren you have a nice solution for a general nicer fix for this problem...

From my point of view these containers we can unprotect without causing harm:

• Dispatcher
• Document
• Form
• Language
• Menu
• Router
• Toolbar

What do you think @HLeithner @wilsonge ?

can you show me your formfactory?

About the list, every singelton (isShared) cant be unprotected in my opinion.

We should have a look to unprotect/allow override for local extension "clones/childs" of the container

can you show me your formfactory?

Here it is: https://github.com/Elfangor93/JG4-dev/blob/listOfAdapters/administrator/com_joomgallery/src/Form/FormFactory.php

That is also the intended use, to only do overrides in the local clone of the extension and people that are afraid it would influence some parent behaviour can be happy, because we just keep that odd protection-flag for their peace of mind.

Here is possible solutuion joomla-framework/di#47

@Fedik thanks.

Some extra information, if someone might be interested in that:
Every extension (core and custom) in Joomla first clones the DI-container. When you place or override a key in the DI-container ($container->set()) it is only set in the cloned container that is locally used to inject dependencies in the locally instantiated objects of that specific extension. That is the intended use of the DI-container and limites the override to the extension. It can not have any influence globally! So it is completely safe to override that (child) container; it cannot give troubles to other extensions. That is the basic idea of it. Locally overriding keys is much safer than exchanging the global object in an early stage for your own object as we did before or are inclined to do if someone is "protecting" us from using our own dependencies in our own extensions.

Theoretically, you could change something in the parent container ($container->parent): the central DI-container which is cloned to use the child container in the extension. That is not the intended use and might give problems with other extensions. So for that use case it might be an idea to avoid that some core objects can be overriden. I would still say: if you replace it while keeping the same interface, then it should not be a problem. But those interfaces are not completely correctly used in the current core, so we might be a bit careful with it, just to avoid problems.

The PR of @Fedik in the DI-container joomla-framework/di#47 makes that possible. No difficulties what keys should stay protected: all keys in the child-container (that we now usually use) can then be overriden. I think that is a simple and good (and safe) way to do it. And anyone having concerns about overriding keys in the "central" container can have peace of mind.

can you show me your formfactory?

Here it is: https://github.com/Elfangor93/JG4-dev/blob/listOfAdapters/administrator/com_joomgallery/src/Form/FormFactory.php

If I see it write the only thing you change is by removing a / in the xpath right? because you may have nested field elements?
could you show me the xml file?

and do I correctly expect that you show our own configuration view instead of the joomla native implementation?

If I see it write the only thing you change is by removing a / in the xpath right? because you may have nested field elements?
could you show me the xml file?

Yes this is currently the difference. But there are plans to add also some additional methods to the ConfigForm class...
Here is the XML: https://github.com/Elfangor93/JG4-dev/blob/listOfAdapters/administrator/com_joomgallery/forms/config.xml

do I correctly expect that you show our own configuration view instead of the joomla native implementation?

Yes, exactly. The component will offer more functionality for the configuration than the native implementation.

Thanks to @HLeithner I found a way around overriding the container for my usecase. Since my initial intention was to replace the FormFactory within my component I can do that by using the switch provided in the Joomla\CMS\MVC\Model\FormBehaviorTrait where the formfactory gets loaded. In this case you have the possibility of providing a getFormFactory() method in your model to create the FormFactory instead of creating the default one provided in the container.

So maybe a clean solution would be to offer such switches for all containers that may be overritten by extensions?

When we go the way from @Fedik, it would men that my system plugin needs to replace the default form factory in every extension container with my shiny awesome factory, instead of replacing just the global one. I don't have an issue with this, just want to make sure we are aware about this.

seems a very big change with lots of potential for screwups with extensions to go in a minor release

How is this a "very big change" en where would those "lots of potential for screwups" come from in existing core and/or 3rd party extensions?

it would men that my system plugin needs to replace the default form factory in every extension container with my shiny awesome factory, instead of replacing just the global one

As we discussed a couple comments up, better to avoid replacing "global resources", only local needed for extension ;)
That the same what you dooing with whole MVC stack, for this reason we do not have MVCFactory in globals. But in some reason FormFactory landed there.

For a FormFactory this makes sense. But I'm pretty sure that extensions want to replace the global Database or Language Factory. So this should still be possible IMO. A selling point of Joomla is the extensibility, but having protected services prevents that. So I still think this here is a valid change, even when it comes with the cost, that it can lead to conflicts two extensions will change the Database factory. So what should be promoted more is the extend feature of the container and not the override.

Maybe it's ok to replace a factory but replacing a singleton will create more problems then solving.
And as long as nobody came up with a real world use case I'm against it.

A real world use case would be an attempt to implement a router alternative, like @roland-d and yours truly attempted.

It is a real shame that Joomla! cannot be fully utilized. Not only as a CMS but also as an application development platform. It has much that is needed but is hampered by obstacles like this.

tbh if this is the only real problem in joomla then we have one. I know so many short comings in joomla and the DIC implementation was never my problem, even if you really want you can bypass it for your self.

Not sure if I interpret that last comment correctly, but do you mean to say that it is not a problem if you don´t experience it as such?

Apart from that I would love to learn the solution for the problem Roland and I experienced. We could not find a way around it, due to the protection flag.

As already mentioned Joomla has many parts in the core which hurt us as maintainer on a daily base because the have been merged without thinking about the consequences. We still struggle on features merged in joomla 10 years ago because we can't replace it with a better/easier/efficient system.

on the other hand we have only a hand full of developer maintaining the core and have to consider all features for each pr merged, as you can see in the latest release it's not easy to see or test all side effects. And the more open we are the more side effects we have.

if you really need it for your own extensions on your own systems you can use the php reflection api an replace more or less everything in joomla. But instead of doing your own thing it would be better to invest your time in the core and help us to maintain joomla and make it better if our apis lacks features.

Thanks for bringing up this issue and the good discussion here. In the meantime allows the Joomla DI container to override services in child containers as the pr joomla-framework/di#48 got merged in the framework repository. That change will be part of Joomla 5.0, so I'm closing this pull request.