? Pending

User tests: Successful: Unsuccessful:

avatar csthomas
csthomas
30 Dec 2017

Pull Request for Issue #18476 .

Summary of Changes

Function call_user_func() can not pass arguments by references.

Reference: http://php.net/manual/en/function.call-user-func.php

On php 7.1 and higher this is an issue that generates a PHP Warning.

JError::handleCallback expected a reference at the first argument.

To fix it, joomla has to use call_user_func_array, which can pass arguments by reference.

Testing Instructions

  1. Test on php 7.1 or higher
  2. Login form module is set to display on all pages.
  3. Be sure that plugin "System - Logout" is enabled.
  4. Login as superadmin on frontend
  5. Edit an article
  6. Logout through the login module
  7. Check error log.

Expected result

No warning.

Actual result

PHP Warning:  Parameter 1 to PlgSystemLogout::handleError() expected to be a reference, value given in ../libraries/legacy/error/error.php on line 780

PHP Stack trace:
PHP   1. {main}() .../index.php:0
PHP   2. Joomla\CMS\Application\SiteApplication->execute() .../index.php:49
PHP   3. Joomla\CMS\Application\SiteApplication->doExecute() .../libraries/src/Application/CMSApplication.php:267
PHP   4. Joomla\CMS\Application\SiteApplication->dispatch() .../libraries/src/Application/SiteApplication.php:233
PHP   5. Joomla\CMS\Component\ComponentHelper::renderComponent() .../libraries/src/Application/SiteApplication.php:194
PHP   6. Joomla\CMS\Component\ComponentHelper::executeComponent() .../libraries/src/Component/ComponentHelper.php:356
PHP   7. require_once() .../libraries/src/Component/ComponentHelper.php:381
PHP   8. ContentController->execute() .../components/com_content/content.php:43
PHP   9. ContentController->display() .../libraries/src/MVC/Controller/BaseController.php:710
PHP  10. JError::raiseError() .../components/com_content/controller.php:101
PHP  11. JError::raise() .../libraries/legacy/error/error.php:277
PHP  12. JError::throwError() .../libraries/legacy/error/error.php:202
PHP  13. JError::handleCallback() .../libraries/legacy/error/error.php:241
avatar csthomas csthomas - open - 30 Dec 2017
avatar csthomas csthomas - change - 30 Dec 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 30 Dec 2017
Category Libraries
avatar csthomas csthomas - change - 3 Jan 2018
Title
Correctly call function with the parameter by reference
Remove PHP warning from PlgSystemLogout::handleError()
avatar csthomas csthomas - edited - 3 Jan 2018
avatar csthomas csthomas - change - 12 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 12 Jan 2018
avatar csthomas csthomas - change - 29 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 29 Jan 2018
avatar csthomas csthomas - change - 29 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 29 Jan 2018
avatar csthomas csthomas - change - 29 Jan 2018
Title
Remove PHP warning from PlgSystemLogout::handleError()
Remove PHP warning when logging out while editing the article on php >= 7.1
avatar csthomas csthomas - edited - 29 Jan 2018
avatar csthomas
csthomas - comment - 29 Jan 2018

@infograf768 @franz-wohlkoenig @Quy Could any of you test it?

avatar infograf768
infograf768 - comment - 29 Jan 2018

Something must have changed in core since the original issue was posted as if, indeed, I do not get the PHP Warning anymore (WITH or WITHOUT this patch), I now get a 403 (You are not authorized etc.) instead of a redirection to the page displaying the article.

avatar csthomas
csthomas - comment - 29 Jan 2018

Maybe you have disabled plugin system logout

avatar csthomas csthomas - change - 29 Jan 2018
The description was changed
avatar csthomas csthomas - edited - 29 Jan 2018
avatar infograf768
infograf768 - comment - 29 Jan 2018

It is enabled.

avatar csthomas
csthomas - comment - 29 Jan 2018

Then it is weird I can reproduce it on php 7.1.13

avatar sandewt
sandewt - comment - 29 Jan 2018

Yes, I can also confirm it.
OS Windows
PHP 7.1.9
MySQLi 5.5.5-10.1.26-MariaDB

@csthomas how did you generate the PHP Stack Trace?

avatar csthomas
csthomas - comment - 29 Jan 2018

On linux I do:

sudo phpenmod -v 7.1 xdebug
sudo systemctl restart apache2

You have to install xdebug module for php and restart apache2.

avatar joomdonation
joomdonation - comment - 29 Jan 2018

I have tested this item successfully on a2f56a6

Tested using PHP 7.2


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

avatar joomdonation joomdonation - test_item - 29 Jan 2018 - Tested successfully
avatar Quy
Quy - comment - 29 Jan 2018

I have tested this item successfully on a2f56a6

Tested with PHP 7.2


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

avatar Quy Quy - test_item - 29 Jan 2018 - Tested successfully
avatar Quy Quy - change - 29 Jan 2018
Status Pending Ready to Commit
avatar Quy
Quy - comment - 29 Jan 2018

RTC


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

avatar infograf768
infograf768 - comment - 29 Jan 2018

folks, can you tell me what you get exactly when you edit an article in frontend and logout via the module? Where are you redirected to?

avatar Quy
Quy - comment - 29 Jan 2018

I get redirected to the main page and in the PHP error file Parameter 1 to PlgSystemLogout::handleError().

avatar infograf768
infograf768 - comment - 29 Jan 2018

my test was done on a multilingual site. will test again tomorrow

avatar infograf768
infograf768 - comment - 30 Jan 2018

Works fine on a clean install.
Remains to find out here why it does not on ALL my legacy test sites.
I get a 403 for all of them, multilingual or not.

avatar infograf768
infograf768 - comment - 30 Jan 2018

I found out why I get a 403 instead of redirecting to the Home page:

The System Redirect plugin was enabled.
If I disable that plugin all is fine.

@csthomas @Quy Please test

avatar infograf768
infograf768 - comment - 30 Jan 2018

Note: the issue I found with the conjunction of both plugins being enabled has no impact on this PR which does work indeed when the Redirect plugin is disabled.

avatar csthomas
csthomas - comment - 30 Jan 2018

I confirm 403 when Redirect plugin is enabled.

How to fix it:
Go to backend, plugin list, order by Ordering ascending, then move your plugin "System - Redirect" above "System - Logout".

avatar infograf768
infograf768 - comment - 30 Jan 2018

@csthomas
The problem is that, by default, the order is set differently when Joomla is installed.
Therefore I think we have to find another solution with code.

avatar csthomas
csthomas - comment - 30 Jan 2018

There are two competing handlers.

// Set the JError handler for E_ERROR to be the class' handleError method.
JError::setErrorHandling(E_ERROR, 'callback', array('PlgSystemRedirect', 'handleError'));

// Set the error handler for E_ALL to be the class handleError method.
JError::setErrorHandling(E_ALL, 'callback', array('PlgSystemLogout', 'handleError'));

avatar csthomas
csthomas - comment - 30 Jan 2018

I suggest to create a new issue about it.

avatar infograf768
infograf768 - comment - 30 Jan 2018

There are two competing handlers.

I meant: do we have a way through code to force the redirect plugin to load before?
Will create issue anyway.

avatar csthomas
csthomas - comment - 30 Jan 2018

Plugin "Logout" should override the plugin "Redirect" when there is a cookie "PlgSystemLogout".
If "Logout" is first then there have to be a way to disallow to override Logout redirection by "Redirect" plugin.

I'm not familiar with that code.

avatar infograf768
infograf768 - comment - 30 Jan 2018

Issue created
#19486

avatar infograf768
infograf768 - comment - 30 Jan 2018

PR to, at least, get the right order between these plugins when installing Joomla.
#19489

avatar zero-24
zero-24 - comment - 7 Feb 2018

@infograf768 as the other PR is merged are you agree to merge here too based on the tests above?

avatar mbabker mbabker - change - 13 Feb 2018
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2018-02-13 00:04:05
Closed_By mbabker
Labels Added: ?
avatar mbabker mbabker - close - 13 Feb 2018
avatar mbabker mbabker - merge - 13 Feb 2018

Add a Comment

Login with GitHub to post a comment