joomla
[4.4] fix missing / in url Route:link on cli
Summary of Changes
Every statis:$base['path'] is prefixed with / except $_SERVER[] ones. This leads to URLs like
http://xyz.decli instead of http://xyz.de/cli
to be sure that this does not happen a check if static:$base['path'] is prefixed correctly bevor returning the base path is added.
This issue exists in J4 and J5
Testing Instructions
- create a cli task
- This cli task needs to use
Route::link('site', 'index.php?SomeValidURL', true, true, true); - call the task with
php cli/joomla.php scheduler:run -i <TASKID> --live-site 'http://localhost/'
Actual result BEFORE applying this Pull Request
Task#TASKID 'TASKNAME' exited with code 5 in 14.10 seconds.
Expected result AFTER applying this Pull Request
successful run
Link to documentations
Please select:
-
[ ] Documentation link for docs.joomla.org:
-
[ ] No documentation changes for docs.joomla.org needed
-
[ ] Pull Request link for manual.joomla.org:
-
[ ] No documentation changes for manual.joomla.org needed
The problem I see is that it can break existing extensions when they add the / by themself, it will then be doubled. Not saying that the fix is incorrect, but I'm worried about the impact, as this is a widely used part of the CMS.
The problem I see is that it can break existing extensions when they add the / by themself, it will then be doubled. Not saying that the fix is incorrect, but I'm worried about the impact, as this is a widely used part of the CMS.
Do you have a scenario where this can happen? From my current perspective it breaks Route:link in a way that is not able to be fixed with a workaround in my extension.
@svanschu Will this PR here fix your issue #42859 ? If yes, then please mention it at the top of the PR before the Summary of Changes as shown in our PR template:
Pull Request for Issue #42859 (with a space after the issue number so GitHub will create a proper link).
And close the issue as we close issues when we have PRs to fix them.
Thanks in advance.
@richard67 no. As mentioned in the issue it is a different place which needs to be fixed.
It's more readable to use ltrim($path, '/') . '/' ... also reduces the if a / exists
It's more readable to use
ltrim($path, '/') . '/' ...also reduces the if a / exists
I've done the requested change @HLeithner
The problem I see is that it can break existing extensions when they add the / by themself, it will then be doubled. Not saying that the fix is incorrect, but I'm worried about the impact, as this is a widely used part of the CMS.
a double slash in a url shouldnt be a problem
