views:

103

answers:

2

Disclaimer; I'm fully aware of the pitfalls and "evils" of eval, including but not limited to: performance issues, security, portability etc.

The problem

Reading the PHP manual on eval...

eval() returns NULL unless return is called in the evaluated code, in which case the value passed to return is returned. If there is a parse error in the evaluated code, eval() returns FALSE and execution of the following code continues normally. It is not possible to catch a parse error in eval() using set_error_handler().

In short, no error capture except returning false which is very helpful, but I'm sur eI could do way better!

The reason

A part of the site's functionality I'm working on relies on executing expressions. I'd like not to pass through the path of sandbox or execution modules, so I've ended using eval. Before you shout "what if the client turned bad?!" know that the client is pretty much trusted; he wouldn't want to break his own site, and anyone getting access to this functionality pretty much owns the server, regardless of eval.

The client knows about expressions like in Excel, and it isn't a problem explaining the little differences, however, having some form of warning is pretty much standard functionality.

This is what I have so far:

define('CR',chr(13));
define('LF',chr(10));

function test($cond=''){
    $cond=trim($cond);
    if($cond=='')return 'Success (condition was empty).'; $result=false;
    $cond='$result = '.str_replace(array(CR,LF),' ',$cond).';';
    try {
        $success=eval($cond);
        if($success===false)return 'Error: could not run expression.';
        return 'Success (condition return '.($result?'true':'false').').';
    }catch(Exception $e){
        return 'Error: exception '.get_class($e).', '.$e->getMessage().'.';
    }
}

Notes

  • The function returns a message string in any event
  • The code expression should be a single-line piece of PHP, without PHP tags and without an ending semicolon
  • New lines are converted to spaces
  • A variable is added to contain the result (expression should return either true or false, and in order not to conflict with eval's return, a temp variable is used.)

So, what would you add to further aide the user? Is there any further parsing functions which might better pinpoint possible errors/issues?

Chris.

+1  A: 

I don't think eval will throw you an exception, it will throw normal errors. To catch these place an

ob_start();

before the eval and and after the eval:

if ('' !== $error = ob_get_clean()) {
    // output the error somehow to the client
}

I don't think there's another function like eval with better error handling. There is check_syntax but it validates file only.

PS: You defined the CR and LF constant, did you? They are not predefined in PHP.

nikic
Yes, I did define them, sorry for not mentioning. Fixed above code. As to exceptions, I'm pretty sure it doesn't neither, however, this code might fail in the future after perhaps adding exceptions to it, so it doesn't hurt handling them anyway.
Christian Sciberras
A: 

I've found a good alternative/answer to my question.

First of, let me start by saying that nikic's suggestion works when I set error_reporting(E_ALL); notices are shown in PHP output, and thanks to OB, they can be captured.

Next, I've found this very useful code:

/**
 * Check the syntax of some PHP code.
 * @param string $code PHP code to check.
 * @return boolean|array If false, then check was successful, otherwise an array(message,line) of errors is returned.
 */
function php_syntax_error($code){
    $braces=0;
    $inString=0;
    foreach (token_get_all('<?php ' . $code) as $token) {
        if (is_array($token)) {
            switch ($token[0]) {
                case T_CURLY_OPEN:
                case T_DOLLAR_OPEN_CURLY_BRACES:
                case T_START_HEREDOC: ++$inString; break;
                case T_END_HEREDOC:   --$inString; break;
            }
        } else if ($inString & 1) {
            switch ($token) {
                case '`': case '\'':
                case '"': --$inString; break;
            }
        } else {
            switch ($token) {
                case '`': case '\'':
                case '"': ++$inString; break;
                case '{': ++$braces; break;
                case '}':
                    if ($inString) {
                        --$inString;
                    } else {
                        --$braces;
                        if ($braces < 0) break 2;
                    }
                    break;
            }
        }
    }
    $inString = @ini_set('log_errors', false);
    $token = @ini_set('display_errors', true);
    ob_start();
    $braces || $code = "if(0){{$code}\n}";
    if (eval($code) === false) {
        if ($braces) {
            $braces = PHP_INT_MAX;
        } else {
            false !== strpos($code,CR) && $code = strtr(str_replace(CRLF,LF,$code),CR,LF);
            $braces = substr_count($code,LF);
        }
        $code = ob_get_clean();
        $code = strip_tags($code);
        if (preg_match("'syntax error, (.+) in .+ on line \d+)$'s", $code, $code)) {
            $code[2] = (int) $code[2];
            $code = $code[2] <= $braces
                ? array($code[1], $code[2])
                : array('unexpected $end' . substr($code[1], 14), $braces);
        } else $code = array('syntax error', 0);
    } else {
        ob_end_clean();
        $code = false;
    }
    @ini_set('display_errors', $token);
    @ini_set('log_errors', $inString);
    return $code;
}

Seems it easily does exactly what I need (yay)!

Christian Sciberras
Wow, this looks nice. And again I've learned something new: `break` can have an argument! I will later cast a look at this code to really understand what it does.Only one note: If there are heavy parse errors in the code, `token_get_all` can output errors by itself, e.g. if you are using `\\` in the source (in PHP <5.3).
nikic