Not loading node status in FeedsNodeProcessor.inc is causing issue with Content Access module [#1300500]

The hook_node_access_records function in the Content Access module that returns all the privileges to apply to a node gets essentially bypassed because the "entityLoad" function in FeedsNodeProcessor.inc does not load the node's status.

When not updating a node, but replacing, I think the db query should grab the node status as well. I think. Is this a bug or working as intended?

protected function entityLoad(FeedsSource $source, $nid) {
    if ($this->config['update_existing'] == FEEDS_UPDATE_EXISTING) {
      $node = node_load($nid, NULL, TRUE);
    }
    else {
      // We're replacing the existing node. Only save the absolutely necessary.
      $node = db_query("SELECT created, nid, vid, type FROM {node} WHERE nid = :nid", array(':nid' => $nid))->fetchObject();
      $node->uid = $this->config['author'];
    }
    node_object_prepare($node);
    // Populate properties that are set by node_object_prepare().
    if ($this->config['update_existing'] == FEEDS_UPDATE_EXISTING) {
      $node->log = 'Updated by FeedsNodeProcessor';
    }
    else {
      $node->log = 'Replaced by FeedsNodeProcessor';
    }
    return $node;
  }

Thanks,
-Derek

Comment	File	Size	Author
#1	1300500-1-node-loads-missing-status.patch	694 bytes	milesw

Comments

Comment #1

milesw commented 15 December 2011 at 11:37

Status:

Active

» Needs review

Status	File	Size
new	1300500-1-node-loads-missing-status.patch	694 bytes

This just bit me hard. None of the existing nodes were being replaced and I was getting the following exception for every item:

SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'status' cannot be null

My node processor is set to "Replace existing nodes". The first import runs fine. Subsequent imports fail because the status never gets set.

Simply adding 'status' to the db query fixed the issue. It's not clear why other missing values, such as sticky and promote, don't cause similar errors since their database config is similar.

 // OLD
 $node = db_query("SELECT created, nid, vid, type FROM {node} WHERE nid = :nid", array(':nid' => $nid))->fetchObject();
 // NEW
 $node = db_query("SELECT created, nid, vid, type, status FROM {node} WHERE nid = :nid", array(':nid' => $nid))->fetchObject();

Patch attached.

Comment #2

emackn commented 22 December 2011 at 23:13

Can you provide some other info on how to reproduce the error. Is this a CSV file node import? Any testing data files, etc that you can provide help me get these issues worked out much more quickly.

Comment #3

emackn commented 29 December 2011 at 22:02

Status:

Needs review

» Postponed (maintainer needs more info)

Comment #4

milesw commented 30 December 2011 at 04:16

Steps to reproduce:

Create a new importer with File Fetcher, CSV Parser, Node Processor
On Node Processor settings, change to "Replace existing nodes"
On Mappings form, add the following mappings...
Title -> Title
GUID -> GUID
Visit /import page and open the standalone form for the importer.
Choose this sample CSV to import: feeds/tests/feeds/nodes.csv
Run the import, which should create 9 nodes.
Go back to the Mappings form and add another mapping...
Body -> Body
(this is only to trigger replacement of the nodes, otherwise Feeds will report "No new content")
Run the import, and you should see the above error repeated 9 times.

Just confirmed this on the dev branch.

Comment #5

milesw commented 30 December 2011 at 04:16

Status:

Postponed (maintainer needs more info)

» Needs review

Comment #6

rooby commented 31 July 2012 at 07:21

Priority:

Normal

» Major

I have also seen this problem.

The proposed solution makes sense.

I will test it out soon and let you know if it works.

By the way I think this is higher than normal priority because access control is serious.

Comment #7

rooby commented 31 July 2012 at 07:22

Version:

7.x-2.0-alpha4

» 7.x-2.0-alpha5

Comment #8

rooby commented 1 August 2012 at 00:11

Status:

Needs review

» Reviewed & tested by the community

I can confirm the fix.

I am testing on a site using content access and importing from an RSS feed.

I am using the devel node access module in debug mode to view node access grants on a node.
I have littered the node & content access modules with dpm() calls to see what is going on.
I am testing by:

1. Running the import so it imports the nodes.
2. In the database, update the feeds_item table to change the hash value of the node I am viewing.
3. Run the import again, it updates the node with the changed hash, and content access breaks. (If I then manually update the node, content access is properly saved and is working correctly.)
4. Apply the patch in #1.
5. Change the node's hash in the database again.
6. Run the import again, it updates the node, this time content access is saved correctly.

I can also confirm again that the content access module doesn't properly set access control if !$node->status.

Also, just to note that this isn't actually a security hole, because it results in people not being able to see content they should be able to, not the other way around.