0

I am currently trying to write complex MySQL WHERE clauses that are generated from $_GET variables (which themselves come from select dropdowns). First, a bit of code so you know what I am talking about:

    if(!isset($_GET['order'])){
        $order= 'start asc';
    } elseif ($_GET['order'] == "dateasc") {
        $order= 'start asc';
    } elseif ($_GET['order'] == "titleasc") {
        $order= 'title asc';
    } elseif ($_GET['order'] == "titledesc") {
        $order= 'title desc';
    };


    if(!isset($_GET['cat'])){
        $cat= '0';
    } else  {
        $cat = $_GET['cat'];
    };


    if(!isset($_GET['loc'])){
        $loc= '0';
    } else  {
        $loc = $_GET['loc'];
    };


    if (isset($_GET['sd']) || isset($_GET['ed']) || isset($_GET['cat']) || isset($_GET['loc']) || isset($_GET['order']) ) {
        $where = 'WHERE ';

        if (isset($_GET['sd'])) {
            $where .= "start = " . $_GET['sd'];
        };

        if (isset($_GET['ed'])) {
            $where .= "AND end = " . $_GET['ed'];
        };

        if (isset($_GET['cat'])) {
            $where .= "AND category = " . $_GET['cat'];
        };

        if (isset($_GET['loc'])) {
            $where .= "AND location = " . $_GET['loc'];
        };
    };


    $result = mysql_query("SELECT * FROM " . TABLE . $where . " ORDER BY " . $order);

Obviously this isn't working, otherwise I wouldn't be here. :) Basically, I have 4 variables that I want to conditionally use for sorting in my query: start date, and end date, a category, and a location. My problem is that all 4 of these may not always be used.. so given the above example, there might be a case where someone selects a category ($cat) but NOT a start date ($sd)... which means my WHERE clause would start off with 'AND', which is obviously invalid. So how do I build a query based off variables that may or may not be used?

I really feel like I am overthinking this, and I am afraid of writing 9000 lines of isset tests to account for every combination of $_GET variable usage. Surely there a simple way to build a WHERE clause from multiple $_GETs that may or may not be used every time..? I've tried Googling but can only find solutions that suggest using a framework for building complex queries and that just seems overly... clunky... for such a simple problem.

Kane Ford
  • 426
  • 6
  • 12
  • The simplest way to do this is to put each of your conditional statements into an array as you go through, instead of building the SQL directly. Then at the end, you can just `implode()` your array using AND as the glue. – andrewsi Sep 12 '13 at 17:45
  • 2
    You shouldn't insert your GET vars directly into your SQL like that. It's not safe. Look at http://www.php.net/manual/en/book.mysqli.php – Vlad Sep 12 '13 at 17:52
  • Suggestion : I think for a complex conditional statements you should use switch-statement.http://stackoverflow.com/questions/2158759/case-vs-if-else-if-which-is-more-efficient – John Harper Sep 12 '13 at 18:17
  • 1
    Please, before you write **any** more SQL interfacing code, you must read up on [proper SQL escaping](http://bobby-tables.com/php) to avoid severe [SQL injection bugs](http://bobby-tables.com/). Also, `mysql_query` should not be used in new applications. It's a deprecated interface that's being removed from future versions of PHP. A modern replacement like [PDO is not hard to learn](http://net.tutsplus.com/tutorials/php/why-you-should-be-using-phps-pdo-for-database-access/) and will make your database code easier to get right. – tadman Sep 12 '13 at 18:26
  • Thanks for the tips! I do frontend and UI stuff almost exclusively for the most part, so I definitely appreciate all the pointers. – Kane Ford Sep 17 '13 at 01:09

2 Answers2

4

If you're just worried about having a where clause that starts with AND you can add 1=1 to account for no filters.

WHERE 1=1

Then, if you have any filters, it will look like this:

WHERE 1=1 AND col1=? AND col2=?
Vlad
  • 978
  • 6
  • 13
-1

This may not be the cleanest solution, but it should be fairly simple to understand and implement.

if (isset($_GET['sd']) || isset($_GET['ed']) || isset($_GET['cat']) || isset($_GET['loc']) || isset($_GET['order']) ) {
    $where = 'WHERE ';

    if (isset($_GET['sd'])) {
        if(strlen($where) > 6) {
           $where .= " AND ";
        }
        $where .= "start = " . $_GET['sd'];
    }

    if (isset($_GET['ed'])) {
        if(strlen($where) > 6) {
           $where .= " AND ";
        }
        $where .= "end = " . $_GET['ed'];
    }

    if (isset($_GET['cat'])) {
        if(strlen($where) > 6) {
           $where .= " AND ";
        }
        $where .= "category = " . $_GET['cat'];
    }

    if (isset($_GET['loc'])) {
        if(strlen($where) > 6) {
           $where .= " AND ";
        }
        $where .= "location = " . $_GET['loc'];
    }
}
Lumberjack
  • 488
  • 4
  • 13
  • **NO**. This is totally crazy. You can't just slap arbitrary `$_GET` content in your query. This isn't clean by any definition of clean. – tadman Sep 12 '13 at 18:25
  • I added `if(strlen($where) > 6) { $where .= " AND "; }` to OPs code. OP didn't ask for us to help secure his code. He asked for help with mitigating leading "AND" in specific cases. Is it important to protect your code from SQL Injection? Yes. Is it ALWAYS important? No. Sometimes I write applications where the only user is ME. – Lumberjack Sep 12 '13 at 18:30
  • It's always important, even if the user is you. One day you'll be searching for category "O'Malley" and then you'll fall victim to your sloppy programming. Seriously, is it too much to ask to slap on some `mysql_real_escape_string` in there and permanently fix the problem? SQL injection bugs are not a joke. Don't brush them off as no big deal. The PHP community needs to get rid of this "It won't happen to me" attitude. – tadman Sep 12 '13 at 18:40