How do I conditionally add a clause to my query in PHP?

Question

In my search form I have, value male, value female, and value both. If you select both, I do not wish to have at WHERE the "sex = '$sex'"

Should i do it like this:

if(empty($sex)){ // value empty if you chose "both"
$query = "SELECT firstname, lastname, id, user_name, sex, last_access, bostadsort FROM users WHERE (firstname LIKE '%$firstname%' OR lastname LIKE '%$lastname%')";
}else{
$query = "SELECT firstname, lastname, id, user_name, sex, last_access, bostadsort FROM users WHERE (firstname LIKE '%$firstname%' OR lastname LIKE '%$lastname%') AND sex = '$sex'";
}

Or is there a smart way to write this?

HAHA! Now when I read the title again... That was funny :P I like using "sex" rather than gender. — Johnson, Sep 29 '10 at 19:14

Tomalak · Answer 1 · 2010-09-29T19:29:53.073

6

Do never build an SQL string from user input. That's what prepared statements are for. They are secure, perform faster when re-executed and they're easy to use, so use them:

$sql = "
  SELECT
    firstname, lastname, id, user_name, sex, last_access, bostadsort
  FROM
    users 
  WHERE
    (firstname LIKE '%'|| ? || '%' OR lastname LIKE '%'|| ? || '%')
    AND Sex = CASE ? WHEN 'both' THEN Sex ELSE ? END
";
$stmt = $mysqli->prepare($sql);
$stmt->bind_param('ssss', $firstname, $lastname, $sex, $sex);

$result = $stmt->execute();

edited Sep 29 '10 at 19:29

answered Sep 29 '10 at 19:24

Tomalak

332,285
67
532
628

This is the correct and much more secure way of doing this. An even higher level approach is to use a class to encapsulate this, or use one of the dozens of ORM frameworks available out there. Like: http://stackoverflow.com/questions/108699/good-php-orm-library – Visionary Software Solutions Sep 29 '10 at 19:34
@visionary: I'd say that this code above is the smallest common denominator. Shallow learning curve, much improved handling and security. ORM frameworks or OO encapsulation may be too much at this stage. – Tomalak Sep 29 '10 at 19:40
I guess i am new to PHP, when i dont have any clue of what bind_param, execute and those commands u use there. And im using mysql not mysqli. – Johnson Sep 29 '10 at 19:43
@user: You should use `mysqli_*`. It's not that hard, with a little reading you will have the basics gathered in less then half an hour. The PHP docs (and this site, too!) are full of ready-to-use, straight-forward examples, and it's not *that* different from `mysql_*`. – Tomalak Sep 29 '10 at 19:51
Ok let me try this code, would i need to change all my mysql to mysqli in order to have this code work? Or do PHP accept mysqli and mysql commands combined – Johnson Sep 29 '10 at 19:55
just got Fatal error: Call to a member function prepare() on a non-object, I guess i stick to mysql_ now when I started building everything in mysql(all my other codes) – Johnson Sep 29 '10 at 19:58
@user: I did not say this code runs right-away. You are not connected to a database, so `$mysqli` is not defined yet. Seriously, just open the PHP docs, look at the samples and do a little reading. You have not even *tried* yet, it's a little too early to give up. – Tomalak Sep 29 '10 at 20:01

score 3 · Answer 2 · answered Sep 29 '10 at 19:08

3

How about not repeating yourself:

$query = "SELECT firstname, lastname, id, user_name, sex, last_access, bostadsort FROM users WHERE (firstname LIKE '%$firstname%' OR lastname LIKE '%$lastname%')";

if(!empty($sex)){
    $query = $query . " AND sex = '$sex'";
}

answered Sep 29 '10 at 19:08

Justin Ethier

131,333
52
229
284

1

How about not building SQL from interpolated strings? – Tomalak Sep 29 '10 at 19:14
Could this be SQL injected byron? (mentioned that in your answer) – Johnson Sep 29 '10 at 19:17
this still doesn't solve the fundamental security issues with this code. Not to mention the maintainability issues of rolling your own queries by hand. – Visionary Software Solutions Sep 29 '10 at 19:38

Byron Whitlock · Answer 3 · 2010-09-29T19:21:09.450

2

You could do this:

$sql = "SELECT ..."; 

if (!empty($gender))
{
    $sql .= " AND gender = '$gender'";
}

And make sure to watch for sql injection.

edited Sep 29 '10 at 19:21

answered Sep 29 '10 at 19:08

Byron Whitlock

52,691
28
123
168

You could minimally do something like `$gender = mysql_real_escape_string($_REQUEST['gender'])` – Byron Whitlock Sep 29 '10 at 19:20
This will "work", and using mysql_real_escape_string() will deal with the security vulnerability (if it's targeting MySql, of course. What about Postgres? MSSql?), but architecturally hand-rolled SQL is a brittle and error-prone approach that is quickly losing favor. In the interest of instilling best practices in the community, we're better off moving people away from these dirty hacks. – Visionary Software Solutions Sep 29 '10 at 19:41

How do I conditionally add a clause to my query in PHP?

3 Answers3