User tests: Successful: Unsuccessful:
PR for #38404
Please read the explanation and comments on the issue
I am unable to test this as I dont have access to a server with the faulty mod_security rule but as no one else looked like they would create the PR here it is.
Based on @woluweb report this should work and I found a few other places to add.
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
Category | ⇒ | Administration com_categories com_contact com_content com_menus com_newsfeeds |
I have tested this item
Txs for the Pull Request.
I have a server havving that mod_security issue so I can test in real conditions :)
Before applying the patch, I had the following error message within the Modal (when editing an Article from a Menu Item for example):
_ Forbidden
You don't have permission to access this resource._
After applying the patch, the modal was working normally, showing the corresponding Article
Status | Pending | ⇒ | Ready to Commit |
RTC
Unfortunately this is not the right way to fix the issue, like Dimitris mentioned. I will be happy to merge a complete refactoring, but I don't want to hide the valid warning, sorry :/
@fancyFranci please reconsider merging this PR as the complete refactoring probably can't happen in the 4.x
@dgrammatiko can you shed some light into the change here as for me it looks like invalid JS code when you have something like
&task=category.edit&id=+ document.getElementById("' . $this->id . '_id").value +
I have to say that I didn't digg into the code which uses this variable, but from a first glance it looks not right, even it fixes the issue.
@laoneo the js code is passed inside the urlparams. In order that code to be come executable there’s an eval() and there is the problem, in sort the id of the field should be passed as a string but then the whole form field modals needs a rewrite as it is based on this monstrosity…
After several discussions in maintainers chat. It is better to close this one and correctly implement the issue, even when it requires a rewrite. I'm also reopening the issue. Thanks for understanding.
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-11-28 08:24:34 |
Closed_By | ⇒ | laoneo | |
Labels |
Added:
?
|
Suprised as the person stating it is not perfect also states that it cant be fixed any other way
I have tested this item✅ successfully on 8464a13
Lacking the appropriate modSecurity rule to replicate the issue I have applied the patch and confirm the changes by code review.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38628.