Page 1 of 1

Help With SQL Statement From URL Parameters

Posted: Fri Oct 19, 2012 2:04 am
by gnatmarie
I think that I need some help with the way I am piecing together a SQL statement from URL parameters. I'm using this statement to sort products by colors, styles, sizes, etc. based on whatever a user clicks. Say a user clicks on the color "berry" and then a style option of "short sleeve" and "sleeveless" - I need a sql statement that searches for a match in the color "berry" and "short sleeve" or the color "berry" and "sleeveless". The following code is what I've written and I have a feeling that it's a bad way to go.

Code: Select all

if (isset($_GET['color']))
	$color_filter = $_GET['color'];
	$color = explode(',', $color_filter);
if (isset($_GET['fit']))
	$fit_filter = $_GET['fit'];
	$fit = explode(',', $fit_filter);
if (isset($_GET['style'])) 
	$style_filter = $_GET['style'];
	$style = explode(',', $style_filter);
if (isset($_GET['fabric'])) 
	$fabric_filter = $_GET['fabric'];
	$fabric = explode(',', $fabric_filter);
if (isset($_GET['chamois'])) 
	$chamois_filter = $_GET['chamois'];
	$chamois = explode(',', $chamois_filter);
if (isset($_GET['ride'])) 
	$ride_filter = $_GET['ride'];
	$ride = explode(',', $ride_filter);

$conditions = '';

if (!empty($color_filter) || !empty($fit_filter) || !empty($style_filter) || !empty($fabric_filter) || !empty($chamois_filter) || !empty($ride_filter)) {
	$i = 0;
	foreach ($color as $color_value) {
		foreach ($fit as $fit_value) {
			foreach ($style as $style_value) {
				foreach ($fabric as $fabric_value) {
					foreach ($chamois as $chamois_value) {
						foreach ($ride as $ride_value) {
							if ($i == 0)
								$conditions .= 'subcategory = "' . $sub . '"';
							if ($i !== 0)
								$conditions .= ' OR subcategory = "' . $sub . '"';
							if (!empty($fabric_filter) && $fabric_value == 'solid' && $sub == 'tops')
								$conditions .=  ' AND name LIKE "%' . $fabric_value . '%" AND active = 1';
							if (!empty($fabric_filter) && $fabric_value == 'print' && $sub == 'tops')
								$conditions .=  ' AND name NOT LIKE "%solid%" AND active = 1';
							if (!empty($color_filter))
								$conditions .=  ' AND colors LIKE "%' . $color_value . '%" AND active = 1';
							if (!empty($fit_filter))
								$conditions .=  ' AND fit = "' . $fit_value . '" AND active = 1';
							if (!empty($style_filter))
								$conditions .=  ' AND style LIKE "%' . $style_value . '%" AND active = 1';
							if (!empty($chamois_filter))
								$conditions .=  ' AND specs LIKE "%' . $chamois_value . '%" AND active = 1';
							if (!empty($ride_filter))
								$conditions .=  ' AND ride LIKE "%' . $ride_value . '%" AND active = 1';
							$i++;
							}
					}
				}
			}
		}
	}
	$filter = $conditions;
}
This code does exactly what I need it to but it does throw out undefined variable notices. Does anyone know of a better way that I can achieve what I with this code or point me in the direction of starting new code that's more efficient? Are the nested foreach's a bad way to go?

Re: Help With SQL Statement From URL Parameters

Posted: Fri Oct 19, 2012 6:35 am
by Mordred

Code: Select all

if (isset($_GET['color'])) { //without brackets the explode is not within the if (this is not like in python)
         $color_filter = $_GET['color'];
         $color = explode(',', $color_filter);
         foreach ($color as $i=>$_) $color[$i] = mysql_real_escape_string($color[$i]); //escape!
} else {
         $color = Array();
}

Re: Help With SQL Statement From URL Parameters

Posted: Sun Oct 21, 2012 1:51 am
by gnatmarie
Thank you for the help with that. Your solution didn't exactly work the way I needed it to though. I realized the problem lied within my select statement. It was far more complicated than it should have been. So I came up with this instead:

Code: Select all

$valid  = array( 'color', 'fit', 'style', 'fabric', 'chamois', 'ride');

if ($_GET) {
	foreach($valid as $column) {
	   // does the key exist?
	   if (isset($_GET[$column])) {
			  // add it to the search array.
			  $esc = mysql_real_escape_string($_GET[$column]);
			  $options = str_replace(',', '|', $esc);
			  $filter .= ' AND ' . $column . ' REGEXP \'' . $options . '\'';
	   }
	}
}

Re: Help With SQL Statement From URL Parameters

Posted: Tue Oct 23, 2012 6:14 am
by Mordred
Much better. And even though it doesn't affect you in this instance, you should fix this in principle:
Escaping should be the last thing you do to a string before placing it in a query.
Also, double quotes make the code more readable:

Code: Select all

                          
$options = str_replace(',', '|', $_GET[$column]);
$esc = mysql_real_escape_string($options);
$filter .= " AND $column  REGEXP  '$esc' ";
Also, research fulltext search, this REGEXP business is only suitable for small tables.

Re: Help With SQL Statement From URL Parameters

Posted: Tue Oct 23, 2012 10:11 pm
by gnatmarie
I actually changed it to use LIKE instead of REGEXP because it was faster. REGEXP was taking about .0007 seconds and LIKE took about .0005 seconds. Should I use fulltext if my table has 39 rows?

Here is the new code:

Code: Select all

$valid  = array( 'color', 'fit', 'style', 'fabric', 'chamois', 'ride');
if ($_GET) {
	foreach($valid as $column) {
	   if (isset($_GET[$column])) {
			  $esc = mysql_real_escape_string($_GET[$column]);
			  $explode = explode(',', $esc);
			  $new = array();
			  foreach ($explode as $value) {
				  $new[] .= $column . ' LIKE "%' . $value . '%"';
			  }
			  $implode = implode(' OR ', $new);
			  $filter .= ' AND (' . $implode . ')';
			  
	   }
	}
}
That code will ouput a statement that looks along the lines of this if a parameter is set: AND (color LIKE "%berry%" OR color LIKE "%amethyst%") AND (fabric LIKE "%print%"). Where should I put the mysql_real_escape_string and why should escaping be the last thing I do to a string? Should I escape in the second foreach loop? Thank you for your help. I really do appreciate it!

Re: Help With SQL Statement From URL Parameters

Posted: Wed Oct 24, 2012 4:21 am
by Mordred
For 38 lines any string matching method is fine for all practical purposes.

As I said before, in this particular code it wouldn't matter, since what you do after the escaping cannot be used to break it.
That said, it should be like this:

Code: Select all

           if (isset($_GET[$column])) {
                           $explode = explode(',', ($_GET[$column]);
                           $new = array();
                           foreach ($explode as $value) {
                                   $value = mysql_real_escape_string($value);
                                  // In other cases, you might or might not also want to prevent the user to enter LIKE metacharacters like % and _ here. For %$value% - type of matches it doesn't really matter anymore.
                                   $new[] .= $column . ' LIKE "%' . $value . '%"';
                           }
                           $implode = implode(' OR ', $new);
                           $filter .= ' AND (' . $implode . ')';
                           
            }