?
avatar minitek
minitek
1 May 2018

Steps to reproduce the issue

File:
libraries/src/Router/Router.php

Conditional in Joomla Router::parse() makes all urls in 3rd party extensions invalid if $uri->getPath() > 0

Steps to reproduce the issue are not really important, also my router.php is very big to paste here.

The comment in line 146 of class Router says:
// Check if all parts of the URL have been parsed.
// Otherwise we have an invalid URL
The check is:
if (strlen($uri->getPath()) > 0)

Why is this check valid?
Why are urls invalid if the path exists?
It just doesn't make sense and breaks urls.
If I remove the check, the urls work fine.

Expected result

Working urls

Actual result

Invalid urls

System information (as much as possible)

Joomla 4.0.0-dev

Additional comments

The exact same router.php works perfectly in Joomla 3.8.5

avatar minitek minitek - open - 1 May 2018
avatar joomla-cms-bot joomla-cms-bot - change - 1 May 2018
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 1 May 2018
avatar franz-wohlkoenig franz-wohlkoenig - change - 1 May 2018
Category Router / SEF
avatar minitek minitek - change - 1 May 2018
The description was changed
avatar minitek minitek - edited - 1 May 2018
avatar minitek minitek - change - 1 May 2018
The description was changed
avatar minitek minitek - edited - 1 May 2018
avatar minitek minitek - change - 1 May 2018
The description was changed
avatar minitek minitek - edited - 1 May 2018
avatar mbabker
mbabker - comment - 1 May 2018

I assume you are referring to this line (or this line in 3.x but the error condition only triggers if the new router is enabled).

This check is done after the component routers have been run, where a component router should be taking path segments and removing them after parsing them into whatever variables are needed. If there are still path segments remaining after parsing has finished then this indicates that a URI was not fully matched.

avatar minitek
minitek - comment - 1 May 2018

Yes, this is the line.

This is about a 3rd party extension, so the condition was never triggered in Joomla 3 as the new router (sef_advanced) is not relevant.

It seems that the problem existed before but was never triggered. Could you please point me to some documentation or relevant code for removing path segments properly after parsing them?

avatar dgrammatiko
dgrammatiko - comment - 1 May 2018

@minitek check:

public function parse(&$segments)
{
$total = count($segments);
$vars = array();
for ($i = 0; $i < $total; $i++)
{
$segments[$i] = preg_replace('/-/', ':', $segments[$i], 1);
}
// View is always the first element of the array
$count = count($segments);
if ($count)
{
$count--;
$segment = array_shift($segments);
if (is_numeric($segment))
{
$vars['id'] = $segment;
}
else
{
$vars['task'] = $segment;
}
}
if ($count)
{
$segment = array_shift($segments);
if (is_numeric($segment))
{
$vars['id'] = $segment;
}
}
return $vars;
}
}

avatar mbabker
mbabker - comment - 1 May 2018

The parse method in routers is basically the reverse of the build method.

If you don't take all the query string variables in the build method and either remove them or convert them to URI segments then you end up with SEF URLs which still have a query string path (i.e. https://www.joomla.org/announcements.html?start=3 for page 2 of our announcements section).

It's important for parsing SEF URLs to remove all segments otherwise this is where you start running into the awkward behaviors of having several URLs for the same content page and the Joomla router would just accept it with no questions asked (if you search around there are some examples of some things that shouldn't work but will because the older router structure is so lenient).

avatar minitek
minitek - comment - 1 May 2018

Thank you guys!

Solved.

avatar minitek minitek - change - 1 May 2018
Status New Closed
Closed_Date 0000-00-00 00:00:00 2018-05-01 21:11:47
Closed_By minitek
avatar minitek minitek - close - 1 May 2018

Add a Comment

Login with GitHub to post a comment