0

I have a search form where users can choose a few checkboxes, set a date range, use a wildcard search etc. and via button click it gets sent via ajax to a PHP file and executed in a MySQL query.

In the php file I set up a cascade of if-statements to ensure the query works regardless of the user having chosen certain values, f.e. like this:

// variable from a risk level dropdown menu
if ($_POST['risklevelcount']=="") {
    $risklevel = "MASTER.RISK_LEVEL != ''";
}

else {
    $risklevel = "MASTER.RISK_LEVEL = "' . $risklevelcount . "'";
}

the variables then are implemented in the where clause of the mysql query, which looks like this:

$result = mysql_query("
SELECT *
FROM        MASTER
WHERE 
        (( ". $buttonvalue ." ) AND ( ". $date . " ) AND ( ". $risklevel ." ))
");

To ensure the query gets executed regardless of the user having checked a checkbox, I analoguously implemented them in a if-clause like this:

if(!empty($checkbox1)) 
    $checkbox1 = "MASTER_CHECK like '%dog%'"; 
else $checkbox1 = "MASTER.CHECK = '' OR MASTER.CHECK != ''";

if(!empty($checkbox2)) 
    $checkbox2 = "MASTER_CHECK like '%cat%'"; 
else $checkbox2 = "MASTER.CHECK = '' OR MASTER.CHECK != ''";

if(!empty($checkbox3)) 
    $checkbox3 = "MASTER_CHECK like '%bird%'"; 
else $checkbox3 = "MASTER.CHECK = '' OR MASTER.CHECK != ''";

If I add these to the above MySQL query where clause I obviously won't get the results I want:

$result = mysql_query("SELECT *
FROM        MASTER
WHERE 
        (
            ( ". $buttonvalue ." ) AND ( ". $date . " ) AND (". risklevel .") AND
                (
                    (". $checkbox1.") OR (". $checkbox2.") OR (". $checkbox3.")
                )
        )
");

I won't get for example results for 'dog' AND 'cat' but not 'bird' (= checkbox 1 and 2 checked but not 3).

I know this is a really stupid approach from the start, regardless of the checkboxes. I sadly don't know how to do any better yet. If someone could point me in the right direction, I would be very glad. I think there is a much more smart way to do it, f.e. with arrays? I just really lack a real basis knowledge!

sardine
  • 157
  • 1
  • 17
  • 2
    Please [stop using `mysql_*` functions](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php). [These extensions](http://php.net/manual/en/migration70.removed-exts-sapis.php) have been removed in PHP 7. Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) statements for [PDO](http://php.net/manual/en/pdo.prepared-statements.php) and [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) and consider using PDO, [it's really pretty easy](http://jayblanchard.net/demystifying_php_pdo.html). – Jay Blanchard Feb 22 '16 at 14:04
  • How large is the data set? – Strawberry Feb 22 '16 at 14:07
  • @Strawberry it's large, a few 10 thousand entries. Why? – sardine Feb 22 '16 at 14:16
  • I was going to suggest bringing the whole dataset into json and handling the filtering there purely in jquery, but this maybe too large for that to work well. – Strawberry Feb 22 '16 at 14:17
  • @JayBlanchard yes I agree.. However I just don't have time atm, I kind of ended up in this coding position at my job although I didn't know any code before and I just have to present this solution rather quick. I will learn PDO statements after this. – sardine Feb 22 '16 at 14:18
  • 4
    I hate when people say *"I'm not that far along..."* or *"This site will not be public..."* or *"It's only for school, so security doesn't matter..."*. If teachers and professors are not talking about security from day one, they're doing it wrong. Challenge them. They're teaching sloppy and dangerous coding practices which students will have to unlearn later. I also hate it when folks say, *"I'll add security later..."*. If you don't have time to do it right the first time, when will you find the time to add it later? – Jay Blanchard Feb 22 '16 at 14:19
  • @Strawberry oh I see! Unfortunately it's far too large for that, I have to restrict the recordset in SQL – sardine Feb 22 '16 at 14:19
  • @JayBlanchard I don't have a teacher or a professor, I'm all on my own. Security is fine because I only operate in a private firm intranet. I agree with you in principle but atm I don't have the choice to completey re-write my whole code. – sardine Feb 22 '16 at 14:21
  • Is it possible for a row to match wildcard searches for multiple options? For example, both 'dog' _and_ 'cat'? – Patrick Q Feb 22 '16 at 14:21
  • @PatrickQ I see what you're getting at, good question. I guess the way I'm setting it up, it wouldn't work either. But *how* would I do it? I mean this has to be a standard thing to do with forms and sql, does it not? – sardine Feb 22 '16 at 14:23
  • I understand @sardine, I just don't want something to jump up and bite you in the butt when someone decides to upgrade PHP on your servers at which point you will have to refactor *all* of your code. – Jay Blanchard Feb 22 '16 at 14:24
  • @JayBlanchard ugh, yes but you're completely right - I *hope* that someone would give me a heads-up before the upgrade, but you're right, I *do* have to spend time on PDO asap.. – sardine Feb 22 '16 at 14:27
  • Part of what I was getting at (and I should have been more clear about) is whether not checking a box means that results matching that option should be _excluded_ or just not actively searched for. So if I check the 'dog' and 'cat' options, but not 'bird', do you want to exclude a row where the value is 'birddog'? Or should that be included in the set because it matches 'dog'? – Patrick Q Feb 22 '16 at 14:29
  • @JayBlanchard don't want to be obnoxious, but you wouldn't know the right direction for a solution for my problem, by any chance? :) – sardine Feb 22 '16 at 14:29
  • @PatrickQ oh ok sorry, I read that too hastily. Thank you for coming back at me :) now I get it. If i check the checkbox for 'dog' and 'cat' I want to have the results for both 'dog' and 'cat' but not 'bird' (and not dogcat). They refer to the same field in my db. I.e. in 'usual' sql it would be like this: WHERE MASTER.CHECK = "dog" OR MASTER.CHECK = "cat" – sardine Feb 22 '16 at 14:34
  • 1
    Small point, 3 If statements are all setting the same variable i.e. `$checkbox1` – RiggsFolly Feb 22 '16 at 14:35
  • @RiggsFolly ARGH sorry that was my fault, of course they're not the same. I had do 'anonymize' the code for this post, this was a slip of the pen :) thank you for noticing – sardine Feb 22 '16 at 14:40
  • 1
    In 'usual' sql, that would be `WHERE MASTER.CHECK IN('dog','cat')`! – Strawberry Feb 22 '16 at 14:44
  • @Strawberry omg that's great!! I didn't know about that!! Wow that helps a lot throughout all of my projects lol, thank you! – sardine Feb 22 '16 at 14:51

1 Answers1

0

I suppose you have a form like this one :

<form id="myFormId" method="POST" action="index.php">
    <div>
        <label for="level">Level :</label>
        <select name="risklevelcount">
            <option value="">-- all level --</option>
            <option value="1">1</option>
            <option value="2">2</option>
            <option value="3">3</option>
        </select>
    </div>
    <div>
        <label>Any Pet ?</label>
        <input type="checkbox" name="pet[]" value="dog">dog
        <input type="checkbox" name="pet[]" value="cat">cat
        <input type="checkbox" name="pet[]" value="bird">bird
    </div>
    <input type="submit" value="submit">
</form>

Here is what I recommend for PHP part :

$strWhere = '1=1';

if(true === isset($_POST['risklevelcount']) && true === is_numeric($_POST['risklevelcount']))
{
    $strWhere = ' AND MATER.RISK_LEVEL = '.mysql_escape_string($_POST['risklevelcount']);
}

if(true === isset($_POST['pet']) && 0 < sizeof($_POST['pet']))
{
    $strWhere .= 'AND (';
    foreach($_POST['pet'] as $pet)
    {
        $strWhere .= ' MASTER_CHECK like "%'.mysql_escape_string($pet).'%" OR ';
    }
    //remove last OR
    $strWhere = substring($strWhere, 0, -3);
    $strWhere .= ')';
}

$result = mysql_query("SELECT * FROM MASTER WHERE ".$strWhere);

I agree with mate, use mysqli or PDO is better, at least escape strings. Moreover, stop saying in your queries MASTER.RISK_LEVEL != '' or worst MASTER.CHECK = '' OR MASTER.CHECK != ''.

In the first case, I supposed risk_level is always defined. So no need to say risk_level != ''. And in the second case, Its useless to say I want string null or not null. Just dont ask and you 'll have all results.

EDIT :

If you check the form at the beginning of my answer, I've add an ID which is myFormId. So with jQuery, if you want to serialize your form data, just do :

javascript:

var formData = $('#myFormId').serialize();
ThinkTank
  • 1,187
  • 9
  • 15
  • I think I can follow this and it looks great, thank you so much! I will implement it and then accept it as answer. I know the 'I want string null or not null'-part is very weird/stupid - it was a workaround because I don't/didn't know about foreach-loops :) – sardine Feb 22 '16 at 14:48
  • it still looks great, however.. Ugh. I'm not able to pass the checkbox array via ajax to my php. How would you send the values of all the checkboxes to the php? I would be eternally grateful for any help.. i tried it with `$('.checkboxes:checked').serialize();` and `$('input[name="checkb[]"]:checked').val();` – sardine Feb 23 '16 at 12:25
  • I've answered, just check the EDIT part. – ThinkTank Feb 23 '16 at 13:08
  • I just can't get this to work.. Thanks so much for your help anyway tho.. If i use console.log on `var formData = $('#myFormId').serialize();` i get `checkb%5B%5D=(MASTER.SERVICE+LIKE+'Network'+OR+MASTER.SERVICE+LIKE+'CORE')` and that doesn't work in my php.. – sardine Feb 23 '16 at 13:23
  • i.e. that doesn't work if I use the php code as you suggested. I also can't see how it should since console log tells me that it contains `+` and `%5B` etc. I'm sorry, I guess your answer would be right theoretically, I will accept it later.. – sardine Feb 23 '16 at 13:27
  • First, you have created your checkbox like that I think `NetWork or Core` and what I suggested was `NetWork`. Second. `+` and `%5B` are present in console log because `serialize` is URL encoding your form values. But don't worry, PHP 'll decode this value when using $_POST and the result 'll be the original value. – ThinkTank Feb 23 '16 at 13:42
  • Thank you so much for coming back at me. I'll work further on the .serialize approach now that I know that php'll decode the values (that irritated me muchly before :) my code now works without error (at last) but my `$strWhere` is always `1=1`. When i echo `$_POST['checkb']` it's always empty, although according to console.log my checkboxes are correctly serialized via `category_id['checkboxvalue'] = $('.checkboxes').serialize();` - would you know what could be the cause of this? – sardine Feb 23 '16 at 14:58
  • i used the if-clause as suggested by you, i.e. `if(true === isset($_POST['checkb']) && 0 < sizeof($_POST['checkb']))` - in html the checkboxes are formatted analoguously to your suggestion - except for the fact i added classes `.checkboxes` and there's part of the sql string in the value. (PS: Please ignore this questions if you're fed up with my n00b issue. i'm very grateful for your help and I guess sometime I will figure this sh** out myself :D ) – sardine Feb 23 '16 at 15:00