[#42766] - SiteRouter throws 404 when parsing other Uris instead of returning variables
- New
- Medium
- Build: 4.4-dev
- # 42766
Steps to reproduce the issue
Edit any system plugin and add this:
public function afterRoute() { // or afterRender, anything after routing the main component.
$router = \Joomla\CMS\Factory::getContainer()->get(SiteRouter::class);
$u = \Joomla\CMS\Uri\Uri::getInstance(Uri::base().'component/content/article/2-demo-article');
// replace the above with the ID and alias of one of your articles
$variables = $router->parse($u, false);
var_dump($variables);
}
Expected result
$variables populated with option, view, id.
Actual result
Throws a 404 error.
System information (as much as possible)
Tested in both J4 and 5.
Additional comments
So, the Joomla\CMS\Router\Router is supposedly able to parse any Uri with the option to $setVars = false.
As far as I understand, this is supposed to mean that you can parse any Uri and get its variables, regardless of the active menu item.
Correct me if I'm wrong.
What actually happens is that while most of the process -I read through a lot of the code- is menu-independent, the default StandardRules, at line 72 and the default NoMenuRules, at line 71 use the active menu item as a guide on how to parse the passed Uri object.
If you replace these lines with $active = null, the expected variables return correctly.
I'm relatively unfamiliar with the rest of the routing system, so I'm probably missing a lot.
Is this the expected behavior or is this a bug?
joomla-cms-bot
- | Labels |
Added:
No Code Attached Yet
|
||
| Title |
|
||||||
| Title |
|
||||||
| Title |
|
||||||
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2024-03-22 11:57:08 |
| Closed_By | ⇒ | Hackwar |
@Hackwar Thank you for your effort. However, I think that returning an empty array doesn't fix the expected functionality. Since the StandardRules and NoMenuRules rely on the active menu item, there's always something left in the $uri->getPath() if you are trying to parse another URL that is not the active menu item. Did you find something different in your tests?
The thing is that the router currently sets the active menu item to the one it parsed in the current run. So when parsing the URL, the active menu item actually is changed, which of course is wrong. But the code now would parse this "correctly". I'm still working on getting the rest cleaned up.
I'd be happy to help if you want me to.
Be my guest. I can use all the help for routing I can get. Ideally could you contact me on the Joomla Mattermost on https://joom.la/chat ? Then we could coordinate that. Just send me a DM
| Status | Closed | ⇒ | New |
| Closed_Date | 2024-03-22 11:57:08 | ⇒ | |
| Closed_By | Hackwar | ⇒ |
We discussed the PR in the maintainer meeting and decided that my approach wasn't correct. The PR also didn't really solve your issue, I think. Seems like I misunderstood you. The main problem is, that the parse method has side effects from setting the active menu item. My idea would be to introduce a new interface for component routers and when those component routers implement that new interface, the parse() method would have an additional parameter with the current parsed query parameters as well and not just the segments. That way we wouldn't have to read the active menu item.
As much as I would like to follow your thinking, I'm not that familiar with the routing process. In my initial comment, I mentioned the points of the default rules that the active menu item is being used. Removing them seems to work for me, but of course I cannot imagine how this would affect the millions of installations of Joomla.
The problem is, that the component router when parsing does not get the currently discovered menu item ID as input, but just can read that from the active menu item in the global application object. That means we have to change that global state, which affects other things and is VERY bad. So we somehow have to get that information directly into the component router. I'm still looking into how we could do that.
I understand. This may be a dumb suggestion, but you can judge it: What if you just use the $setVars parameter of the Router to determine if you should use the menu item? Or even add a different parameter? Then, anyone that tries to use the Router::parse would come across this parameter and know what to do with his own code.
We don't have the $setVars available at the point where we would have to decide this. However we will probably change the component router to something like this:
public function parse(&$segments, &$vars)
where $vars contains the parsed queries so far, among others the Itemid. I was unsure if this works with our interface, etc. right now, but it does and we hopefully can do this in 5.2 or 5.3.
I created a PR to fix this with #43118. Thus I'm closing this issue.