MySQL Query - Only including variables with data

[WickidGRAFX]

<?php
$tags = $_POST["search-tags"];
$age = $_POST["search-age"];
$rating = $_POST["search-rating"];
$city = $_POST["search-city"];
$state = $_POST["search-state"];

if (!$tags = "") { $tags = " WHERE `tags`='$tags'"; }
if (!$age = "") { $age = " WHERE `age`='$age'"; }
if (!$rating = "") { $rating = " WHERE `rating`='$rating'"; }
if (!$city = "") { $city = " WHERE `city`='$city'"; }
if (!$state = "") { $state = " WHERE `state`='$state'"; }
?>

This is all I got so far...
This is for a search page that displays the results in a table. I have all the other code I need but I'm having issues trying to ONLY include the search query variables that have data.

I would structure the query string like this:

$result = mysql_query( "SELECT * FROM `bgc_models` " . $tags . $age . $rating . $city . $state ) or trigger_error( mysql_error() );

But i then have the issue of how do I insert the "AND" where it's needed? I assume there has to be and easier way to do this. Any suggestions?

[WickidGRAFX]

i forgot to mention that "rating" will always have a value and I have adjusted my code to reflect that.

[diafol]

$post = array_map("mysql_real_escape_string",$_POST);
$s_where = "";
foreach($post as $k=>$v){
	if(substr($k,0,7) == 'search-' && $v != '')$r_where[] = substr($k,7) . " = '$v'" ;	
}
if(isset($r_where))$s_where = " WHERE " . implode(" AND ", $r_where);
$result = mysql_query( "SELECT * FROM `bgc_models`{$s_where}" ) or trigger_error( mysql_error() );

how's that? not tested.

[Stefano Mtangoo]

introducing variables into query will expose you to dangers of SQL injection. it is bad practice AFA security is concerned. I would have if..else for that, something like below. Also instead of using mysql_** I would use PDO or MySQLi

$tags = $_POST["search-tags"];
$age = $_POST["search-age"];
$rating = $_POST["search-rating"];
$city = $_POST["search-city"];
$state = $_POST["search-state"];

$sql = "";
if($tags==""){
    //tags is empty, write query excluding age column
    $sql = "SELECT * FROM bgc_models WHERE age = '$age'  AND rating ='$rating' AND city = '$city' AND state='$state' ";

}else if($age==""){
    //age is empty, write query excluding age column
    $sql = "SELECT * FROM bgc_models WHERE tags = '$tags'  AND rating ='$rating' AND city = '$city' AND state='$state' ";
}
//more if else
else{
//no empty field, write full query
$sql = "SELECT * FROM bgc_models WHERE age = '$age'  AND tags = '$tags'  AND rating ='$rating' AND city = '$city' AND state='$state' ";
}
$res = mysql_query($sql) or die(mysql_error());
//more code

[diafol]

its a lot of code ev - what if there were 15 search terms or more?
I'd have a list of search-keys and place them in an array:

$post = array_map("mysql_real_escape_string",$_POST); //clean input
$s_where = ""; //set where clause to nothing as default
$params = array("tags","age","rating","city","state"); //search-* fields to include
foreach($post as $k=>$v){
	//if the post key is in the array, add the value to the $r_where array 
	if(substr($k,0,7) == 'search-' && $v != '' && in_array(substr($k,7),$params))$r_where[] = substr($k,7) . " = '$v'" ;	
}
//if $r_where exists, create the where clause
if(isset($r_where))$s_where = " WHERE " . implode(" AND ", $r_where);
//run the query
$result = mysql_query( "SELECT * FROM `bgc_models`{$s_where}" ) or trigger_error( mysql_error() );

This way if you need to add extra search fields, you just add a value to the $params array - otherwise you'd need to add a whole conditional statement.

However, I agree with ev, PDO is safer, although if you clean your input, you should be OK. The code above does not validate expected data, e.g. integer, date etc.

[Stefano Mtangoo]

its a lot of code ev - what if there were 15 search terms or more?
I'd have a list of search-keys and place them in an array:

$post = array_map("mysql_real_escape_string",$_POST); //clean input
$s_where = ""; //set where clause to nothing as default
$params = array("tags","age","rating","city","state"); //search-* fields to include
foreach($post as $k=>$v){
	//if the post key is in the array, add the value to the $r_where array 
	if(substr($k,0,7) == 'search-' && $v != '' && in_array(substr($k,7),$params))$r_where[] = substr($k,7) . " = '$v'" ;	
}
//if $r_where exists, create the where clause
if(isset($r_where))$s_where = " WHERE " . implode(" AND ", $r_where);
//run the query
$result = mysql_query( "SELECT * FROM `bgc_models`{$s_where}" ) or trigger_error( mysql_error() );

This way if you need to add extra search fields, you just add a value to the $params array - otherwise you'd need to add a whole conditional statement.

I agree with that ardav, but one thing you have forgotten: you know how dangerous what you have just done and mitigation and s/he is likely not knowing. it increases angles hacker can use can use to penetrate your app.

If he was experienced, I would say s/he would just build list of column into array of whitelist and check if hacker have tempered with column name in POST array. So your code (which is simpler and extensible) plus whitelist of column plus parametric queries makes simple and powerful code!

However, I agree with ev, PDO is safer, although if you clean your input, you should be OK. The code above does not validate expected data, e.g. integer, date etc.

Validation is alway our pitfall. so even if I use PDO I will still try best to do validation

One thing I must admit is, since I jumped into security thing I'm sec-paranoid ;)

[diafol]

I agree with that ardav, but one thing you have forgotten: you know how dangerous what you have just done and mitigation and s/he is likely not knowing.

What have I done??

sanitized input (mysql_real_escape_string)
limited allowed fields to add (in_array)

I suppose you could do an extra validation on datatype and length etc, but that's probably beyond the scope of the original question.