dg / dibi

Dibi - smart database abstraction layer
https://dibiphp.com
Other
487 stars 136 forks source link

%and should parse composite argument values #264

Closed jprk closed 7 years ago

jprk commented 7 years ago

As a newcommer to Dibi (version 3.0.x, installed yesterday using composer, but I am unable to find exact version) I was slightly puzzled by different behaviour of composite arguments in different SQL commands. I am not sure if it is by design or if is really a bug, nevertheless, it would be nice to have it mentioned in the documentation ...

If I set query arguments as an associative array (see https://dibiphp.com/cs/quick-start#toc-slozitejsi-vyrazy-v-polich)

$args = [
    'a' => "qwerty",
    'b' => array('MD5(%s)', 'bflmpsvz')
];

then calling

dibi::test('UPDATE sometable SET ', $args);

will format the query with b concatenated and interpreted as SQL, i.e.

UPDATE sometable SET `a`='qwerty', `b`=MD5('bflmpsvz')

but calling

dibi::test('SELECT * FROM sometable WHERE %and', $args);

will not process the composite field b and generate

SELECT * FROM sometable WHERE (`a` = 'qwerty') AND (`b` = 'MD5(%s)', 'bflmpsvz')

instead. To circumvent this, one needs to set %sql modifier on b, i.e.

$args = [
    'a' => "qwerty",
    'b%sql' => array('MD5(%s)', 'bflmpsvz')
];

Then both examples work as expected.

Jan

dg commented 7 years ago

It can be fixed, it will not break tests, but I do not know if it is a BC break…

JanTvrdik commented 7 years ago

Are sure @dg? This will result in a lot of SQL injections is a lot of applications. Consider the following code

$dibi->query('SELECT * FROM [products] WHERE %and', [
    'id' => $_GET['id'],
]);
dg commented 7 years ago

That's a problem.

What about this solution:

$args = [
    'a' => "qwerty",
    'b' => new Dibi\Expression('MD5(%s)', 'bflmpsvz')
];

However, the current Literal works a little differently because it is no longer processed…

JanTvrdik commented 7 years ago

@dg That was wrong with the previous solution?

$dibi->query('SELECT * FROM [products] WHERE %and', [
    'foo%ex' => ['MD5(%s)', 'bflmpsvz'],
]);
dg commented 7 years ago

Just because of

$dibi->query('UPDATE sometable SET', [
    'id' => $_GET['id'],
]);

it would be better to introduce an object replacement for array. For example Dibi\Expression?

dg commented 7 years ago

Since v3.1 you can replace arrays with Dibi\Expression and it will work for UPDATE, WHERE, everywhere :)

$args = [
    'a' => 'qwerty',
    'b' => new Dibi\Expression('MD5(%s)', 'bflmpsvz')  // or $dibi::expression('MD5(%s)', 'bflmpsvz') in Dibi 4
];

$dibi->test('UPDATE sometable SET ', $args);
$dibi->test('SELECT * FROM sometable WHERE %and', $args);