joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.77k stars 3.65k forks source link

Publish task of JControllerAdmin or AdminController reports some errors as success #17808

Closed ggppdk closed 2 years ago

ggppdk commented 7 years ago

Steps to reproduce the issue

i do not have steps but it happens in cases of 2 error in the publish task of (J3.7.5)JModelAdmin or (J3.8+)AdminModel JLIB_APPLICATION_ERROR_EDITSTATE_NOT_PERMITTED JLIB_APPLICATION_ERROR_CHECKIN_USER_MISMATCH


[EDIT] short explanation publish task , call model->publish , which return without setting error via setError publish task look at getError, see none, and assumes success

Also faster to understand is to look at this code lines: https://github.com/joomla/joomla-cms/blob/staging/libraries/src/MVC/Controller/AdminController.php#L211-L224

https://github.com/joomla/joomla-cms/blob/staging/libraries/src/MVC/Model/AdminModel.php#L1005-L1024

And also in then menus controller identical code as (J3.7.5)JControllerAdmin or (J3.8)AdminController https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_menus/controllers/items.php#L224-L238


Detailed Explanation of the issue The publish method of (J3.7.7)JModelAdmin or (J3.8)AdminModel

Instead the error is added to the log file via JLog

Looking at the code it looks like

Then back at the publish() control task of (J3.7.5)JControllerAdmin or (J3.8)AdminController, it does not check if model->publish() returned false !

Expected result

There model errors are show as failure by the publish() controller task JLIB_APPLICATION_ERROR_EDITSTATE_NOT_PERMITTED JLIB_APPLICATION_ERROR_CHECKIN_USER_MISMATCH

Actual result

They are shown as success

System information (as much as possible)

J3.7.5 / J3.8 staging

Additional comments

I think the intention of the code since the bad $PK is being unset, was to allow the publish task at controller to decide if it will show the error or continue and call the model->publish task again, or something like this

An example of this happening with publish task of menus controller is here (note that menus controller has same code as (J3.7.5)JControllerAdmin or (J3.8)AdminController)

https://github.com/joomla/joomla-cms/issues/15938#issuecomment-326304172

ggppdk commented 7 years ago

It is easy to fix this , just a decision needs to be made

ggppdk commented 7 years ago

I have edited description to add links to the code, now it will be easy to see what is happening

ghost commented 6 years ago

Status chnged to "Needs Review".


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17808.

gwsdesk commented 6 years ago

I tend to opt for the third "dot" option

brianteeman commented 2 years ago

Thank you for raising this issue.

Joomla 3 is now in security only mode with no further bug fixes or new features.

As this issue doesn't relate to Joomla 4 it will now been closed.

If we are mistaken and this does apply to Joomla 4 please open a new issue (and reference this one if you wish) with updated details for testing in Joomla 4. cc @zero-24