[SOLVED] Securing posted form data before storing in database (function not working)

[techtheatre]

So i have the following function i have been working on...it seems (to me) that it logically SHOULD work and i plan to run it on all $_POST and $_GET arrays upon page load on all my form submittal pages before i insert the data to my databases.  The function should take a variable that i pass (either a string or an array) and parse it however is necessary (strip slashes and convert htmlentities...do i need anything else i have left out?),  I am trying to set up the function so that i can simply call it on all my pages at the top and immediately secure all my forms.  I am using var_dump($_GET) for now at the top and bottom of the "processing" page to see what is happening and if it is working, but it keeps returing bool(false) for each value within the array.  Please let me know what i am doing wrong, and also if there are other considerations i should be making (or an easier way to accomplish this).  THANKS!!

 

function PrepareArrayOrStringForDB($val,$stripSlashes=false, $htmlEntities=false)
{
  	if( is_array($val) )
  	{
  		//if we need to strip slashes (default is false, but appropriate value (true/false) can be passed by calling get_magic_quotes_gpc() in the function call)
  		if( $stripSlashes )
  		{
  		foreach( $val as $ky=>$vl)
  		{
			$val[$ky]=stripslashes( $vl );
  		}
  		}
  		
  		//if we need to convert characters to HTML equivalents
	if( $htmlEntities )
  		{
  			foreach( $val as $ky=>$vl)
  		{
			$val[$ky]=htmlentities( $vl );
  		}
  		}
  		
  		//this should always be done before values are inserted into a database
  		foreach( $val as $ky=>$vl)
  		{
		$val[$ky]=mysql_real_escape_string( $vl );
  		}
  	}
  	else
  	{
  		//if we need to strip slashes
	if( $stripSlashes )
  		{
  			$val = stripslashes( $val );
  		}
  		
  		//if we need to convert characters to HTML equivalents
	if( $htmlEntities )
  		{
  			$val = htmlentities( $val );
  		}
  		
  		//this should always be done before values are inserted into a database
  		$val= mysql_real_escape_string($val);
  	}
  	return $val;
}

 

 

 

<?php

var_dump($_GET);
echo "<br /><br />";

$_GET = PrepareArrayOrStringForDB($_GET, get_magic_quotes_gpc(), true);

var_dump($_GET);

?>

[bubblegum.anarchy]

There is a limitation if only one value out of the entire array requires html entities to be converted.

 

... and use get_magic_quotes_gpc internally, if that is the only condition used for stipslashes.

 

I prefer prepareing post data for database insertion in a couple of seperate steps.

 

First just lightly clean up the data

 

foreach ($_POST as $key => $item) $_POST[$key] = trim( get_magic_quotes_gpc() ? stripslashes($item) : $item );

 

... and then the more complex type validation that includes string quoting or returns NULL;

 

mysql_query($query = "INSERT INTO table SET value = ".to_string($_POST['value']);

// return an escaped and quoted (if required html entity converted) value or null
function to_string($value, $htmlentities=false)
{
     return is_null($value) || empty($value) ? "NULL" : "'".mysql_real_escape_string( $htmlentities ? htmlentities($value) : $value )."'";
}

 

NOTE: The above function can not be used to convert numbers since empty(0) returns true.

[techtheatre]

There is a limitation if only one value out of the entire array requires html entities to be converted.

 

Right...except i can run the function on specific variables too...and control whether htmlentities is done or not...that is why i have it set up as one of the variables (with false as the defualt) that gets passed to the function.  Your methods do look much cleaner to use.  A definitely agree that i should use get_magic_quotes_gpc internally since that is the only condition used for stipslashes (at least now).

 

One (or two) question(s):

 

to solve the problem with passing a numeric value to the second function...couldn't the line be changed to:

 

return is_null($value) || ( empty($value) && $value!=0 ) ? "NULL" : "'".mysql_real_escape_string( $htmlentities ? htmlentities($value) : $value )."'";

 

I have heard that the ternary operator (aka ? degrades performance slightly. For better performance, the longer-winded version is better (even if it requires more code).  (reference: Ilya's talk at EZ-Norway).  As such...using my example above...would the following work?

 

if( is_null($value) || ( empty($value) && $value!=0 ) )
{
   return "NULL";
}
else
{
   if ( $htmlentities )
   {
       return htmlentities($value);
   }
   else
   {
       return $value ;
   }
}

 

 

THANKS!

[bubblegum.anarchy]

yeah empty($value) && value != 0 would be fine but I would write value != 0 first considering the empty condition depends on the outcome.

 

I often use  <condition> ? <true statement>  : <false statement> and always assumed that the syntax was just shorthand (I must of read that somewhere).

[techtheatre]

good point.  Thanks!  I will switch the order of those...

 

As far as the <condition> ? <true> : <false> it is usually referred to as shorthand (and is a heck of a lot quicker/easier to use)...i don't know if it is really any slower...and certainly would not matter at the low level that I program...but i thought i would mention it just in case since i had read about it.  I am trying to start using the long format when i remenber.  Anyway, probably only a small matter that really doesn't affect anythign anyway.

 

Thank you for your help!

[bubblegum.anarchy]

I performed some tests that produced some interesting results:

<?php
for ($i = 0; $i < 500000; $i++)
{
     $_POST['value'] = " ";

     preg_match('/^\s*$/', $_POST['value']) ? $_POST['value'] = "value" : $_POST['value'] = $_POST['value'];
}

SCRIPT END - Script Execution Time: 1.4779 Seconds 
SCRIPT END - Script Execution Time: 1.4797 Seconds 
SCRIPT END - Script Execution Time: 1.4729 Seconds 

for ($i = 0; $i < 500000; $i++)
{
     $_POST['value'] = " ";

     $_POST['value'] = preg_match('/^\s*$/', $_POST['value']) ? "value" : $_POST['value'];
}

SCRIPT END - Script Execution Time: 1.4364 Seconds
SCRIPT END - Script Execution Time: 1.4353 Seconds  
SCRIPT END - Script Execution Time: 1.4458 Seconds

for ($i = 0; $i < 500000; $i++)
{
     $_POST['value'] = " ";

     if (preg_match('/^\s*$/', $_POST['value'])) // TRUE
     {
          $_POST['value'] = "value";
     }
     else
     {
          $_POST['value'] = $_POST['value'];
     }	
}

SCRIPT END - Script Execution Time: 1.3963 Seconds 
SCRIPT END - Script Execution Time: 1.4059 Seconds 
SCRIPT END - Script Execution Time: 1.4021 Seconds 


for ($i = 0; $i < 500000; $i++)
{
     $_POST['value'] = " ";

     if (preg_match('/^\s*$/', $_POST['value'])) // TRUE
     {
          $_POST['value'] = $_POST['value'];
     }
}


SCRIPT END - Script Execution Time: 1.4746 Seconds 
SCRIPT END - Script Execution Time: 1.4774 Seconds 
SCRIPT END - Script Execution Time: 1.4808 Seconds 
?>

 

The results for the last two tests are quiet contradictory, could someone please verify the last two tests!! - The test result suggests that adding a superfluous else to an if would decrease the processing time.

 

I may have to stop using ternary operators so often.