System plugin redirect expect deprecated JException, throw fatal error when sent an Exception object

[weeblr]

Steps to reproduce the issue

This is a coding issue, needs code to be reproduced.

In /libraries/legacy/error/error.php, at line 177 (J! 3.5.0), replace
$exception = new JException($msg, $code, $level, $info, $backtrace);
with
$exception = new Exception($msg, $code, $level, $info, $backtrace);
Then select a link to an article, unpublish that article and visit the front end link.

Expected result

The Joomla 404 error page.

Actual result

Fatal error: Wrong parameters for Exception([string $exception [, long $code [, Exception $previous = NULL]]]) in xxxxxxxx/libraries/legacy/error/error.php on line 178

System information (as much as possible)

Joomla 3.5.0, system is irrelevant

Additional comments

This comes from /plugins/system/redirect/redirect.php, at line 55: the handlerError() method signature is:
public static function handleError(JException &$error)
while it should be:
public static function handleError(Exception &$error)

As JException is deprecated, and has been for a while, we should not be forced to use it.

The only fix currently is to disable the plugin when an extension throws Exception (the recommended Joomla API to use) instead of the legacy JException.

Rgds

[zero-24]

@weeblr

Can you send your changes as pull request so it can be tested?

[mbabker]

The redirect plugin is supposed to have two handlers now, handleError which handles JException objects thrown by the JError API, and handleException which acts as a global Exception/Throwable handler (needed because PHP 7 doesn't use references for the argument).

Given this looks like a theoretical issue (changing internals of JError to reproduce an issue), can you provide a functional code demo that triggered the incorrect behavior?

[weeblr]

Hi Michael,

It's not theoretical, but it involves other extensions in real life, so the example above was meant to reproduce only with Joomla.

I see now that I have not yet identified the whole process involved. I'll dig more and come back with appropriate details, or close the issue if not relevant.

Thanks for your quick handling

Yannick

[mbabker]

For reference #9270 explains why handleError is used explicitly for the JError API and the newer handleException method was added.

[weeblr]

Hi Michael

After further research, this issue (in sh404SEF) is indeed caused by the change in signature of the handleError() method in #9270:

-   public static function handleError(&$error)
+   public static function handleError(JException &$error)

Under some (rare) circumstances, we create an Exception object, but use call_user_func_array downstream to send that Error to the Joomla error handler. This was good before, but now that handleError() requires a JException, it fails.

Any reason to use the deprecated JException in that signature over the regular Exception?

Thanks

[mbabker]

Because that error handler is now very explicitly set to only catch JException objects. With the changes in PHP 7, the signature for uncaught Throwable objects has to be handleError(Throwable $error) (note no &). So I split handleError() purposefully into two methods (one for JError and one for Exception/Throwable) and purposefully typehinted the JError handler to only function against that API. The typehinting can probably be removed but my personal suggestion is that as of 3.5 if you have to proxy into that plugin to use the handleException() method for non JException objects as its signature is fully compatible with PHP 5 and 7's set_exception_handler() callable. I'd also suggest handleError() be deprecated and to be removed when JError finally moves to /dev/null in favor of handleException().

Ultimately both call the same private function so there's no functional difference, just more of a declaration difference than anything.

[weeblr]

Hi

I'm very fine with the split, just not sure handleError() will always be called with a JException, which is why a Exception type hint may be safer.
For instance, if one uses JError::throwError($exception), this might be done passing in an Exception object, and the result will be the same as with our code: a fatal error. If you didn't hear about it yet, that must not happen often, but just let you know that hinting at Exception would probably be safer than JException.

Anyway, I can handle this situation at extension level, so there's no need to actually push that PR from where I stand. It'd be safer I feel, but I can handle on my side, as said.

Rgds

[mbabker]

Well, JError won't die unless we force it to go away. So consider this my taking a stand against letting it live forever and forcing modernization of some code 😆

[weeblr]

Fine with me ;) closing this issue

[adambako]

hi.. i get this error on joomla 3.5.1 after confirm order in virtuemart..
im not a coder.. will this be fixed or how i can get it work? i need it asap.
thx

[brianteeman]

This sounds like an issue being created by something in virtumart and not in the core of joomla. Please ask at forum.virtumart.net or forum.joomla.org

[xristoph]

I have the same error from new user registrations. When the site admin clicks the approval link within the email, the site displays blank white page. When error reporting enabled, get the below error message.

Joomla 3.5.1
JomSocial 4.2.1
sh404SEF 4.7.3.3292
PHP 5.4.16

Catchable fatal error: Argument 1 passed to PlgSystemRedirect::handleError() must be an instance of JException, instance of phpmailerException given in /path2site/mydomain.com/plugins/system/redirect/redirect.php on line 55

It's 3 months later from last post, can anyone provide a fix for this without updating to joomla 3.6?

[brianteeman]

Always update!!

[xristoph]

Thank you. Had I posted last week (prior to 3.6.0 release) what would the answer be?

[weeblr]

Hi @xristoph

Just disable the redirect system plugin. It has no effect with sh404SEF enabled anyway.

Rgds

[xristoph]

Okay, thank you @weeblr greatly appreciated

[weeblr]

You're welcome. Note however that this means you have a 404 on this page, so that's something you have to address.

Rgds

[brianteeman]

And that's what the answer would have been (or stop using sh404 :) )