[#17483] - Erroneous url-encoding on Redirect after frontend Article Save
- Closed
- 11 Aug 2017
- Medium
- Build: staging
- # 17483
- Diff
- LivioCavallo:staging
User tests: Successful: Unsuccessful:
On user-specified redirect after article saving in frontend, the code uses JRoute constructor to build url, but doing so it uses default parameter that leads to erroneously encoding all the url query part, so converting '&' to ' & amp;' (entity, without space).
That is not correct and may lead to components error or malfunction, particularly in the case of third party components, as they could not be able to get Itemid param and all the other query params.
The same problematic default param is used in case of edit error (same class, edit function), that redirects to return page (via getReturnpage).
Summary of Changes
Add 'false' param in two JRoute constructors calls
Testing Instructions
Create a menuitem to show list of articles of 'uncategorized' type
Create a menuitem to Submit New Article and specify a redirect after submit to previous menuitem.
Submit a new article
Expected result
redirection to: index.php?option=com_content&view=category&id=mm&Itemid=nnn
Actual result
redirection to: index.php?option=com_content& amp;view=category& amp;id=mm& amp;Itemid=nnn (entities, without spaces)
Documentation Changes Required
none
joomla-cms-bot
- | Category | ⇒ | Front End com_content |
| Status | New | ⇒ | Pending |
| Labels |
Added:
?
|
||
This adds false to the second argument of $this->setRedirect(), which would be the $msg argument. That looks wrong.
Maybe the intention was to add it to JRoute::_()? Then it would be the $xhtml argument which indeed would steer if the ampersands are replaced or not.
| Category | Front End com_content | ⇒ | Administration Language & Strings Front End com_content |
Most $this->setRedirect(JRoute::_ in core contains the false
The argument is added in the JRoute part.
Examples:
$this->setRedirect(JRoute::_('index.php?option=com_admin&view=profile&layout=edit&id=' . JFactory::getUser()->id, false));
$this->setRedirect(JRoute::_('index.php?option=com_contact&view=contact&id=' . $stub, false));
etc.
Indeed, PR is wrong on this.
| Labels |
Added:
?
|
||
Yes I intended to add false as second parameter to JRoute constructor.
I have updated the PR (I hope the procedure is right).
I wanted to make a second PR to suggest adding support to redirect after Cancelling, but i fear that the modifications that involve the same file, came in this PR.
How to solve this now and keep the two issues separate?
I guess it is indeed better to separate.
Also not sure about adding the cancel redirect. I have to test this.
Ok, so I soppose I should close this and should make 2 PR (this time from my two different branches, not from my staging fork...).
Right?
Yes, ideally you create an own branch for each PR you want to make. And keep your staging clean.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-08-11 12:57:12 |
| Closed_By | ⇒ | LivioCavallo |
This makes sense imho.
When using JRoute do we have to always add
false?@mbabker @Bakual
What you think?
If yes, there are multiple occurrences of this in core, not only in this PR.