Fixed \XF\PermissionCache::getContentPerms causes extra queries for non-existent entries if cacheAllContentPerms has been called

[Xon]

Affected version

2.2.2

If \XF\PermissionCache::cacheAllContentPerms() has been called, then $globalCacheRun is set which indicates all possible values for $permissionCombinationId/$contentType have been fetched.

However getContentPerms() function assumes does not check if $globalCacheRun is set for that combination, which can result in additional queries which are known rather than just simply returning null

 

[Mike]

I'm not fundamentally against changing this, though what situation comes up that involves checking against IDs that aren't in the permission cache? I could potentially see an edge case with reports, but in normal usage, there should be an entry for each content-container (node, resource category, etc), so every call would normally return a record.

 

[Lukas W.]

I'm not fundamentally against changing this, though what situation comes up that involves checking against IDs that aren't in the permission cache? I could potentially see an edge case with reports, but in normal usage, there should be an entry for each content-container (node, resource category, etc), so every call would normally return a record.

There's a number of edge cases that can profit from this, such as iterating over content ids without actually having verified that the content ID exists. You can do this if you know a maximum ID, but don't want to look up all intermediate IDs and don't explicitly want to care if there's any holes in your ID list, or even to simplify fetching all existing IDs to just fetching the maximum ID and then just iterating over all.

Basic example:

$viewableIds = [];
for($i = 0; $i <= $maxContentId; $i++) {
    $viewableIds[] = \XF::visitor()->hasContentPermission($contentType, $i, 'view');
}

// Proceed to fetch content with these IDs, or store IDs in cache for later fetching

 

[Xon]

I'm not fundamentally against changing this, though what situation comes up that involves checking against IDs that aren't in the permission cache? I could potentially see an edge case with reports, but in normal usage, there should be an entry for each content-container (node, resource category, etc), so every call would normally return a record.

The pattern I'm thinking of;

\XF::visitor()->cacheNodePermissions();
...
\XF::visitor()->hasNodePermission(9999998, 'view');
\XF::visitor()->hasNodePermission(9999999, 'view');

If there are no permissions for 9999998. 9999999, then this will cause additional unexpected queries. But since cacheNodePermissions() has been called, it is reliable to say we know that the non-existence of nodeIds 9999998/9999999 have been proven (at least in this request).

This should be rare. But the most likely case is when rendering search results or reports on posts/threads where the forum no longer exists.

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.7).

Change log:

Do not attempt to query for uncached content permissions after the global cache has already been run

There may be a delay before changes are rolled out to the XenForo Community.