Geeklog-Core / geeklog

Geeklog - The Secure CMS.
https://www.geeklog.net
24 stars 19 forks source link

If PHP is used by the Staticpage lets try to catch any parse errors as it is now possible in PHP 7 #1038

Closed eSilverStrike closed 4 years ago

eSilverStrike commented 4 years ago

If PHP is used by the Staticpage lets try to catch any parse errors when the page loads (and on save of the page) as it is now possible in PHP 7. Obviously we cannot catch logic errors that traps the code in an infinite loop or something but this should catch most mistakes.

In PHP 7+, eval() terminates the script if the evaluated code generate a fatal error. For example:

<?php
$response = @eval('$content = (100 - );');
if (error_get_last()){
    echo 'Show your custom error message';
    //Or you can 
    print_r(error_get_last());
}
?>

To catch it, do:

<?php
try {
    eval('$content = (100 - );');
} catch (ParseError $e) {
    $content = null;
    echo 'Caught exception: '.$e->getMessage()."\n";
}
?>

and lets return an error message of some sort after saving. (like how we catch errors when loading template staticpages, search for libxml_use_internal_errors in code)

See: https://stackoverflow.com/questions/10085284/how-to-handle-parse-error-for-eval-function-in-php https://www.php.net/manual/en/function.eval.php

eSilverStrike commented 4 years ago

Related to https://github.com/Geeklog-Plugins/autotags/issues/12

mystralkk commented 4 years ago

Implemented COM_handleEval() with change set 6b2e19d.

eSilverStrike commented 4 years ago

@mystralkk - After some tests I had to do a few fixes. In the template class I had to put back the eval php code. I hate to duplicate the code but the template code passed contains references to the template class like '$this->...' which caused PHP errors since COM_handleEval is outside the scope of the template class (since obviously COM_handleEval is not declared inside the class) . Do you know a way around this or should we leave it as is?

mystralkk commented 4 years ago

No. I tried to use the Closure class and its bindTo() method to refer to the Template class, but this way doesn't work since Template::val_echo() method is private and cannot be called from within the Closure class.

eSilverStrike commented 4 years ago

Thanks for looking into it. The code seems to only get executed when the template class function set_view is used (like in the staticpages plugin when a page is used as a template).

The normal cached template files also use $this-> but don’t have this issue as it is not evaluated in this part of the code.

I don’t see any other way at the moment to get it working using the COM_handleEval.

eSilverStrike commented 4 years ago

Forgot to mention this issue in the commit 78c42b233726342ef900cd110d9e4e656b0f0c65

Now that I think of it though having the PHP parse error check on save well create an issue if the php contains a redirect or something similar there by stopping the execution of code and the actually saving of the PHP. Wonder if I should add a config option to allow overriding this since in most cases it shouldn't be an issue...