From the discussion started in #1267508: Subselects don't work in DBTNG conditions, except when used as value for IN at comment #83:
\Drupal\tmgmt_locale\Tests\LocaleSourceUiTest is failing.
We have a condition like this:
$select->isNull("lt_$missing_target_language.language");
$missing_target_language in this case is 'gsw-berne'. That used to be converted to gswberne, but no longer.
Maybe that's on purpose and can't be avoided, but I'm not sure?
We already have other places where we explicitly escape..
The code is in \Drupal\tmgmt_locale\LocaleSourcePluginUi
- discuss
- we can fix it for now manually, see how we use db_escape_field() elsewhere in the same class
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | fix_escape_condition_in-2783123-6.patch | 1.79 KB | Bambell |
Comments
Comment #2
fietserwinDon't fix it here, not escaping has security implications (although I am not sure that the field part can ever come directly from user supplied input).
[EDIT: that said, relying on escaping by the DB layer may be considered as not being best practice. If the functions are there and public, it is apparently also considered a calling side responsibility].
However, you may fix the theoretical bug as described in comment #1267508-86: Subselects don't work in DBTNG conditions, except when used as value for IN.
Comment #3
daffie commentedComment #4
Bambell commentedLocaleSourceUiTest is green after manually escaping, although I don't think that's the fix expected... It's also green with #2783165: Follow up to "Subselects don't work in DBTNG conditions, except when used as value for IN". I'll try to dig a bit more on comment 11505747, a suggested.
Comment #5
fietserwinWait with fixing it, the issue causing this has been reverted and will not land in 8.2.x anymore.
Comment #6
Bambell commentedAlright, maybe we can still change db_escape_field to db_escape_table as you suggested, however? I removed the fix of the previous patch and made those changes.
Comment #7
daffie commented@Bambell: You can fix this for now with your patch, but this should be fixed in the Drupal core class Drupal\Core\Database\Query\Condition.
Comment #8
fietserwinThis fixes my suggestion and leaves escaping the field in the isNull() condition to Core as was and will be the case.
Comment #9
berdirCommitted.