node_access should cache by vid and not nid.

The problem exists in workflow systems, where the published node is different than the draft node, and where the node access for each might be different. For example, the published node might be viewable by all, but the draft node might only be viewable by document authors, editors, publishers, etc... This later group of people are not super-users, so they don't get 'bypass node access' permission, but through some hook_node_access(), they do have access to unpublished/draft revisions. In this use case, we can not cache by nid, but must cache by vid.

This problem is in node_access() and is not the same problem in #1064954: _node_revision_access() static usage, which is specific to _node_revision_access().

I think that this problem is already solved in Drupal 8 through the use of entity->uuid().

#3 drupal-node_access_static_cache_vid-2026817-3.patch568 bytesazinck
PASSED: [[SimpleTest]]: [MySQL] 40,753 pass(es). View
core-node-access.patch411 bytesdouggreen
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch core-node-access.patch. Unable to apply patch. See the log in the details link for more information. View
Members fund testing for the Drupal project. Drupal Association Learn more


douggreen’s picture

Title: node_access() static usage » node_access() should static cache by vid and not nid.

Status: Needs review » Needs work

The last submitted patch, core-node-access.patch, failed testing.

azinck’s picture

Issue summary: View changes
Status: Needs work » Needs review
568 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,753 pass(es). View

I agree 100%. The need for this is exemplified even by _node_revision_access() in core which contains the following line which calls node_access() twice: once each on different revisions of the same node as though the results could differ!

$access[$cid] = node_access($op, $node_current_revision, $account) && ($is_current_revision || node_access($op, $node, $account));

Here's a clean patch.

arknoll’s picture

This appears to still be a problem in D8. The node_access table still only has a nid column. It does not use vid or uuid.