Jump to content

Do I have to bind int paramters in a pdo query or can it be done without?


imgrooot

Recommended Posts

Normally this is how I do my query. I am binding min, max and featured parameters like this.

$min_price = 10;
$max_price = 50;
$featured  = 1;

$find_records = $db->prepare("SELECT * FROM projects WHERE min_price >= :min_price AND max_price <= :max_price AND featured = :featured");
$find_records->bindParam(':min_price', $min_price);
$find_records->bindParam(':max_price', $max_price);
$find_records->bindParam(':featured', $featured);
$find_records->execute();
$result_records = $find_records->fetchAll(PDO::FETCH_ASSOC);

 

Now I was wondering if it's ok to do the same thing but like this? I am adding the parameters directly in the query. Would it be prone to SQL injection?

$min_price = 10;
$max_price = 50;
$featured  = 1;

$find_records = $db->prepare("SELECT * FROM projects WHERE min_price >= $min_price AND max_price <= $max_price AND featured = $featured");
$find_records->execute();
$result_records = $find_records->fetchAll(PDO::FETCH_ASSOC);

 

Link to comment
Share on other sites

It is since the parms are not 'user input' but simply values that your script produced.

FYI - you can avoid the bind-param calls if you simply create an array of your parms and their values like this:

$parms = array(
	'min_price' => $min_price,
	'max_price' => $max_price,
	'featured' => $featured);
$find_records-> execute($parms);

allows for easier maintenance later on and avoids all of the bind-param calls.

Link to comment
Share on other sites

6 minutes ago, ginerjm said:

It is since the parms are not 'user input' but simply values that your script produced.

FYI - you can avoid the bind-param calls if you simply create an array of your parms and their values like this:


$parms = array(
	'min_price' => $min_price,
	'max_price' => $max_price,
	'featured' => $featured);
$find_records-> execute($parms);

allows for easier maintenance later on and avoids all of the bind-param calls.

I see. But if you see my min and max price, i use equals to and less/more than operators to compare.  But your array does not show that.

Link to comment
Share on other sites

If you do it the second way (no placeholders), there is no point in preparing it; just use $db->query().

CAVEAT: If $vars originated from an external source ($_GET, $_POST, $_COOKIE etc) then you are injection-prone and, as you are not even escaping the values your queries could fail.

EG

$username = "O'Reilly";

$res = $db->query("SELECT password FROM user WHERE username = '$username' ")      // fails with syntax error and open to injection

If in doubt, prepare();

 

1 minute ago, imgrooot said:

But if you see my min and max price, i use equals to and less/more than operators to compare.  But your array does not show that.

Your bindings do not either, the query does. The array is just a more convenient way of binding.

Link to comment
Share on other sites

16 minutes ago, Barand said:

If you do it the second way (no placeholders), there is no point in preparing it; just use $db->query().

CAVEAT: If $vars originated from an external source ($_GET, $_POST, $_COOKIE etc) then you are injection-prone and, as you are not even escaping the values your queries could fail.

EG


$username = "O'Reilly";

$res = $db->query("SELECT password FROM user WHERE username = '$username' ")      // fails with syntax error and open to injection

If in doubt, prepare();

 

Your bindings do not either, the query does. The array is just a more convenient way of binding.

Ah I see. So based on your array, this is how my new query would look like.

$parms = array(
	'min_price' => $min_price,
	'max_price' => $max_price,
	'featured' => $featured);

$find_records = $db->prepare("SELECT * FROM projects WHERE min_price >= :min_price AND max_price <= :max_price AND featured = :featured");
$find_records->execute($parms);
$result_records = $find_records->fetchAll(PDO::FETCH_ASSOC);
  
  

Not exactly the solution I was looking for but I suppose it's a bit better than before.

Link to comment
Share on other sites

3 minutes ago, Barand said:

Or you can use ? placeholders.


$find_records = $db->prepare("SELECT * FROM projects WHERE min_price >= ? AND max_price <= ? AND featured = ? ");
$find_records->execute( [ $min_price, $max_price, $featured ] );
$result_records = $find_records->fetchAll(PDO::FETCH_ASSOC);

 

 

That could work too.


Thanks.

Link to comment
Share on other sites

I have a function that I use so that you can still code like old school concatenation, but use modern binding.  It goes a little something like this:

function Parameterize($value, &$binds){
    static $counter = 0;

    if (!is_array($binds)){
        $binds = [];
    }

    if (is_array($value)){
        if (count($value) == 0){
            return 'null';
        } else {
            $allParams = [];
            foreach ($value as $v){
                $allParams[] = Parameterize($v, $binds);
            }

            return implode(',', $allParams);
        }
    } else {
        if (is_bool($value)){
            $value = (int)$value;
        } else if ($value instanceof \DateTime){
            $value = $value->format('Y-m-d H:i:s');
        }

        $param = ':param' . (++$counter);
        $binds[$param] = $value;

        return $param;
    }
}

You'd then use it like:

$min_price = 10;
$max_price = 50;
$featured  = 1;
$binds = [];

$sql = '
SELECT * 
FROM projects 
WHERE 
    min_price >= '.Parameterize($min_price, $binds).'
    AND max_price <= '.Parameterize($max_price, $binds).'
    AND featured = '.Parameterize($featured, $binds).'
';

$find_records = $db->prepare($sql);
$find_records->execute($binds);
$result_records = $find_records->fetchAll(PDO::FETCH_ASSOC);

I find it keeps things easy to understand and helps reduce the code footprint while still keeping things safe.

Edited by kicken
Link to comment
Share on other sites

13 hours ago, kicken said:

I have a function that I use so that you can still code like old school concatenation, but use modern binding.  It goes a little something like this:


function Parameterize($value, &$binds){
    static $counter = 0;

    if (!is_array($binds)){
        $binds = [];
    }

    if (is_array($value)){
        if (count($value) == 0){
            return 'null';
        } else {
            $allParams = [];
            foreach ($value as $v){
                $allParams[] = Parameterize($v, $binds);
            }

            return implode(',', $allParams);
        }
    } else {
        if (is_bool($value)){
            $value = (int)$value;
        } else if ($value instanceof \DateTime){
            $value = $value->format('Y-m-d H:i:s');
        }

        $param = ':param' . (++$counter);
        $binds[$param] = $value;

        return $param;
    }
}

You'd then use it like:


$min_price = 10;
$max_price = 50;
$featured  = 1;
$binds = [];

$sql = '
SELECT * 
FROM projects 
WHERE 
    min_price >= '.Parameterize($min_price, $binds).'
    AND max_price <= '.Parameterize($max_price, $binds).'
    AND featured = '.Parameterize($featured, $binds).'
';

$find_records = $db->prepare($sql);
$find_records->execute($binds);
$result_records = $find_records->fetchAll(PDO::FETCH_ASSOC);

I find it keeps things easy to understand and helps reduce the code footprint while still keeping things safe.

Oh wow! This is exact solution what I was looking for. 

Link to comment
Share on other sites

Actually let me post my whole code. I am trying something different so you may find a more efficient method to doing it.

So with this code below, I get that "Notice: Array to string conversion" error on the select query.

$min_price = 10;
$max_price = 50;
$featured  = 1;
$binds = [];

$param_1 = "AND projects.min_price >= '.Parameterized($url_min_price, $binds).'";
$param_2 = "AND projects.max_price <= '.Parameterized($url_max_price, $binds).'";
$param_3 = "AND projects.featured = '.Parameterized($url_featured, $binds).'";

if($min_price > 0) {
  $param_min_price = $param_1;
}
if($max_price > 0) {
  $param_max_price = $param_2;
}
if($featured == 0 OR $featured == 1) {
  $param_featured = $param_3;
}

$find_records = $db->prepare("SELECT * FROM projects WHERE $param_min_price $param_max_price $param_featured");
$find_records->execute($binds);
$result_records = $find_records->fetchAll(PDO::FETCH_ASSOC);
if(count($result_records) > 0) {
  foreach($result_records as $row) {
    // OUTPUT RESULTS
  }
}

 

Edited by imgrooot
Link to comment
Share on other sites

The query has inbuilt syntax errors. Your WHERE clause will always begin with "WHERE AND … "

IMO a cleaner way to include conditions only if there is a value is

$min_price = 10;
$max_price = 50;
$featured  = 1;
$binds = [];

$where = [];
$whereclause = '';

if ($min_price > 0) {
    $where[] = "min_price >= ?";
    $binds[] = $min_price;
}
if ($max_price > 0) {
    $where = "max_price <= ?";
    $binds[] = $max_price;
}
if (in_array($featured, [0,1])) {
    $where[] = "featured = ?";
    $binds[] = $featured ;
}

if ($where) $whereclause = 'WHERE ' . join(' AND ', $where);

$find_records = $db->prepare(" SELECT * 
                               FROM projects
                               $whereclause
                               ");
$find_records->execute($binds);
$result_records = $find_records->fetchAll(PDO::FETCH_ASSOC);

 

Link to comment
Share on other sites

29 minutes ago, Barand said:

The query has inbuilt syntax errors. Your WHERE clause will always begin with "WHERE AND … "

IMO a cleaner way to include conditions only if there is a value is


$min_price = 10;
$max_price = 50;
$featured  = 1;
$binds = [];

$where = [];
$whereclause = '';

if ($min_price > 0) {
    $where[] = "min_price >= ?";
    $binds[] = $min_price;
}
if ($max_price > 0) {
    $where = "max_price <= ?";
    $binds[] = $max_price;
}
if (in_array($featured, [0,1])) {
    $where[] = "featured = ?";
    $binds[] = $featured ;
}

if ($where) $whereclause = 'WHERE ' . join(' AND ', $where);

$find_records = $db->prepare(" SELECT * 
                               FROM projects
                               $whereclause
                               ");
$find_records->execute($binds);
$result_records = $find_records->fetchAll(PDO::FETCH_ASSOC);

 

I am getting this error.

Warning: join(): Invalid arguments passed in

On this line

$whereclause = 'WHERE ' .join('AND', $where);

 

Also I assume if ($where) is a mistake on your part on this line?

if ($where) $whereclause = 'WHERE ' . join(' AND ', $where);

 

Link to comment
Share on other sites

54 minutes ago, Barand said:

Sorry, there is an error


$where = "max_price <= ?";

should be


$where[] = "max_price <= ?";

 

Ah yes that was it. I should've seen that.

One more thing. If I want to use this method in my search query, how would you add this code to the above method?

WHERE MATCH(title) AGAINST('$search_query' IN BOOLEAN MODE)

 

Link to comment
Share on other sites

2 hours ago, imgrooot said:

So with this code below, I get that "Notice: Array to string conversion" error on the select query.

Because you're not concatenating the function call like I showed, your just embedding it in the string and it doesn't work like that.  The proper way is to end the string and concatenate, like so:

$param_1 = "AND projects.min_price >= " . Parameterized($url_min_price, $binds);
$param_2 = "AND projects.max_price <= " . Parameterized($url_max_price, $binds);
$param_3 = "AND projects.featured = " . Parameterized($url_featured, $binds);

As Barand mentioned though, there are other issues with that version of the code.

6 minutes ago, imgrooot said:

One more thing. If I want to use this method in my search query, how would you add this code to the above method?

$where[] = 'MATCH(title) AGAINST(? IN BOOLEAN MODE)';
$binds[] = $search_query;

 

Link to comment
Share on other sites

9 minutes ago, kicken said:

Because you're not concatenating the function call like I showed, your just embedding it in the string and it doesn't work like that.  The proper way is to end the string and concatenate, like so:


$param_1 = "AND projects.min_price >= " . Parameterized($url_min_price, $binds);
$param_2 = "AND projects.max_price <= " . Parameterized($url_max_price, $binds);
$param_3 = "AND projects.featured = " . Parameterized($url_featured, $binds);

As Barand mentioned though, there are other issues with that version of the code.


$where[] = 'MATCH(title) AGAINST(? IN BOOLEAN MODE)';
$binds[] = $search_query;

 

Perfect. That works. 

Thanks.

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.