What Does This Code Do? [Resolved, Comments Welcome]

[Travis G]

function compare($var1,$var2)
{
	$string = "if($var1 $var2)
	{
		\$res=1;
	}
	else
	{
		\$res=0;
	}";

	eval("$string;");
	return $res;
}

I'm trying to make sense of this code in this busted PHP application at work. Just to give you an idea, this function is usually called compare("foo", "==bar") . The quotes around the if blocks, the lack of a logic operator, the dereferencing of $res, the random eval(), I don't get it.

[TalonSoftware]

basicaly it does a replace, so $var1 is replaced with foo and $var2 gets replaced with ==bar.... which results in the following:
if(foo==bar) {.....

then the eval gets called to evaluate the string - basicaly executing it. And returns the results.

[Travis G]

Oh, I get it now. The backslashes aren't dereferencing, they are escaping. The entire if block is a string literal and doesn't get parsed until the eval().

Thanks. It isn't working as it should, it's giving false positives. Now that I understand what it is doing, I think I can fix it.

Seems to me that if(foo ==bar) isn't the same as if("foo"=="bar"), which is what I think they wanted.

[Travis G]

The cases with the false positives had an additional character being interpreted as an operator. For example: compare("-foo", "==bar"). I've made some changes to avoid these cases, but I can think of other cases that would break this function.

function thgcompare($var1,$var2) {

  eval ("\$res = (\"$var1\"$var2);");
  return $res;

}

[Travis G]

More improvements.

  function thgcompare($var1,$var2) {

    $var2 = preg_replace('/([a-z0-9]+.*$)/i', '"\1"', $var2);
    eval("\$res = (\"$var1\"$var2);");
    return $res;

  }

I'm sure it is possible to break this, but it is a little more robust. I probably should've started my regex on non logic operator characters and not on alpha numeric characters.

[CornedBee]

Can you explain why there is need for such a function in the first place? This looks like a problem that would best be killed at the root - and the root here is the eval.

[Travis G]

Once you see the entire application you realize that the root is well beyond this function. The whole thing is an ugly mess.

In the database for this application, you can enter conditions for some tests that will return a string of text. You then enter the values you want to use to find a key part of that string. Lastly, you enter what that key needs to be evaluated against. For example, ping a machine, or run a script, or call on cURL Some of the scripts will return "Application-OK-online" or some such. The values to find the key would be "-,-", meaning you want what is between the dashes. Then in the database would be "==OK". For the ping you could enter "<35", for the cURL you could enter "!=404".

I like the ingenuity of trying to do logic operations on the fly, but I think it was the wrong way to go. I could think of several different ways to do this. The first is just simply using regex.

I don't have the latitude to re-write the entire application, but I will definately fix this little function.

[CornedBee]

Sounds awful. And dangerous.