PHP newbie trying to design a search engine, some advice needed!

[Josh5442]

Hi all,

 

My name is Josh, and I'm a fairly in-experienced php programmer. Anyways, I'm creating a web-site and the backbone is going to be php. I'm building a products database, and trying to make a good search engine. Here is my problem, I got it all to work correctly, but I know it's wide open to injection and it's written half-assed. I was wondering if someone could point me in the right direction on the "correct" way to write what I'm trying to do. Here is my part of my current code:

 

if ($_GET["brand"] == NULL) {$var_brand = "";}
elseif ($_GET["brand"]) {$var_brand = "AND brand_name = '$_GET[brand]'";}

if ($_GET["category"] == NULL) {$var_cat = ""; }
elseif ($_GET["category"]) {$var_cat = "AND cat_name = '$_GET[category]'";}

if ($_GET["sub_category"] == NULL) {$var_scat = ""; }
elseif ($_GET["sub_category"]) {$var_scat = "AND s_cat_name = '$_GET[sub_category]'"; }

if ($_GET["product_type"] == NULL) {$var_protype = ""; }
elseif ($_GET["product_type"]) {$var_protype = "AND product_type = '$_GET[product_type]'"; }
///___________________________///
////////////////////////////////
///END IF STATEMENTS FOR SQL///
//////////////////////////////


//////////////////////////////
///START OP FOR SEARCH TYPE//
////////////////////////////
//_______________________//
switch ($_GET["type"]){
case "search":
	$sql = "select * from products WHERE active = 'yes' $var_cat $var_scat $var_brand $var_protype ORDER BY brand_name ASC"; 
	$result = mysql_query($sql,$conn) or die(mysql_error());

 

Now, basically what it does is takes any data input into the browser and pops it into the SQL statement. I used the if and else statements because the database wouldn't work properly if one field wasn't entered (very crappy coding, yes). So, if that field is not entered in the browser it's set as nothing. Can someone help me out and give me some insight on how to write this correctly? I would greatly appreciate it.

 

Josh

[cooldude832]

for data that looks like it should be in a mysql select I like to use array in to array out

 

With your brands make an array called brand and create your select drop down from the array and then verify that it is in_array on processing

i.e

<?php
$brands = array("Brand A", "Super Brand", "Gold Brand");
#Output of select
echo "<select name=\"brand\">";
foreach($brands as $value){
echo "<option value=\"".$value."\">".$value."</option>\n";
}
echo "</select>";

#Verify the get vars
if(in_array($_GET['Brand'],$brands)){
#Its a valid input
}
else{
#User tried to put something else in reject brand or use a default
}
?>

 

That is best for the set pool of choices.

 

If its data like a zip code or text string verify it as best you can and run mysql_real_escape_string() on it.

[Josh5442]

Okay, I get you, If I'm reading that correctly that's just going to verify that the data that's entered matches what's in the array, correct? That's not really the issue I have with it, problem is, say a user wants to search by a category and not a brand, if only a category is entered the code currently will only search by category; It works the way it's suppost to, but I know there has to be a better way to code that. So basically say:

 

<?php

if ($_GET["brand"] == NULL) {$var_brand = "";}
elseif ($_GET["brand"]) {$var_brand = "AND brand_name = '$_GET[brand]'";}


$sql = "select * from products WHERE $var_cat ORDER BY brand_name ASC"; 

?>

 

Is there a better way to do this?

[Josh5442]

Here is the whole code:

 

<?php

if ($_GET["brand"] == NULL) {$var_brand = "";}
elseif ($_GET["brand"]) {$var_brand = "AND brand_name = '$_GET[brand]'";}

if ($_GET["category"] == NULL) {$var_cat = ""; }
elseif ($_GET["category"]) {$var_cat = "AND cat_name = '$_GET[category]'";}

if ($_GET["sub_category"] == NULL) {$var_scat = ""; }
elseif ($_GET["sub_category"]) {$var_scat = "AND s_cat_name = '$_GET[sub_category]'"; }

if ($_GET["product_type"] == NULL) {$var_protype = ""; }
elseif ($_GET["product_type"]) {$var_protype = "AND product_type = '$_GET[product_type]'"; }
///___________________________///
////////////////////////////////
///END IF STATEMENTS FOR SQL///
//////////////////////////////


//////////////////////////////
///START OP FOR SEARCH TYPE//
////////////////////////////
//_______________________//

if ((empty($_GET["brand"])) && (empty($_GET["category"])) && (empty($_GET["sub_category"])) && (empty($_GET["product_type"]))) {
header( 'Location: index.php?error=noProd' );
	} else {
		$sql = "select * from products WHERE active = 'yes' $var_cat $var_scat $var_brand $var_protype ORDER BY brand_name ASC"; 
		$result = mysql_query($sql,$conn) or die(mysql_error());

		if (mysql_num_rows($result) < 1) {header( 'Location: index.php?error=noProd' ); 
			} elseif (mysql_num_rows($result) > 0){
			while ($array = mysql_fetch_array($result)) {
				$pid = $array['product_id'];
				$text = $array['html'];
				$catname = $array['cat_name'];
				$scatname = $array['s_cat_name'];
				$bname = $array['brand_name'];
				$ptype = $array['product_type'];
				$pthumb = $array['thumbnail'];
				$ptitle = $array['title'];
				$pdescription = $array['description'];
				$pkeywords = $array['keywords'];
				$table .= "Removed, way too long.";
			}
		}
	}
?>

 

Any help is greatly appreciated!

[Josh5442]

Anyone care to chime in? I'm really stuck on exactly what to do. I mean, this is working for me but is it correct?

[Trekx]

It seems alright to me, though I might write:

 

function optionalWhereFieldMatchesGet($fieldName, $getName) 
{ 
    if($_GET[$getName] == NULL) 
        return "";
    else
        return " AND $fieldName = '" . mysql_real_escape_string($_GET[$getName], $mysql) . "'";

}

$where = "active = 'yes'";
$where .= optionalWhereFieldMatchesGet("brand_name", "brand");
$where .= optionalWhereFieldMatchesGet("cat_name", "category");
$where .= optionalWhereFieldMatchesGet("s_cat_name", "sub_category");
$where .= optionalWhereFieldMatchesGet("product_type", "product_type");

 

The function needs a better name, but if it were part of a library, that would be fairly close to minimal. Using the same names for the GET and the database field would simplify it even more.

[mikefrederick]

Here's an example where I just did what you are trying to, might take you a few to read through but it's right:

 

 

 

if($title=="Enter a phrase..." || $title=='') { $titleone=''; } else { $titleone="name like '%$title%' and"; }

if($amenities=="Enter values seperated by commas (,)..." || $amenities=='') { $amenitiesone=''; } else {

$test=explode(",",$amenities);

$numtest=count($test);

$x=0;

while ($x<$numtest) {

$amenitiesone.="and vr_amenities like '%$test[$x]%' ";

 

 

$x++; }

}

 

 

 

if($type=="Select") { $types=''; } else { $types=$type; }

if ($images=='on') { $imagesone=''; } else { $imagesone="and images!=''"; }

if ($children=='on') { $childrenone=''; } else { $childrenone="and children='1'"; }

if ($smoking=='on') { $smokingone=''; } else { $smokingone="and smoking='1'"; }

if ($pets=='on') { $petsone=''; } else { $petsone="and pets='1'"; }

if ($type=='' || $type=='Select...') { $typeone=''; } else { $typeone="and type='$type'"; }

if ($bedrooms=='' || $bedrooms=='Select...') { $bedroomsone=''; } elseif ($bedrooms=='6') { $bedroomsone="and bedrooms>='$bedrooms'"; }

else {  $bedroomsone="and bedrooms='$bedrooms'"; }

 

if ($bathrooms=='0' || $bathrooms=='Select...') { $bathroomsone=''; } elseif ($bathrooms=='6' || $bathrooms=='6+') { $bathroomsone="and bathrooms>='$bathrooms'"; }

else {  $bathroomsone="and bathrooms='$bathrooms'"; }

if($_POST['order']) { $order=$_POST['order']; } else { $order='pstate'; }

 

$sqly="select * from properties where $titleone status='1' and pcountry='$country' $imagesone $childrenone $typeone $smokingone $petsone $bedroomsone $bathroomsone $amenitiesone order by $order";

$rs=mysql_query($sqly);

$rows=mysql_num_rows($rs);

 

 

[Josh5442]

It seems alright to me, though I might write:

 

 

Okay it worked halfway, it sees active='yes', but it does not see the function inside the SQL statement....Can you even insert a function inside and SQL statement?

 

 

 

Code is:

 

<?php

function search($fieldName, $getName) 
{ 
    if($_GET['getName'] == NULL) 
        return "";
    else
       return " AND $fieldName = '$_GET[getName]'";

}

$where = "active = 'yes'";
$where .= search("brand_name", "$_GET[brand]");
$where .= search("cat_name", "$_GET[category]");
$where .= search("s_cat_name", "$_GET[subcategory]");
$where .= search("product_type", "$_GET[product_type]");


///
		$sql = "select * from products WHERE $where ORDER BY brand_name ASC"; 
		$result = mysql_query($sql,$conn) or die(mysql_error());

		if (mysql_num_rows($result) < 1) {header( 'Location: search_test.php?error=noProd&narrow=yes' ); 
			} elseif (mysql_num_rows($result) > 0){
			while ($array = mysql_fetch_array($result)) {
				$pid = $array['product_id'];
				$text = $array['html'];
				$catname = $array['cat_name'];
				$scatname = $array['s_cat_name'];
				$bname = $array['brand_name'];
				$ptype = $array['product_type'];
				$pthumb = $array['thumbnail'];
				$ptitle = $array['title'];
				$pdescription = $array['description'];
				$pkeywords = $array['keywords'];
				$table .= "Removed";
			}
		}
?>

[Josh5442]

Err my fault, I had an error. But it still doesn't work, the SQL statement doesn't see it....

 

<?php

function search($fieldName, $getName) 
{ 
    if($_GET['getName'] == NULL) 
        return "";
    else
       return " AND $fieldName = '$_GET[getName]'";

}

$where = "active = 'yes'";
$where .= search("brand_name", $_GET['brand']);
$where .= search("cat_name", $_GET['category']);
$where .= search("s_cat_name", $_GET['subcategory']);
$where .= search("product_type", $_GET['product_type']);


///
		$sql = "select * from products WHERE $where ORDER BY brand_name ASC"; 
		$result = mysql_query($sql,$conn) or die(mysql_error());

		if (mysql_num_rows($result) < 1) {header( 'Location: search_test.php?error=noProd&narrow=yes' ); 
			} elseif (mysql_num_rows($result) > 0){
			while ($array = mysql_fetch_array($result)) {
				$pid = $array['product_id'];
				$text = $array['html'];
				$catname = $array['cat_name'];
				$scatname = $array['s_cat_name'];
				$bname = $array['brand_name'];
				$ptype = $array['product_type'];
				$pthumb = $array['thumbnail'];
				$ptitle = $array['title'];
				$pdescription = $array['description'];
				$pkeywords = $array['keywords'];
				$table .= "Removed";
			}
		}
?>

[Trekx]

If you're passing $_GET['...'] to the function (which I do like better), the function should be:

 

function search($fieldName, $value) 
{ 
    if($value == NULL) 
        return "";
    else
       return " AND $fieldName = '$value'";

}

$where = "active = 'yes'";
$where .= search("brand_name", $_GET['brand']);
$where .= search("cat_name", $_GET['category']);
$where .= search("s_cat_name", $_GET['subcategory']);
$where .= search("product_type", $_GET['product_type']);

 

Using a function also makes it easier to add mysql_real_escape_string().