hook_menu denies access to the revisions page when no multiple revisions for the page exist:

        $revisions_access = (user_access('view revisions') || user_access('administer nodes')) &&
                            node_access('view', $node);
// $revisions_access is later used as the 'access' value of the menu array, of course.

This is misleading. A "false" access setting will cause Drupal to send a 403 Forbidden error to the user, who (like me) will be pretty confused that there is actually a page the admin is not authorized to see (something similar happens with user/register, but there at least it's immediately apparent that logged-in users shouldn't be able to register again). This unexpected behavior is why I'd call it a bug.

I know the intention is to not show the tab if it doesn't do anything useful. However, a workaround that does the same, but results in a more sensible error if any curious user does manually type in the /revisions URL:

$revision_count = db_result(db_query('SELECT COUNT(vid) FROM {node_revisions} WHERE nid = %d', arg(1))) > 1;
$menu_type = $revision_count?MENU_LOCAL_TASK:MENU_CALLBACK;
// where $menu_type later becomes the 'type' value of the menu array.

The page will still exist and be accessible, but the local task tab is gone.

In the callback function itself, a message would then say that "no revisions exist". And bam, no more weird 403 pages. :)

Comments

cburschka’s picture

cburschka
they
commented

Correction: The first of the two code snippets currently reads

        $revisions_access = (user_access('view revisions') || user_access('administer nodes')) &&
                            node_access('view', $node) &&
                            db_result(db_query('SELECT COUNT(vid) FROM {node_revisions} WHERE nid = %d', arg(1))) > 1;

(with the db_result that sets access to false when less than 2 revisions exist)

What I quoted was what it would look after the proposed change.

moonray’s picture

moonray commented
Project: Diff » Revision Moderation

Shouldn't this be under "revision moderation?"

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Project: Revision Moderation » Drupal core
Version: 5.x-1.x-dev » 6.x-dev
Component: Code » node system
Priority: Normal » Minor

I don't really see how either module is doing this. This behaviour comes from core, as I can duplicate a 403 on node/X/revisions with only one revision on a clean install.

In 5.x:

$revisions_access = ((user_access('view revisions') || user_access('administer nodes')) && node_access('view', $node) && db_result(db_query('SELECT COUNT(vid) FROM {node_revisions} WHERE nid = %d', arg(1))) > 1);

In 6.x:

function _node_revision_access($node) {
  return (user_access('view revisions') || user_access('administer nodes')) && node_access('view', $node) && db_result(db_query('SELECT COUNT(vid) FROM {node_revisions} WHERE nid = %d', $node->nid)) > 1;
}

Downgrading to a minor bug. The only way to possibly get to a node/X/revisions tab that doesn't exist yet is to manually type it in.

eriktoyra’s picture

eriktoyra commented
Status: Closed (works as designed) » Active

Downgrading to a minor bug. The only way to possibly get to a node/X/revisions tab that doesn't exist yet is to manually type it in.

This is not completely true. You can also run into this problem if using a view with one of the fields set to Node revision: Title (available if Revision moderation module is installed). This would generate a node title link that looks like this: http://example.com/node/[nid]/revisions/[vid]/view. This is a blocking bug for me in my View. This should be reported as a bug in core since it's a rather annoying bug.

One solution for the Revision moderation module is to rewrite the function revision_moderation_handler_view_revision_link to exclude the /revisions/[vid] part from the uri if only one revision exists.

Edit: I have encountered this problem in Revision Moderation 5.x-1.3.

damien tournoud’s picture

damien tournoud commented
Status: Active » Closed (works as designed)
Issue tags: +view, +Access denied, +revision moderation, +revision tab

You can / should use hook_menu_alter() if you want to change the default access control mechanisms on the revision tab. The current behavior is by design.

eriktoyra’s picture

eriktoyra commented
Status: Active » Closed (works as designed)

Thanks for the information. But in Drupal 5.x, hook_menu_alter() is not available. Can I achieve the same with hook_menu()?

Since the Revision module will assume that I have access to the revision tab, I would say that my solution to remove the /revisions/[vid]/view part of the uri when only one revision exists, should be safe. Would that be considered bad programming?

johnv’s picture

johnv
Primary language Dutch
commented
Project: Drupal core » Revision Moderation
Version: 6.x-dev » 6.x-1.x-dev
Component: node system » Code
Status: Closed (works as designed) » Closed (duplicate)

IMO root cause is Drupal core, which disables the page when only one revision exists. It should be dealt with there.
Tagging this as a duplicate of #808730: Show the Revisions tab/page even when only one revision exists.