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

[imgrooot]

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);

 

[ginerjm]

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.

[imgrooot]

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.

[Barand]

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.

[imgrooot]

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.

[Barand]

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);

 

[imgrooot]

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.

[ginerjm]

I think we gave you what you asked for.  And more.  My array suggestion is just a (imho) better way of connecting the values to the parms.

[imgrooot]

1 hour ago, ginerjm said:

I think we gave you what you asked for.  And more.  My array suggestion is just a (imho) better way of connecting the values to the parms.

Understood. Thanks again.

[kicken]

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 November 19, 2019 by kicken

[imgrooot]

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. 

[imgrooot]

So I tried your method and it seems to give me an error in the query where Parameterize is set.

Notice: Array to string conversion 

 

[imgrooot]

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 November 19, 2019 by imgrooot

[Barand]

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);

 

[imgrooot]

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);

 

[Barand]

Sorry, there is an error

$where = "max_price <= ?";

should be

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

 

[imgrooot]

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)

 

[kicken]

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;

 

[imgrooot]

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.