[#12499] - Joomla 3.6.3 breaks many sites due to badly written 3rd party component routers
- Closed
- 2 Nov 2016
- Medium
- Build: master
- # 12499
Steps to reproduce the issue
This issue affects sites using many 3rd party components. I don't want to mention all the affected components here in public and the results may differ depending on what you have installed.
Easiest way to simulate the issue is to create a clean Joomla site, select random component and add line JHTML::_('behavior.mootools'); into its router file. To simulate really badly written component just add die('CRASH'); instead to simulate hard crash.
Please note that this is really not a Joomla bug but a simulation to show how #8986 breaks sites because of the changed behaviour. Many components seem to assume that router.php gets loaded only when the component is active and have code/includes outside of the routing functions.
Expected result
If modified component is not active, Mootools should not be loaded. In the case of crashing component, it shouldn't break the whole site.
Actual result
Mootools gets loaded in all pages. Badly written component router will take the whole site down.
System information (as much as possible)
Affects all J! 3.6.3 sites.
Additional comments
Pull request #8986 has a few bad (site breaking) side effects caused by many badly written components installed to user's sites.
First like @vinespie said, it seems to load routers from all components, including those which are disabled.
Secondly surprisingly many component routers (including few popular Joomla extensions) are badly written running a lot of code outside of routing functions ASSUMING that the component is active if its router gets called. Some of them are even loading the whole framework and setting up stuff, which they really, really should not do.
I'm quite frustrated about this change as I've already spent 3 full days on tracking down issues which have been caused by someone else's components, but where issues are showing up in Gantry templates. Many of those call JHtml or $app->getTemplate() before template style has been set breaking the template style assignments, but there are some which are even worse than that.
Please consider reverting or at least fixing this new feature to ignore disabled components to at least make it possible to find broken components without uninstalling them one by one.
| Title |
|
||||||
That pr was merged into staging but had a milestone of 3.7 so either was merged to the wrong branch or the milestone label is incorrect.
Not sure how the core can be expected to know about and/or handle an extension that is "badly written". If they are such well known extensions them they really should be testing their code against every release etc. If they didn't I dont have much sympathy
Easiest way to simulate the issue is to create a clean Joomla site, select random component and add line JHTML::_('behavior.mootools'); into its router file.
So basically go against all best practice, and decent separation of code, and break your site....
You should not be loading JS from a router file... a router file is for...um.. routing... not views...
Not a Joomla issue - a 3pd code quality issue. I strike to just close this issue.
I agree with @PhilETaylor
badly written extensions should not work, at all
zero-24
- | Labels |
Added:
?
|
||
| Category | ⇒ | Router / SEF |
Previously you were only able to mess up your own component... But now badly written component can break any other component installed to the site while running just fine by itself.
That is what made me to report about this issue. In Gantry we are using heavily some advanced Joomla features, which break if someone starts messing up things by calling functions before those have been properly initialized by Joomla. JHtml is one of those as they often end up setting default template style as it has not yet been defined by router.
Yeah, I guess that this feature was accidentally merged to wrong branch just like @brianteeman mentioned. Its not a bug fix but a new feature. I think we can all agree that for whatever reason, the change was committed too early.
@mbabker The only reason I was so vocal in this issue is because of I didn't expect to see so many issues after a patch release. I have been following all the changes and I totally love to get all of these issues fixed and I generally don't care about badly written extensions breaking. I'm really loving the changes you make for 4.0 and I wish I had time to participate, but unfortunately I just do not have the time right now.
Well, considering plugins have been massively able to screw up a site since they were mambots, unfortunately folks should be used to defensive coding (especially around template and JDocument related stuff, or those plugins that overload the core API so you have no idea if what you're working with is even "good"). Regardless of when it was merged it was going to screw something up for someone because someone else has crap code. If we used that as a metric for merging pull requests, honestly half of them would probably be turned down because "X extension relies on Y buggy behavior".
@mbabker I agree. I guess it doesn't make sense to revert the change just to add it back to 3.7...
I hope #12469 will be included to the security release as it would really help people to fix their sites after J3.6.3 update.
After that I will consider my request fulfilled: "Please consider reverting or at least fixing this new feature to ignore disabled components..."
It will not be in the security release
This is the same issue I had with an old version of xmap working on J3.6.2 and takes sites down on 3.6.3.
We need to review the policy of changes per release even RC's so we can test them thoroughly.
@xtech86 this is not a core Joomla issue - badly written extensions not following the Joomla API or best practice will always be a liability and could possibly break in new versions of Joomla if they rely on bugs or "special workarounds" - This is the nature of code.
There was a very very long release candidate period for 3.6.3 - its a pity more people never bothered testing their sites with it. As the opener states, the update took his site down - that would have been quickly identified if testing had been done in RC stage... its not even about testing thoroughly, for this situation it would have taken seconds to fail the first test of his site in a RC.
More people testing = better quality code = better quality releases... shout before the release, not after.
Many components seem to assume that router.php gets loaded only when the component is active and have code/includes outside of the routing functions.
Many components WRONGLY assume that router.php gets loaded only when the component is active - in order to create SEF links to extensions, the router needs to build these, so on any page, with a link to a component, the router needs to be loaded...
Even worse, a lot of components and plugins wrongly assume Joomla only spits out HTML documents and because of load order they can screw up a request by forcing it into HTML mode when it was intended to be another format.
| Status | New | ⇒ | Needs Review |
My gut feeling is that this should be closed as not a bug in the core and expected behaviour
@rdeutz @wilsonge can you make a decision please
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12499.
I said that 12 days ago... #12499 (comment)
The Opener himself states:
Please note that this is really not a Joomla bug but a simulation to show how #8986 breaks sites because of the changed behaviour. Many components seem to assume that router.php gets loaded only when the component is active and have code/includes outside of the routing functions.
Closing as not a bug in core.
| Status | Needs Review | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-11-02 15:07:59 |
| Closed_By | ⇒ | Bakual |
Sooner or later we have to stop not merging things because extensions are written like complete garbage.