I ran into a subtle bug on a recent project that looked correct at first glance but was quietly returning too many results.
The code was building a query to apply permissions for each entity a user had access to.
Each entity was identified by a combination of an id and a type, and we were looping through a list of them:
$query->where(function ($query) use ($entities): void {
foreach ($entities as $entity) {
$query->orWhere([
'entity_id' => $entity['id'],
'entity_type' => $entity['type'],
]);
}
});
This looks reasonable.
You might read that orWhere call as "where entity_id is X and entity_type is Y."
But that is not what Laravel does with an associative array.
When you pass an array to orWhere, Laravel loops through each key-value pair and adds them as separate OR conditions.
Compare what you might expect the SQL to be versus what it actually generates:
-- What you might expect
WHERE ((`entity_id` = ? AND `entity_type` = ?) OR (`entity_id` = ? AND `entity_type` = ?))
-- What you actually get
WHERE ((`entity_id` = ? OR `entity_type` = ?) OR (`entity_id` = ? OR `entity_type` = ?))
Every condition is joined with OR.
Instead of requiring both the id and type to match, it would match a row where just the type was correct, or just the id.
The fix is to use a closure, which groups the conditions with AND:
$query->where(function ($query) use ($entities): void {
foreach ($entities as $entity) {
$query->orWhere(function ($q) use ($entity): void {
$q->where('entity_id', $entity['id'])
->where('entity_type', $entity['type']);
});
}
});
This produces the intended SQL from our example above and enforces the correct entity permissions.
Here to help,
Joel
P.S. Bugs like this are easy to miss in your own code. A second set of eyes can catch what you overlook. Learn more about our code review service.