Solved

MYSQL checkbox query

Posted on 2008-10-14
25
497 Views
Last Modified: 2012-05-05
Hello experts!!!

I have created a form where users select search values from a combination of textfields and checkboxes. The text field part is fine but I can't get the checkboxes to return any values from the query. Basically the checkboxes have a selected value of 'sony', 'panasonic', etc

<input type="checkbox" name="sony" ... value="sony" />
<input type="checkbox" name="panasonic" ... value="panasonic" />

So if the 'sony' checkbox is selected then all products with that name in the 'brand' column called 'sony' be selected. If both checkboxes are selected then both 'sony' and 'panasonic' brands are selected.

The default values for the brands is 0.

Thanks for help
SELECT *

FROM products, details

WHERE products.prod_id = details.prod_id AND (price BETWEEN 'minprice' AND 'maxprice') AND (brand = 'brand1' OR 'brand1' = '0') AND (colour = 'brand' OR 'brand2' = '0')

ORDER BY name, price ASC

Open in new window

0
Comment
Question by:allanch08
  • 12
  • 8
  • 5
25 Comments
 

Author Comment

by:allanch08
Comment Utility
sorry should read
SELECT *

FROM products, details

WHERE products.prod_id = details.prod_id AND (price BETWEEN 'minprice' AND 'maxprice') AND (brand = 'brand1' OR 'brand1' = '0') AND (brand = 'brand2' OR 'brand2' = '0')

ORDER BY name, price ASC

Open in new window

0
 
LVL 39

Expert Comment

by:Roger Baklund
Comment Utility
Can you please explain a bit more.

What programming language are you using to fetch the values from the checkboxes?

Your sql statement is kind of meaningless, I suspect you have put strings like 'minprice' and 'maxprice' where you actually have a variable?

'brand1' = '0' is a string comparison that will allways return false. Did you mean brand1=0 ?

A full description of the tables could be usefull, execute...

SHOW CREATE TABLE products;
SHOW CREATE TABLE details;

...and show us the result.
0
 

Author Comment

by:allanch08
Comment Utility
Hello

PHP is used to fetch the values for the variables, so

$minprice = $_POST['minprice'];
$maxprice = $_POST['maxprice'];
$brand1=$_POST['sony'];
$brand2=$_POST['panasonic'];

The database has three columns of products referred to -

prod_id - smallint (5)
brand - varchar(20)
price - decimal(5,2)

The values are passed on correctly to the results page. It's just the syntax of the query that I'm trying to figure out.
0
 

Author Comment

by:allanch08
Comment Utility
so basically the checkbox has two values ie. 'sony' or '0'. If checbox is checked then value=sony so all products with that brand is returned. If the checkbox is unchecked then value=0 so no products called sony are returned.
0
 
LVL 39

Expert Comment

by:Roger Baklund
Comment Utility
Ok, I see.

The code below use the IN operator to check for membership in a set. It will only make $brand_string when either of sony or panasonic is selected. It can easily be expanded with more brands.

This will return only rows where brand='sony'.
  brand in ('sony')

This will return rows where brand is 'sony' OR 'panasonic'
  brand in ('sony','panasonic')

When no brand is selected, no check will be done on the brand column, and all brands will be returned.
$minprice = (float)$_POST['minprice'];

$maxprice = (float)$_POST['maxprice'];

$brand1=$_POST['sony'];

$brand2=$_POST['panasonic'];
 

$brands = array();

if($brand1=='sony') $brands[] = 'sony';

if($brand2=='panasonic') $brands[] = 'panasonic';

$brand_string = (count($brands)>0) ? 

  'AND brand IN ('.implode(',',$brands).')' : '';
 

$sql = "SELECT *

  FROM products, details

  WHERE products.prod_id = details.prod_id AND 

    (price BETWEEN $minprice AND $maxprice) $brand_string

  ORDER BY name, price ASC";

Open in new window

0
 

Author Comment

by:allanch08
Comment Utility
thanks, will try and get back to you on this!
0
 
LVL 39

Expert Comment

by:Roger Baklund
Comment Utility
I spotted a bug with the quoting. Replace these lines:
$brand_string = (count($brands)>0) ? 

  "AND brand IN ('".implode("','",$brands)."')" : '';

Open in new window

0
 
LVL 13

Expert Comment

by:AielloJ
Comment Utility
allanch08:

There is a much easier way to do what you're doing, but first a couple of things about PHP and how it handles checkboxes.  First, only the value of checked boxes are returned in the $_POST array.  Second, to get the values of all checkbox arrays to be returned in the $_POST array, they have to be declared as arrays in your form.  By making some simple changes you can eliminate a lot of your code, and be able to add other items to the form in the future and not having to change the code.

1) Change the checkbox names to be the category (brand):

  <input type="checkbox" name="brand[]" ... value="sony" />
  <input type="checkbox" name="brand[]" ... value="panasonic" />

2) You can now eliminate the hard coded brand searches by stepping through the 'brand' array as in the code sample below.  This means you can add brands without adding or changing the code.

ELIMINATE THIS:

$brand1=$_POST['sony'];
$brand2=$_POST['panasonic'];
 
$brands = array();
if($brand1=='sony') $brands[] = 'sony';
if($brand2=='panasonic') $brands[] = 'panasonic';
$brand_string = (count($brands)>0) ?
  'AND brand IN ('.implode(',',$brands).')' : ''

AND ADAPT THIS:

if (isset($_POST['brand']))
{
  $intListItems = count($_POST['brand']);
}
else
{
  $intListItems = 0;
}

for ($intIndex = 0; $intIndex < $intListItems; $intIndex++)
{
  Do some thing with $_POST['brand'][$intIndex] like:

  WhereClauseString = AppendToWhereClause($_POST['brand'][$intIndex]);
}
0
 

Author Comment

by:allanch08
Comment Utility
thanks aielloj. so the sql statement would stay the same just the php would be different?
0
 
LVL 13

Accepted Solution

by:
AielloJ earned 300 total points
Comment Utility
allanch08:

Pretty much so.  The PHP would actually be a loop that could simply append the necessary values into your IN clause.  Since it's a loop, there wouldn't be any changes to the PHP either.  Using my prior code snippet as a concept it could look something like this:

if (isset($_POST['brand']))
{
  $intListItems = count($_POST['brand']);      // Get count of checked boxes.
}
else
{
  $intListItems = 0;     // No boxes checked.
}

$strWhereClauseIN = '';

for ($intIndex = 0; $intIndex < $intListItems; $intIndex++)
{
  if ($strWhereClauseIN != '')
  {
    $strWhereClauseIN = $strWhereClauseIN . ',';   // Add comma if needed.
  }

  $strWhereClauseIN = $strWhereClauseIN . "'" . $_POST['brand'][$intIndex] . '"';    // Append box value.
}

  'AND brand IN (' . $strWhereClauseIN . ')';   // Assemble string.

You're also using the text values of the brands to do your search.  Since there are probably just a small number of brands it wouldn't affect performance too much, but it's usually more efficient to use the database ID of the brands instead of the names.

Hope this helps.  Let me know how it goes or if I can be of any more help.

JRA
0
 

Author Comment

by:allanch08
Comment Utility
thanks again. going to do some intense coding and research and get back to you
0
 
LVL 39

Assisted Solution

by:Roger Baklund
Roger Baklund earned 200 total points
Comment Utility
This generic way to handle the checkboxes introduces a security risk: you need to validate the input. In my suggestion, based on your current code, we explicitly checked if the brand input was 'sony' or 'panasonic'. I also included a casting to float of the minprize/maxprice parameters, eliminating evil user input:

$minprice = (float)$_POST['minprice'];

This generic handling of brands as suggested by AielloJ involves a risk called SQL injection. The evil user can craft a special request to the server, resulting in information leak from your database. This is however easily avoided, as long as you validate the input from the user.

The risk is in this line:

$strWhereClauseIN = $strWhereClauseIN . "'" . $_POST['brand'][$intIndex] . '"';

You could do a database lookup here, and check if the brand exists. If it does, it is safe. If you can not do this, the variable $_POST['brand'][$intIndex] must be 'sanitized'. You can do that with a simple regular expression, but it could also be done using a custom function you define yourself.

$brand = preg_replace('/[^a-z0-9 .,_-]/i','',$_POST['brand'][$intIndex]);
$strWhereClauseIN = $strWhereClauseIN."'".$brand.'"';

In this example only characters a-z and digits, spaces, dots, commas, underscores and dashes are allowed in a brand name. You may need to adjust that.

This eliminates the problem with the evil user, he can no longer insert the special characters he need to do a SQL injection attack.
0
How to run any project with ease

Manage projects of all sizes how you want. Great for personal to-do lists, project milestones, team priorities and launch plans.
- Combine task lists, docs, spreadsheets, and chat in one
- View and edit from mobile/offline
- Cut down on emails

 
LVL 13

Expert Comment

by:AielloJ
Comment Utility
My prior post was a concept only, not finished code.  Cxr's post about sanitizing the input against SQL injection attacks should ALWYS be used whenever your dealing with data user input.  However, checking input values by hardcoding them is not the way to go.  I'm assuming that was just a concept also.  Checking the values against the database is the way to go.  Keeps the code cleaner, more efficient, and expansion easier.

I've added a placeholder for a function to check the values in the $_POST array.

if (isset($_POST['brand']))
{
  $intListItems = count($_POST['brand']);      // Get count of checked boxes.
}
else
{
  $intListItems = 0;     // No boxes checked.
}

$strWhereClauseIN = '';

for ($intIndex = 0; $intIndex < $intListItems; $intIndex++)
{
  if ($strWhereClauseIN != '')
  {
    $strWhereClauseIN = $strWhereClauseIN . ',';   // Add comma if needed.
  }

  if (BrandExists($_POST['brand'][$intIndex]))
  {
    $strWhereClauseIN = $strWhereClauseIN . "'" . $_POST['brand'][$intIndex] . '"';    // Append box value.
  }
  else
  {
    // Unknown value entered.  THIS SHOULD NEVER HAPPEN.  Suspect cxr's 'evil user' attacking, or
    // a bug in the form or PHP code.  Since there is the possibility of this being an 'evil user' (love the
    // term!!) SQL injection attack, you should probably assume everything about this post is malicious
    // and not process anything from this post.
  }
}

I'm assuming that all the brands you have checkboxes for were populated on your form based on some table in your database.  This limits the user to valid brands, and gives you a list to verify the input against.

JRA
0
 

Author Comment

by:allanch08
Comment Utility
hi guys, the brands would have been populated in the table. The brand chechboxes reflect what is available to search. I'm aware of sql injection so for $minprice which is a text field and should consist of numbers only
$minprice = preg_match('/^\d', $_POST['$minprice']);

Open in new window

0
 
LVL 39

Assisted Solution

by:Roger Baklund
Roger Baklund earned 200 total points
Comment Utility
That does not look right.

if minprice is a decimal number, you can use (float) to cast the string in the $_POST array to a float, in the process any string not evaluating as a decimal number will be ignored. If minprice is an integer, use (int) instead of (float).

Read about PHP type casting here:

http://php.net/manual/en/language.types.type-juggling.php#language.types.typecasting
$minprice = (float) $_POST['$minprice'];

Open in new window

0
 

Author Comment

by:allanch08
Comment Utility
ok thanks!
0
 
LVL 39

Expert Comment

by:Roger Baklund
Comment Utility
There should not be a $ in the key:
$minprice = (float) $_POST['minprice'];

Open in new window

0
 

Author Comment

by:allanch08
Comment Utility
thanks that was a typo on my behalf.
0
 

Author Comment

by:allanch08
Comment Utility
hi aielloj tried the code you suggested but unable to get any matching results, do you think it could be a prob with mysql statement, any help appreciated
$search = "SELECT *

FROM products, details

WHERE products.prod_id = details.prod_id AND (price BETWEEN 'minprice' AND 'maxprice') AND brand IN '$strWhereClause'

INORDER BY name, price ASC";

Open in new window

0
 
LVL 13

Expert Comment

by:AielloJ
Comment Utility
allanch08:

There's a SQL syntax error that looks like an editing error in the statement you posted.  The ORDER BY clause is types as INORDERBY.  Try removing the 'IN' from it and see if that works.
0
 
LVL 39

Expert Comment

by:Roger Baklund
Comment Utility
This phrase:

price BETWEEN 'minprice' AND 'maxprice'

...will never match any rows. it should be

price BETWEEN $minprice AND $maxprice
0
 
LVL 39

Assisted Solution

by:Roger Baklund
Roger Baklund earned 200 total points
Comment Utility
This phrase:

brand IN '$strWhereClause'

...should probably be:

brand IN ($strWhereClause)
0
 

Author Comment

by:allanch08
Comment Utility
Hi guys made the suggested corrections and double checked syntax but still no joy. I'll keep debugging
$search = "SELECT *

FROM products, details

WHERE products.prod_id = details.prod_id AND (price BETWEEN '$minprice' AND '$maxprice') AND brand IN ($strWhereClause)

ORDER BY name, price ASC";

Open in new window

0
 
LVL 13

Assisted Solution

by:AielloJ
AielloJ earned 300 total points
Comment Utility
allanch08:

You missed a critical suggestion by cxr on 10.20.2008 at 09:09AM EST, ID: 22757948 that reads as follows:

==================================================================================
cxr:

This phrase:

price BETWEEN 'minprice' AND 'maxprice'

...will never match any rows. it should be

price BETWEEN $minprice AND $maxprice
==================================================================================

You would only use quotes around minprice and maxprice if they were strings or constants of "minprice" and "maxprice."  Since they are PHP variables, they must start with a $.  Since your query is a dynamically generated string, you want to see what the SQL query string generated by your script is for debugging purposes.  I would place a temporary "echo $search" string in your page to display the SQL query string.  I also copy and paste this string into the mysql command console for debugging purposes.  It gives much more specific information about errors than the PHP interpreter.
0
 

Author Comment

by:allanch08
Comment Utility
Hi guys! Got it to work!

Checked everything again and used

brand IN ($strWhereClause)

as the sql statement and it's all cool. Really grateful for the help cxr/AielloJ, much appreciated!
0

Featured Post

Highfive Gives IT Their Time Back

Highfive is so simple that setting up every meeting room takes just minutes and every employee will be able to start or join a call from any room with ease. Never be called into a meeting just to get it started again. This is how video conferencing should work!

Join & Write a Comment

A lot of articles have been written on splitting mysqldump and grabbing the required tables. A long while back, when Shlomi (http://code.openark.org/blog/mysql/on-restoring-a-single-table-from-mysqldump) had suggested a “sed” way, I actually shell …
Both Easy and Powerful How easy is PHP? http://lmgtfy.com?q=how+easy+is+php (http://lmgtfy.com?q=how+easy+is+php)  Very easy.  It has been described as "a programming language even my grandmother can use." How powerful is PHP?  http://en.wikiped…
It is a freely distributed piece of software for such tasks as photo retouching, image composition and image authoring. It works on many operating systems, in many languages.
Illustrator's Shape Builder tool will let you combine shapes visually and interactively. This video shows the Mac version, but the tool works the same way in Windows. To follow along with this video, you can draw your own shapes or download the file…

771 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

15 Experts available now in Live!

Get 1:1 Help Now