Feed refresh fails to delete old feed nodes when run as a cron job.

feedapi_node.module's _feedapi_node_delete() (at line 375) calls node_delete() Drupal function.

Unlike node_save(), node_delete() runs node_access(), which returns true only if a user with proper permission is logged in. Since cron jobs would run without user interaction, node deleting routine would have to impersonate a user, probably user 1 (admin) to delete nodes successfully.

Just before node_delete() is run, $user object would need to be set to admin's account. Following is my implementation.


  global $user;
  if (!$user->uid and strpos($_SERVER['REQUEST_URI'], 'cron.php')) {
    $user = user_load(array('uid' => 1));
  }
  node_delete($feed_item->nid);

The conditional makes sure $user object is set to admin only once because the enclosing function tends to run multiple times during feed refresh. This would run even if someone modified his cron.php to accept query string like cron.php?id=1234 for whatever reason.

Comments

aron novak’s picture

Well, this seems to be a possible solution.
But now i have some problems:
The first user remains logged in after cron, right?

ki’s picture

Yes, if somebody opens the site at the web server, he will be logged in as admin.
Maybe we could destroy $user object after things are taken care of.


$user = user_load(array('uid' => 0));

//Or
$user = new stdClass();

//Or simply
$user = null;

It's kind of inefficient though if we have to reset $user every time we run node_delete().

Or just write our own node_delete() without checking node_access()?

aron novak’s picture

Component: Code » Code feedapi_node
Status: Active » Fixed

Fixed. Thanks for catching it.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

mark.’s picture

Status: Closed (fixed) » Active

Bookmarking.

I'm having this problem with the 5.x version.

aron novak’s picture

Version: 6.x-1.4 » 5.x-1.x-dev
Status: Active » Closed (won't fix)

Unfortunately the 5.x branch is unsupported, however it's really easy to backport the above solution.