0

I am trying to filter some inputs of the user with select boxes. I am figuring if there is a better way doing this.

    if(isset($_POST['action']))
    {
        
        $sql = "SELECT * FROM occasions WHERE naam IS NOT NULL";
        
        if(isset($_POST['merk'])){
            
            $merk = $_POST['merk'];
            
            $merkQuery = implode(',', array_fill(0, count($merk), '?'));
            
            $sql .= " AND merk IN(".$merkQuery.")";
        }
        
        if(isset($_POST['brandstof'])){
            
            $brandstof = $_POST['brandstof'];
            
            $brandstofQuery = implode(',', array_fill(0, count($brandstof), '?'));
            
            $sql .= " AND brandstof IN(".$brandstofQuery.")";
        }
        
        
        //We prepare our SELECT statement.
        $statement = $pdo->prepare($sql);
        
        
        
        if(isset($_POST['merk'])){
            //Execute statement.
            $statement->execute(array_merge(array_values($merk)));
        }

        if(isset($_POST['brandstof'])){
            //Execute statement.
            $statement->execute(array_merge(array_values($brandstof)));
        }
        
        if(isset($_POST['merk']) && isset($_POST['brandstof']))
        {
            $statement->execute(array_merge(array_values($merk), array_values($brandstof)));
        }
        else
        {
            $statement->execute();
        }
   }

Cause if there are many select boxes that need filtering, the code would become long. I was wondering if there is a better way of filtering multiple select boxes.

Here is an example: link

  • Yup there is a better way – RiggsFolly Jan 08 '21 at 17:37
  • Could we see the prepared query that you are trying to execute? – RiggsFolly Jan 08 '21 at 17:38
  • If your selects were named `name="data[brand]"` etc. then you just pass `$_POST['data']`. But the prepare is different for your scenarios. so maybe use the keys under `data` for that. – AbraCadaver Jan 08 '21 at 17:39
  • @RiggsFolly I've added the prepared query now – Nabil_coder Jan 08 '21 at 17:59
  • Now it makes more sense – RiggsFolly Jan 08 '21 at 18:02
  • @RiggsFolly Do you have a suggestion – Nabil_coder Jan 08 '21 at 18:31
  • It looks ok to me, what exactly is th problem – RiggsFolly Jan 08 '21 at 19:20
  • I don’t think he saw your response (no @ ). I see your concern, that the `in()` clause could get large— but how large are you talking? I think you can have at least 2,000. – Tim Morton Jan 08 '21 at 22:38
  • @TimMorton Hi Tim, the merging of the arrays becomes very long when there are more select boxes. Like you should check for every $_POST if it is set, then you have to check if specific inputs are set together, and you should check which aren't set. Is there a better way of merging arrays without checking every set $_POST? – Nabil_coder Jan 09 '21 at 13:30
  • @RiggsFolly If there are more select boxes that need filtering, the code can become very large. Is there a better way to prevent this? – Nabil_coder Jan 09 '21 at 13:38
  • what does your UI look like, how many checkboxes? perhaps give a sample of the html that is submitting it? I can’t imagine having thousands of checkboxes for user to click. – Tim Morton Jan 09 '21 at 19:41
  • re checking for every post: use a nested associative array: ``, `` etc. Then you can simply foreach through the arrays. – Tim Morton Jan 10 '21 at 00:11
  • @TimMorton I have added a link above. My question is how can I use array_merge best in my situation – Nabil_coder Jan 10 '21 at 14:01
  • @TimMorton I have provided the code you gave as an answer. It did not work, here is the error: Array ( [0] => Array ( [0] => renault ) ) SELECT * FROM occasions WHERE naam IS NOT NULL AND merk IN(?) Notice: Array to string conversion in /mnt/web411/b1/61/59929461/htdocs/Bashir-Buitenbos/test-filter.php on line 47 Array ( [0] => Array ( [0] => renault ) ) geen resultaten – Nabil_coder Jan 12 '21 at 22:48
  • append your post to show the lines in question. please include the context, too. I just reused the code you had provided to build the `in`clause; I don’t see anything in my answer that would cause an array to be passed into `$query` – Tim Morton Jan 12 '21 at 23:01
  • @TimMorton The code is fine, but somehow the $params variable is a multidimensional array that cant be imploded to put in $statement->execute() Testlink: http://buitenbos.webexact.nl/test-filter – Nabil_coder Jan 12 '21 at 23:27
  • what does `print_r($params)` give you? – Tim Morton Jan 12 '21 at 23:49
  • @TimMorton it gives me: Array ( [0] => Array ( [0] => renault ) ) and also Array ( [0] => Array ( [0] => bmw ) ) when clicked – Nabil_coder Jan 12 '21 at 23:53
  • Oh, I see what the problem is. I was thinking I was pushing a string into $params; but array_keys returns an array. Might have to use a foreach to get the keys one at a time. Sorry I slipped up there... and I’m having a mental block getting around that :( – Tim Morton Jan 13 '21 at 00:14
  • @TimMorton No problem Tim, thanks for helping me out. It means a lot to me. I am trying out prepared statements since I heard it is a safer way, but yesterday I have discovered that injections can also be stopped by validating user input. I chose that route now so PDO is not an option for me anymore. It brought me headaches for five days :( – Nabil_coder Jan 13 '21 at 13:47
  • Strongly advise you to reconsider. Using prepared statements is the standard. period. It’s far easier and far more secure. You certainly should validate, but it’s not a substitute. Be sure you start with some easier queries that don’t have to be generated, that way you’ll have pdo under your belt when you try one like this. solving more than one problem at a time gets exponentially harder. – Tim Morton Jan 13 '21 at 14:05
  • @TimMorton Why is validating user input not a substitute? – Nabil_coder Jan 13 '21 at 16:24
  • validation and sanitization are two different things. validation means it makes sense. sanitization means it is safe. prepared statements are the latter. how will you deal with `'`, `<`, etc? You have to either whitelist all user input, or know all hacker tricks to sanitize yourself. prepared statements (properly used) takes care of that. – Tim Morton Jan 13 '21 at 17:33
  • Doesn't pregmatch help? if(!preg_match('/^[\p{L} ]+$/u', $userInput)) . That takes care of the ' and ; and < doesn't it? – Nabil_coder Jan 14 '21 at 14:30

1 Answers1

1

I would suggest renaming the post variables; grouping them into a single two dimensional array.

<input type="checkbox" name="data[merk][bmw]" />
<input type="checkbox" name="data[merk][skoda] />

and so forth.

What this does, is it allows you to use a foreach to iterate through whatever values are checked.

$data = $_POST['data'] ?? []; // null coalesce defaults to a blank array if post var is null
foreach($data as $category=>$val) {
    settype($val, 'array');
    $query = implode(',', array_fill(0, count($val), '?'));
    foreach($val as $k=>$v) {
        $params[] = $k;
    }

    // DON'T DO THIS!
    $sql .= " AND $category IN(".$query.")";
}

The reason you shouldn’t do it as shown is because you should never build a query with user-supplied data.

What you can do, however, is map user-supplied data with hard-coded data.

$map = [
    // form value => db field
    'merk'      => 'MERK',
    'brandstof' => 'BRANDSTOF',
    // ... etc
];

and then when building your query,

$sql .= " AND $map[$category] IN($query)";

In the meantime, you have built your parameters in $params.

—-

Bottom line, what we have done is refactor the code since we were noticing things getting repeated. For example, you were having to repeat code for each occasion(?). One solution would be to continue to check each post value and call a function to calculate the ?s. But even then, it would be repetitive to type out all those isset()s.


In retrospect, it probably would have been better to do inputs like this:

<input type="checkbox" name="data[merk][]" value="bmw" />
<input type="checkbox" name="data[merk][]" value="skoda" />

This would no doubt be more intuitive, although you would still have to build the params array.

foreach($val as $v) {
    $params[] = $v;
}
Tim Morton
  • 2,614
  • 1
  • 15
  • 23
  • So the array_merge function is not needed with this alternative? Will the filter system still work when a user inputs values together? – Nabil_coder Jan 11 '21 at 15:26
  • Have I understood this correctly when I say every time a user inputs a value, it will loop and give a parameter per value? Then after the loop I should say $statement->execute($params) ? Then it doesn't matter how many inputs there are? – Nabil_coder Jan 11 '21 at 15:31
  • This is off the top of my head and typed in on my phone; I didn't have a way to verify it. But the idea is that you are building a parameter list in the same order as your query. So unless your query has other parameters before or after these, then no merge would be required. – Tim Morton Jan 11 '21 at 15:31
  • The foreach will just iterate through whatever it's given. – Tim Morton Jan 11 '21 at 15:32
  • Should I change the way I pass values to the PHP with AJAX? var action = 'data'; var merk = get_filter_text('merk'); var brandstof = get_filter_text('brandstof'); var carrosserie = get_filter_text('carrosserie'); $.ajax({ url: 'php_file', method: 'POST', data: { action: action, merk: merk, brandstof: brandstof, carrosserie: carrosserie }, ... – Nabil_coder Jan 11 '21 at 15:37
  • You would want to send the form as `data`. https://stackoverflow.com/questions/2019608/pass-entire-form-as-data-in-jquery-ajax-function. `action` is whatever you want it to be, I didn't use it here. I would probably make it a hidden input to simply tell your script to process the filter. `data` would be an array that replaces merk, brandstof, etc. because that information is now in the keys of the array (take note of how the checkboxes are named). – Tim Morton Jan 11 '21 at 15:59
  • Keep in mind, though, that AJAX is beyond the scope of this question. That's adding confounding factors; you may want to check with just plain old html submission to verify that this idea works first. – Tim Morton Jan 11 '21 at 16:03
  • I will test it then I will let you know thanks – Nabil_coder Jan 11 '21 at 16:57
  • Should be trivial to build the checkboxes dynamically. HTH – Tim Morton Jan 11 '21 at 17:00
  • I tried the code you provided, but it did not work: Array ( [0] => Array ( [0] => renault ) ) SELECT * FROM occasions WHERE naam IS NOT NULL AND merk IN(?) Notice: Array to string conversion in /mnt/web411/b1/61/59929461/htdocs/Bashir-Buitenbos/test-filter.php on line 47 Array ( [0] => Array ( [0] => renault ) ) geen resultaten – Nabil_coder Jan 12 '21 at 22:45
  • changed the code. hopefully a quick and simple fix – Tim Morton Jan 13 '21 at 01:22