Page 1 of 1

Function to escape POST data and store it dynamically

Posted: Thu Jan 19, 2006 5:32 am
by jayshields
Hi guys,

I thought of defining a function to save rewriting the same piece of code loads of times, althought it's not totally necessary I want to fix it.

Here is my code:

Code: Select all

//Define a validation method to save re-typing
function validate_input($field, $msgvar = 'msg', $method = '_POST') {
	if($$method[$field]) {
		$$field = mysql_real_escape_string(trim(stripslashes($$method[$field])));
		return TRUE;
	} else {
		$$field = FALSE;
		$$msgvar .= '<font color="red">You haven\'t entered a ' . $field . '.</font><br />';
		return FALSE;
	}
}
I know I probably need to look into variable variables but I don't really understand them or how to implement them.

I've tried doing it like this also:

Code: Select all

//Define a validation method to save re-typing
function validate_input($field, $msgvar = 'msg', $method = 'POST') {
	if($_{$method}[$field]) {
		${$field} = mysql_real_escape_string(trim(stripslashes($_{$method}[$field])));
		return TRUE;
	} else {
		${$field} = FALSE;
		${$msgvar} .= '<font color="red">You haven\'t entered a ' . $field . '.</font><br />';
		return FALSE;
	}
}
But I get the same errors, which with E_ALL on, look like this:

Code: Select all

PHP Notice: Undefined variable: _ in D:\Webspace\wrightandshields.co.uk\wwwroot\collegestuff\carmanage\functions.php on line 10 PHP Notice: Undefined variable: msg in D:\Webspace\wrightandshields.co.uk\wwwroot\collegestuff\carmanage\functions.php on line 15 PHP Notice: Undefined variable: _ in D:\Webspace\wrightandshields.co.uk\wwwroot\collegestuff\carmanage\functions.php on line 10 PHP Notice: Undefined variable: msg in D:\Webspace\wrightandshields.co.uk\wwwroot\collegestuff\carmanage\functions.php on line 15 PHP Notice: Undefined variable: _ in D:\Webspace\wrightandshields.co.uk\wwwroot\collegestuff\carmanage\functions.php on line 10 PHP Notice: Undefined variable: msg in D:\Webspace\wrightandshields.co.uk\wwwroot\collegestuff\carmanage\functions.php on line 15 PHP Notice: Undefined index: img in D:\Webspace\wrightandshields.co.uk\wwwroot\collegestuff\carmanage\addcar.php on line 23 PHP Notice: Undefined variable: msg in D:\Webspace\wrightandshields.co.uk\wwwroot\collegestuff\carmanage\addcar.php on line 27 PHP Notice: Undefined variable: _ in D:\Webspace\wrightandshields.co.uk\wwwroot\collegestuff\carmanage\functions.php on line 10 PHP Notice: Undefined variable: msg in D:\Webspace\wrightandshields.co.uk\wwwroot\collegestuff\carmanage\functions.php on line 15 PHP Notice: Undefined index: img in D:\Webspace\wrightandshields.co.uk\wwwroot\collegestuff\carmanage\addcar.php on line 143 PHP Notice: Undefined index: priv_sale in D:\Webspace\wrightandshields.co.uk\wwwroot\collegestuff\carmanage\addcar.php on line 155 PHP Notice: Undefined index: priv_sale in D:\Webspace\wrightandshields.co.uk\wwwroot\collegestuff\carmanage\addcar.php on line 157
Thanks for any help :)

Posted: Thu Jan 19, 2006 6:01 am
by Jenk

Code: Select all

${'_' . $method}[$field];
However, it isn't best practice to automatically escape anything and everything in the superglobals, only escape what you need to escape, when you need to escape.

If you are looking on cutting down the number of times you need to type out mysql_real_escape_string(), then use the following function:

Code: Select all

function SqlClean ($string)
{
    if (get_magic_quotes_gpc()) {
        $string = stripslashes($string);
    }
    return mysql_real_escape_string($string);
}
Or even better, write up some DB classes, or use existing ones, that do all the escaping for you :P

Posted: Thu Jan 19, 2006 6:06 am
by Maugrim_The_Reaper
Probably need a usage example to see where all the parameters come from.

Where does $$field go to? Before you return true, if this is a function and you're not using GLOBALS, how d use this variable outside the function??? Same for $msgvar...

You also need to be careful about when, and how you escape. If this data is being used in your application logic, it should not be immediately escaped (only stated as being clean/safe after filtering/validation). You should only escape when the context changes - i.e. the variable is being passed to another location, e.g. into HTML, or the database. And even then you should use the right escaping for the right context.

So you only use mysql_real_escape_string JUST BEFORE using the variable in any SQL. You also only use htmlentities('string', ENT_QUOTES, 'UTF-8') JUST BEFORE using it in any HTML. Typically I'd either store the escaped values in a $sql and $html array (to distinguish between escaping methods and contexts) and keep the original validated data in its original form somewhere else (maybe $clean) - this separates original data from escaped data (and may even get you out of tracking when data is escaped or not). I may also have a presentation and data access layer that handles escaping automatically - just to avoid the necessity of thinking about it.

I may be preaching to the converted after misunderstanding something, but escaping before using the data in PHP code (i.e. the underlying logic of the app) can lead to unexpected results - at a minimum it'll play havoc with any REGEX you may be using later on. After all escaping can add extra characters you would need to adjust for in any regex - this is not an ideal situation, and one of the reasons why its recommended to disable (or reverse) magic_quotes_gpc before doing anything else with input (PHP6 apparently is removing this altogether - hurrah!)

Timing really is everything - ;)

Posted: Thu Jan 19, 2006 9:29 am
by jayshields
Thanks guys.

Everytime I use a HTML fom for inserting data into a MySQL database I write out the same validation process for every text field. So this is just to stop me doing so.

Usually I write this out:

Code: Select all

if($_POST['details']) {
		$details = mysql_real_escape_string(trim(stripslashes($_POST['details'])));
	} else {
		$details = FALSE;
		$msg .= '<font color="red">You haven\'t entered the details.</font><br />';
	}
for every field.

so my function saves doing that.

Here is example usage of my function:

Code: Select all

//If the user has submitted some data
if($_POST['submit']) {
	
	//Validate the input fields and prepare the variables for SQL use
	validate_input('model');

	validate_input('reg');

	validate_input('year');
if($model && $reg && $year) {
//Execute query and set $msg to a success message
} else {
//Add try again to the $msg
}
}
echo $msg;

Posted: Thu Jan 19, 2006 9:46 am
by Jenk
a couple things, to check the existance of a variable, you don't want to be using

Code: Select all

if ($var) {}
because if the value of $var = 0, or $var = false, it will fail the if.

Use

Code: Select all

if (isset($var)) {}
Second, you do not want to run stripslashes unecessarily - if the user submitted, for example, something like "C:\My Documents" stripslashes would turn that into "C:My Documents".

Check if magic_quotes_gpc directive is On, like I have shown in my function in a previous reply before using stripslashes.

Posted: Thu Jan 19, 2006 4:56 pm
by jayshields
I understand your stripslashes comment, and I realise that, but at the moment no one will be using blackslashes in my fields :)

Also, the point of if ($var) { //whatever is so == 0 or == FALSE will fail it, thats why i set the var to FALSE if it fails validation, no one will be entering 0's as they are text based fields I'm checking against anyway. I know about isset but sometimes its just quicker to leave it out :)

Posted: Fri Jan 20, 2006 3:09 am
by duk
Jenk wrote:a couple things, to check the existance of a variable, you don't want to be using

Code: Select all

if ($var) {}
because if the value of $var = 0, or $var = false, it will fail the if.

Use

Code: Select all

if (isset($var)) {}
Second, you do not want to run stripslashes unecessarily - if the user submitted, for example, something like "C:\My Documents" stripslashes would turn that into "C:My Documents".

Check if magic_quotes_gpc directive is On, like I have shown in my function in a previous reply before using stripslashes.
isset($var) will just check if is set i think the best solution is to check if is check and if is not empty

Use

Code: Select all

if ((isset($var)) && ($var)) {} //or you can use the empty function

Posted: Fri Jan 20, 2006 4:43 am
by Jenk
jayshields wrote:I understand your stripslashes comment, and I realise that, but at the moment no one will be using blackslashes in my fields :)

Also, the point of if ($var) { //whatever is so == 0 or == FALSE will fail it, thats why i set the var to FALSE if it fails validation, no one will be entering 0's as they are text based fields I'm checking against anyway. I know about isset but sometimes its just quicker to leave it out :)
1. Famous last words...

2. You have $_POST data in the format of if ($_POST['blah']) {} .. if the user enters 0, that will fail. 0 is a form of text.

You are also creating excpetions/errors when you don't need to by running if's like that, which also hinder performance and is just plain old bad practice.

Posted: Fri Jan 20, 2006 5:57 am
by jayshields
Don't get me wrong, I can see exactly where you're coming from, but I am just a lazy coder who cannot be bothered changing my ways for such a task.

As far as I can see, it's only an administration panel which requires login, so I highly doubt anyone will be logging into administration and deliberately trying to enter bogus data. If that's the case, they may aswell just highlight everything in the screen and delete it, it would be much more effective in messing up the system.

In a public HTML form I would probably take a more cautious approach, using isset(), empty() and is_numeric().

Thanks for the suggestions anyway, I will definately consider them in the future :)

Posted: Fri Jan 20, 2006 6:17 am
by Jenk
The number one rule for development, web or not, is to not trust ANY input from external sources. Some people like to take it a step further than that and not trust anything from anywhere, which I can see the reasons for but am yet to fall into that fold.

But anywho, your site, your choice.