Hello -- I upgraded to 6.2 (from 4.7!!) a month or two back. Since the upgrade, I've had this problem twice:

- Anonymous users do not see any nodes (they get the "how to set up drupal" message on the home page and "access denied" on a specific node).
- It turns out that anonymous has disappeared from the node_access table
- If I run the sql command: INSERT INTO node_access (nid, gid, realm, grant_view, grant_update, grant_delete) VALUES (0, 0, 'all', 1, 0, 0); it adds anon back in and the problem is fixed (as recommended here: http://drupal.org/node/134505 )

It's nice to have a quick fix but this happens seemingly at random. The second time it happened I am sure that I have made absolutely no changes to the site permissions nor installed any modules or anything like that. It just happened literally overnight.

I see that this bug has been reported for version 5: http://drupal.org/node/124137

But I am on version 6 so I thought I should post a separate report. The previously linked forum thread, where I found the fix, shows that others are having this happen to them as well on version 6.

I am running on linux/apache with php5 and mysql 4.1. This never happened until the upgrade to 6.2. If I can provide more info please let me know...

Thanks,
Rich

Files: 
CommentFileSizeAuthor
#28 node_access-wiped-out-prevent.patch1.28 KBgnindl
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es). View
#27 node_access-wiped-out-prevent.patch1.26 KBgnindl
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es). View
#26 282555-node_access-wiped-out-26.patch1.23 KBjcisio
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es). View
#20 node-access-grants-delete-check.patch1.19 KBgnindl
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch node-access-grants-delete-check.patch. Unable to apply patch. See the log in the details link for more information. View
#16 282555_make_sure_we_have_a_node_revisited.patch732 bytesg4b0
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es). View
#11 282555_make_sure_we_have_a_node.patch919 bytesgreggles
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 282555_make_sure_we_have_a_node.patch. View
#2 duplicate_key_bug.txt4.7 KBayurchen

Comments

jbomb’s picture

Version: 6.2 » 6.4

I've also experienced this problem with drupal 6.4

ayurchen’s picture

Version: 6.4 » 6.10
FileSize
4.7 KB

When posting a story (or comment?) Drupal issues INSERT INTO node_revisions ... query. If it fails due to duplicate key Drupal catches the error, but handles it in a peculiar way: at first it logs it (INSERT INTO watchdog ...), and then does DELETE FROM node_access WHERE nid = 0. This command renders the site inaccessible to anyone but root user. For root user it shows the normal site and for others it displays a page shown when Drupal is just installed and has no contents: "Welcome to your new Drupal site bla-bla-bla."

Well, it is true that INSERT INTO node_revisions ... should never fail. However it may do so in MySQL: http://bugs.mysql.com/bug.php?id=41984. And surely, DELETE FROM node_access WHERE nid = 0 does not seem a right thing to do regardless. It seems that Drupal site cannot exist without this row.

Attached excerpt from the mysql query log showing the problem.

wilgrace’s picture

Has anyone found a fix for this? I've just done the same upgrade and am having the same problem with anonymous access. It's driving me nuts!

wilgrace’s picture

Rebuilding user permissions on Post Settings sorted this for me

jbomb’s picture

I think the option to rebuild the permissions table is only available on the "post settings" page if the content access module is installed.

imrook’s picture

I have also ran across this problem, and after looking at the code and a bit of testing, I highly doubt that rebuilding permissions is a permanent solution. My problem arises in this manner: I have two content types, one uses workflow and workflow_access to restrict edits, the second does not use a workflow at all. Here's the first piece of the puzzle from node.module

<?php
function node_access_acquire_grants($node) {
  $grants = module_invoke_all('node_access_records', $node);
  if (empty($grants)) {
    $grants[] = array('realm' => 'all', 'gid' => 0, 'grant_view' => 1, 'grant_update' => 0, 'grant_delete' => 0);
  }
  else {
    // retain grants by highest priority
    $grant_by_priority = array();
    foreach ($grants as $g) {
      $grant_by_priority[intval($g['priority'])][] = $g;
    }
    krsort($grant_by_priority);
    $grants = array_shift($grant_by_priority);
  }

  node_access_write_grants($node, $grants);
}
?>

It appears that the default view permission is only granted if no other modules are granting permissions. Since workflow_access is giving grants sometimes the default view grant is not given. A look at node_access_write_grants() shows the death blow for the default view grant

<?php
function node_access_write_grants($node, $grants, $realm = NULL, $delete = TRUE) {
  if ($delete) {
    $query = 'DELETE FROM {node_access} WHERE nid = %d';
    if ($realm) {
      $query .= " AND realm in ('%s', 'all')";
    }
    db_query($query, $node->nid, $realm);
  }
...
?>

So it looks like delete is always gonna be true in this invocation and regardless of whether realm is set or not, the all grant gets deleted. What I haven't traced down yet is why rebuilding permissions restores the grant to it's proper state. I do know, however, that subsequent edits will break the permission again. I'm not really sure the correct approach to solving this problem, that's why I haven't supplied a patch. I think the default behavior of Drupal is confusing and leaves a lot of corner cases for access control modules to cover. I'm going to look over the rebuild permissions code and see if I can find any insight there, then I'll check back with some possible solutions.

Leeteq’s picture

Version: 6.10 » 6.x-dev
imrook’s picture

After further investigation, I found that I had a misbehaving access control module. The behavior of Drupal core is correct and matches the documentation. My opinion is that some module developers do not understand how the node_access system works and are creating modules with incorrect assumptions about it.

akarpavicius’s picture

Version: 6.x-dev » 6.12

I can replicate the issue when node saving fails for issues like unsupported characters. I am reading from csv file and creating nodes for each line.

Code:

$importNode = new StdClass();
$importNode->uid = $user->uid;
$importNode->name = (isset($user->name) ? $user->name : '');
$importNode->type = 'employee';
...
node_save($importNode);

if node_save fails, that is $importNode->nid =0, in db log I see a call "delete from node_access where nid=0".
When node_save is successfull, in db log I see a call "delete from node_access where nid=165" (or what ever the node id is)

So I guess, there should be a check that nid>0 when deleting from node_access table, as 0 is a default value for integer and unfortunatelly a special value for node_access table.

salvis’s picture

I was led here by plj in #124137-32: Access denied to anonymous users (was: Node access settings). Some of the posters there may be experiencing this issue here.

I'm not sure what exactly can fail in node_save() that will lead to $node->nid remaining 0, but it's certainly true that the node_access_acquire_grants($node); call near the end of node_save() will cause problems if $node->nid==0.

node_save() could well be made more robust, but I'm not sure there's a consensus that that's the way to go. We could also argue that whoever causes node_save() to fail needs to clean up their act...

greggles’s picture

Version: 6.12 » 6.19
Status: Active » Needs review
FileSize
919 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 282555_make_sure_we_have_a_node.patch. View

Here's a simple patch based on salvis' investigation. I agree that in the ideal case we would figure out why this is happening. On the site where I encounter it the most likely cause is nodes being imported by feeds module. The problem comes up irregularly which makes it really hard to debug, of course, and based on the discussions here that seems fairly common.

So I propose that we just make node_save more robust. The comment is trickier here than the code.

Status: Needs review » Needs work

The last submitted patch, 282555_make_sure_we_have_a_node.patch, failed testing.

salvis’s picture

Thanks for picking this up, greggles!

Do your comment lines go beyond col 80?

Can a node that won't load be displayed at all? If we don't node_access_acquire_grants(), the node becomes inaccessible to everyone except user 1 and users with the 'administer nodes' permission. This should probably be mentioned in the comment.

Should we write a watchdog entry? I imagine that these nodes can cause all sorts of trouble (for search, for example), and the admin should be made aware of them. This might also lead to further investigation and create some pressure on the modules that cause this problem in the first place.

greggles’s picture

If we don't have a nid how can we watchdog anything useful?

salvis’s picture

True, especially because we can't add new strings. Also, it might make more sense to display a message immediately, so that it appears in the proper context.

There's "The post could not be saved." — can we do something with that?

Since we know that something is wrong, it would be good to provide some kind of indication of that fact and give the site maintainer a fighting chance. But I don't have any good ideas...

g4b0’s picture

Version: 6.19 » 6.x-dev
Status: Needs work » Needs review
FileSize
732 bytes
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es). View

I'm falling into the problem described here...
Attached the greggles patch revisited in the git way, I hope it will pass the testing.
I also hope it will be inserted into the 6.21 version.

rbayliss’s picture

This is still an issue. One of our sites had this grant removed after a poorly timed mysql error. This patch looks great to me, and doesn't cause any ill effects I can see. Could we still get this in?

greggles’s picture

Looking at the patch again the only complaint I have is that it could be better to use if (!empty($node->nid)) but I'm not sure if that provides real benefit.

salvis’s picture

Comments still extend beyond col 80.

I think if $node->nid is empty then we were unable to save the node — we didn't get a nid.

  drupal_set_message(t('The post could not be saved.'), 'error');

might be appropriate.

Not sure whether $node->nid is set, so !empty() would probably be better.

gnindl’s picture

FileSize
1.19 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch node-access-grants-delete-check.patch. Unable to apply patch. See the log in the details link for more information. View

I'd rather take inspiration from this thread http://drupal.org/node/124137#comment-1991686. We should put the check into node_access_write_grants() itsself as third party modules, e. g. like workflow_access or apachesolr might call this function directly. Further there should be error message written to the screen and the watchdog (as wished in comment #19).

Status: Needs review » Needs work

The last submitted patch, node-access-grants-delete-check.patch, failed testing.

salvis’s picture

@gnindl:

I'd rather take inspiration from this thread http://drupal.org/node/124137#comment-1991686. We should put the check into node_access_write_grants() itsself as third party modules, e. g. like workflow_access or apachesolr might call this function directly.

You can take your inspiration from wherever you want, but a.d.o clearly says for D7 that no contrib should call node_access_write_grants() directly. This applies to D6 just the same. The fact that it's not mentioned explicitly for D6 is a documentation bug.

Any contrib that calls node_access_write_grants() directly is abusing the node access system and is incapable of successfully cooperating with other node access modules.

gnindl’s picture

You can take your inspiration from wherever you want, but a.d.o clearly says for D7 that no contrib should call node_access_write_grants() directly. This applies to D6 just the same. The fact that it's not mentioned explicitly for D6 is a documentation bug.

Well if that's a rule we should stick to that. Unfortunately there's nothing preventing you from calling this function directly. This would be different if we had access control modifiers, like "protected", with a proper OO design. So you can bitch around this as much as you like, but we let a potential door open for abuse.

But OK, let's imagine all community members stick to that rule and read the documentation which I may quote here:

Note: Don't call this function directly from a contributed module. Call node_access_acquire_grants() instead.

Unfortunately in D6 the function node_access_acquire_grants() doesn't check the node object as well! Therefore I recommend to check the parameter for all possible use cases, not just for node save operations. Introducing a parameter check before running a SQL DELETE statement sounds like common sense.

salvis’s picture

Unfortunately in D6 the function node_access_acquire_grants() doesn't check the node object as well!

That's what #16 tries to fix. While we're touching the file, we should fix the documentation error (#22) as well.

Passing a $node with $node->nid==0 is a programming error, not a user error. This has been discussed elsewhere: We don't display error messages to the user for programming errors, and we certainly don't introduce new strings in D6 anymore.

(Not being able to "wipe out" existing grants is not an error, as there may (rightfully) not be any for a given node. Also, you have trailing spaces and non-conforming comments.)

I'm not sure it makes sense to pursue this at all, unless you're willing to start with D8 and work your way backwards through D7 to D6.

awm’s picture

I am experiencing this as well and would like to to know if this will be committed.

jcisio’s picture

Issue summary: View changes
Priority: Normal » Major
Status: Needs work » Needs review
FileSize
1.23 KB
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es). View

Clean up of patch #26:
- Still check in node_access_write_grants() because modules like workflow_access call it directly. It's a fact and the modules and the code have been there for years.
- Write to watchdog only: it's admin stuff.
- Some coding standard fixes.

I don't care if it get committed directly in D6. I experienced this problem and want to share a patch. The bug makes the whole site unusable, so it's at least major.

gnindl’s picture

FileSize
1.26 KB
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es). View

Improvement of patch #26: Actually the node id should be checked if it's an integer value (>0), otherwise empty strings or the value 0 might wipe out the entry. This way deleting node id 0 in node_access table is made impossible.

gnindl’s picture

FileSize
1.28 KB
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es). View

Amended patch in comment #27, added is_int() check.

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.