Parametrized query unknown total params

Question

i am using safemysql class for parametrized queries https://github.com/colshrapnel/safemysql. Usualy, when preparing a query, it goes like this:

$entries = $db->getAll("SELECT * FROM table WHERE age = ?i AND name = ?s ",$age,$name);

This kind of queries where i know in advance the total number of parametres to be parsed are pretty straight fwd but it seems i am stacked at queries where I do not know how many parametres I will be using - eg. a search form:

What I would like to do, is parametrize the folowing query:

if($_POST['nameparts']){

    $parts = explode(' ',$_POST['nameparts']);

    foreach((array)$parts as $part){

        $q .= " AND ( `name` LIKE '%".$part."%' OR `firstname` LIKE '%".$part."%' ) ";

    }

    if($_POST['age'])
        $q .= " AND `age` = '".$_POST['age']."' ";

    $entries = $dbs->getAll("SELECT * FROM table WHERE 1 = 1 ".$q." ");

Any suggestions?

Why can't you use [PDO](http://wiki.hashphp.org/PDO_Tutorial_for_MySQL_Developers) or `mysqli` instead? — samayo, Apr 23 '14 at 23:15
`$_POST['age']` is still open to sql injection. also wont `1 = 1` just return all the rows.. confused — Lawrence Cherone, Apr 23 '14 at 23:19
I know. I haven't tried to parametrize anything in the example. Right now I am doing some escaping to the data but that is not the subject here. 1=1 should return all rows if $q is not set. Else 1=1 helps in adding a $q that starts with "AND"; — valiD, Apr 23 '14 at 23:22
Solution: build the *query text dynamically with placeholders*, and then call the function with an *array applied* to bind the values. That is, *do not encode [user] values into the query text itself*. — user2864740, Apr 23 '14 at 23:26
As you can see, I know how to create a parametrised query with exact set of parameters. I am presenting the old one just because the total number of parametres is variable. If I would've used placeholders instead of '".$variable."' nobody would have understand what I want. — valiD, Apr 23 '14 at 23:27
Sure I would have: this question isn't novel or unique. The answer you received actually stems from limiting the question to include such query text generation with data-in-the-text and does *not* show placeholders being used. — user2864740, Apr 23 '14 at 23:40

score 1 · Answer 1 · answered Apr 23 '14 at 23:32

1

It doesn't look like safemysql supports variable number of placeholders (otherwise you could build array of parameters in parallel with your query). But you can use methods like escapeString(...). It'll give you the same level of safety, but not so elegant. For example:

$q .= " AND `age` = ".$dbs->escapeInt($_POST['age')]." ";

answered Apr 23 '14 at 23:32

Ivan Nevostruev

28,143
8
66
82

That is what I wanted to know. I was sure I can create an array of params and I did not know how to. – valiD Apr 23 '14 at 23:35
@valiD You can and likely *should* create an array of parameters. While this will work and provide some level of sanity, it does not do so. – user2864740 Apr 23 '14 at 23:39
Yes, this is what I'll recommend to do in "normal" situation (like PDO). But I couldn't find how to pass that array as placeholders for safemysql. Looks like it always use `func_get_args()` to get list of arguments https://github.com/colshrapnel/safemysql/blob/master/safemysql.class.php#L261 – Ivan Nevostruev Apr 23 '14 at 23:46

score 0 · Accepted Answer · edited May 23 '17 at 11:49

0

As with any other SQL query with user-supplied data, to handle this in a truly safe way (or rather to push off the work where it belongs), use placeholders.

Yup, let's start with that goal and not lose sight of it - if the query text contains [user-supplied] data then the code is in violation of one of safemysql's (and safe SQL usage) tenants and the following is not necessarily true anymore!

[safemysql is] safe because every dynamic query part [or "bit of user data"] goes into query via placeholder.

The solution is then to build the query text with placeholders and the data array dynamically - but separately. At no time is the DQL (SQL syntax) and the data mixed. It is this separation (and the guarantee of the lower levels) that guarantees that there is no SQL Injection when this approach is followed.

$data = array();
$q = "SELECT * FROM table WHERE 1 = 1 ";
if($_POST['nameparts']){
    $parts = explode(' ',$_POST['nameparts']);
    foreach((array)$parts as $part){            
        $q .= " AND (`name` LIKE ?s OR `firstname` LIKE ?s )";
        $data[] = '%' . $part . '%'; // add one for each replacement
        $data[] = '%' . $part . '%';
    }
    if($_POST['age']) {
        $q .= " AND `age` = ?i ";
        $data[] = $_POST['age'];
    }
}

And now we have the query text with placeholders and an array of the data to bind. Yippee, we are almost there! Now, create the array that will be passed and invoke the method supplying an array for the parameters.

$params = array($q);
$params = array_merge($params, $data);

$entries = call_user_func_array(array($dbs, 'getAll'), $params);

And, finished!

edited May 23 '17 at 11:49

Community

1
1

answered Apr 23 '14 at 23:51

user2864740

60,010
15
145
220

Damn!!!!! I have just modified my safemysql class and was very proud of myself. But this is just **Wow!** :) call_user_func_array(array($dbs, 'getAll'), $params); – valiD Apr 24 '14 at 00:03
`call_user_func_array` looks like overkill in this case. Anyway this shows that there is design flaw in https://github.com/colshrapnel/safemysql. May be someone will create a patch to it. – Ivan Nevostruev Apr 24 '14 at 00:52
Btw. There is open issue describing this problem: https://github.com/colshrapnel/safemysql/issues/18 – Ivan Nevostruev Apr 24 '14 at 01:00
@IvanNevostruev I maintain that it's not overkill - and in fact, the "correct" method in this case, for the reasons outlined above. While it would be nice if the safemysql methods had a form which took an array of arguments (and perhaps even named arguments), the approach of calling a variadic function with an array is functional, clean, and simple. It would be trivially to create an appropriate `getAllEx` wrapper method, if desired (to maintain backwards compatibility with binding of `?a` it would need to be a separate method). – user2864740 Apr 24 '14 at 03:05
@user2864740 I believe correct method will be use standard `PDO` module or extend `safemysql` with appropriately named method. `call_user_func_array` solution works, but reader of the code will be confused why it was used. Mainly because its semantics is different from "flatten array as parameters". Also code analysis tools will have hard time to tell which method is called here. So I would not recommend this to use in production quality code. – Ivan Nevostruev Apr 24 '14 at 03:17
@IvanNevostruev A code analysis tool that doesn't understand `call_user_func_array(array(obj, 'method').., ..)` should be suspect, and any confused reader/programmer should be taught a new tool. As far as the use of PDO [directly] - *why*? For better or worse, this library was chosen (and it uses mysqli underneath which is "sufficient"). I am not opposed to wrapping or extending with `getAllEx`, etc, but it might internally use this same approach to call the original `getAll` method. Consider with a wrapper, everything being the same: `$dbs->getAllEx($q, $data)`. – user2864740 Apr 24 '14 at 03:25
`if($anum == 1){ if(is_array($args['0'])){ $args = $args['0']; $anum = count($args); } }` And that's the way the class could cover array arguments. – valiD Apr 25 '14 at 20:07
@valiD While this would work in many cases, such an approach requires changing the safemysql API because it understands `?a` (array) bindings already. – user2864740 Apr 25 '14 at 20:42

Parametrized query unknown total params

2 Answers2