getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.32k stars 168 forks source link

DB query where clause breaks when parameter is 0 #4518

Closed toebu closed 2 years ago

toebu commented 2 years ago

When you have a db query, and you try to set 0 as the parameter ($query->where('field', '=', 0);), the created query is invalid. It looks like this is because in Kirby\Database\Query::filterQuery you use in_array() without strict mode. The last argument will then always be removed and treated as the mode instead of the parameter.

This would be fixed by using strict mode in_array($mode, ['AND', 'OR'], true). Probably, this would also have to be changed in other places, such as in andWhere and orWhere.

I'm using the workaround to for now always explicitly state the mode $query->where('field', '=', 0, "AND');.

A related problem that comes up here: if you're parameter happens to be "AND" or "OR", this will also not work as intended... But that's a bit more tough to fix.

lukasbestle commented 2 years ago

This would be fixed by using strict mode in_array($mode, ['AND', 'OR'], true)

To be honest I don't see how the strict mode will change the behavior. in_array(0, ['AND', 'OR']) without the strict mode and in_array(0, ['AND', 'OR'], true) both return false.

A related problem that comes up here: if you're parameter happens to be "AND" or "OR", this will also not work as intended

AND and OR (unquoted!) are not valid values in SQL. Is there a use case I missed?

toebu commented 2 years ago

To be honest I don't see how the strict mode will change the behavior. in_array(0, ['AND', 'OR']) without the strict mode and in_array(0, ['AND', 'OR'], true) both return false.

I searched around a bit more, and found out this is depends on the PHP version. I'm using PHP 7.4, so for me the two expressions return something else. In PHP 8 this was fixed, that's why you see another behaviour. See for example https://wiki.php.net/rfc/string_to_number_comparison.

Kirby says it supports PHP 7.4, so one should fix it for this PHP version. There, you have to explicitly tell in_array to use strict mode.

A related problem that comes up here: if you're parameter happens to be "AND" or "OR", this will also not work as intended

AND and OR (unquoted!) are not valid values in SQL. Is there a use case I missed?

The 'AND' will be passed with sql parameter binding, so quotation happens after the code that is problematic.

The use case is this:

Db::table('some_table')->where('some_db_field, '=', 'AND')->all();

Here, the last argument will be interpreted as the mode for the where clause (because https://github.com/getkirby/kirby/blob/main/src/Database/Query.php#L944 evaluates to true).

The desired behaviour would be, that the 'AND' is used to compare for equality on the some_db_field.

It's quite an edge-case, I agree. It only happens when you want to compare exactly to 'AND' or 'OR'. But nevertheless not ideal (and not dependent on the PHP version here).

lukasbestle commented 2 years ago

Oh yes, I can reproduce the issue on PHP 7.4. This is indeed a bug.

Regarding AND and OR: I think it would be ideal to make the mode a separate argument in filterQuery(). The andWhere() and orWhere() methods could then explicitly pass that mode argument while where() doesn't pass it. It would be a breaking change, but a sensible one. I've opened a separate issue here: #4564

distantnative commented 2 years ago