Multichoice module causes string of access_denied() in cron, search output and node preview [#536116]

After enabling search, whenever I ran cron I would get pages and pages of Access Denied. The browser would display one for each node that was being indexed.

I finally figured out that it was only multichoice nodes that had this problem, and it's because of this access_denied() call in view:

/**
* Implementation of hook_view().
*/
function multichoice_view($node, $teaser = FALSE, $page = FALSE) {
if (!$teaser && $page) {
// This is not necessary:
//$mynode = node_prepare($node, $teaser);
$node->content['body'] = array('#value' => multichoice_render_question($node));
return $node;
}
elseif ($teaser) {
$node = node_prepare($node, $teaser);
return $node;
}
else {
drupal_access_denied();
}
}

For me this is the final nail in the coffin of multichoice and I'm switching to choice. I expect it'll be humming along nicely by 4.0 release. I thought about marking this "won't fix" but after some thought I realized we may keep multichoice in 4.0, however deprecated. Users may want to keep using multichoice if it's working for them, and maybe they have quiz results that depend on their multichoice nodes. Keeping it also gives us the option of not implementing a conversion system and instead having people export their multichoice questions and reimporting them to get choice questions.

Just some extra thoughts on a bug report.

Comment	File	Size	Author
#19	quiz.multichoice_access_denied.patch	533 bytes	rfay
#14	quiz_cron.png	43.18 KB	patator

Comments

Comment #1

turadg commented 3 August 2009 at 19:46

Committed a fix that removes access_denied() in multichoice_view().

This could really use some review as I'm not sure what the data protection implications are of this. Node access should be handled by other hooks, but I'm not sure they are.

Comment #2

turadg commented 12 August 2009 at 16:40

Status:

Needs work

» Fixed

This is fixed well enough, considering that the multichoice module is completely redone by Falcon in 4.0.

Comment #3

26 August 2009 at 16:50

Status:

Fixed

» Closed (fixed)

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

Comment #4

mlncn commented 23 September 2009 at 00:55

In 3.3: Other weirdness from the multichoice content types:

warning: Invalid argument supplied for foreach() in /var/www/anjali/drupal/modules/taxonomy/taxonomy.module on line 1214.

/**
 * Implementation of hook_nodeapi('update_index').
 */
function taxonomy_node_update_index(&$node) {
  $output = array();
  foreach ($node->taxonomy as $term) {
    $output[] = $term->name;
  }
  if (count($output)) {
    return '<strong>('. implode(', ', $output) .')</strong>';
  }
}


// add ing this to the above
drupal_set_message('<pre>'. var_export(debug_backtrace(),TRUE) .'</pre>');


// provides this:
array (
  0 => 
  array (
    'file' => '/home/ben/code/anjali/drupal/modules/taxonomy/taxonomy.module',
    'line' => 1205,
    'function' => 'taxonomy_node_update_index',
    'args' => 
    array (
      0 => 
      stdClass::__set_state(array(
         'content' => NULL,
         'body' => NULL,
      )),
    ),
  ),
  1 => 
  array (
    'file' => '/home/ben/code/anjali/drupal/modules/node/node.module',
    'line' => 673,
    'function' => 'taxonomy_nodeapi',
    'args' => 
    array (
      0 => 
      stdClass::__set_state(array(
         'content' => NULL,
         'body' => NULL,
      )),
      1 => 'update index',
      2 => NULL,
      3 => NULL,
    ),
  ),
  2 => 
  array (
    'file' => '/home/ben/code/anjali/drupal/modules/node/node.module',
    'line' => 1825,
    'function' => 'node_invoke_nodeapi',
    'args' => 
    array (
      0 => 
      stdClass::__set_state(array(
         'content' => NULL,
         'body' => NULL,
      )),
      1 => 'update index',
    ),
  ),
  3 => 
  array (
    'file' => '/home/ben/code/anjali/drupal/modules/node/node.module',
    'line' => 1801,
    'function' => '_node_index_node',
    'args' => 
    array (
      0 => 
      stdClass::__set_state(array(
         'nid' => '24',
      )),
    ),
  ),
  4 => 
  array (
    'function' => 'node_update_index',
    'args' => 
    array (
    ),
  ),
  5 => 
  array (
    'file' => '/home/ben/code/anjali/drupal/includes/module.inc',
    'line' => 450,
    'function' => 'call_user_func_array',
    'args' => 
    array (
      0 => 'node_update_index',
      1 => 
      array (
      ),
    ),
  ),
  6 => 
  array (
    'file' => '/home/ben/code/anjali/drupal/modules/search/search.module',
    'line' => 273,
    'function' => 'module_invoke',
    'args' => 
    array (
      0 => 'node',
      1 => 'update_index',
    ),
  ),
  7 => 
  array (
    'function' => 'search_cron',
    'args' => 
    array (
    ),
  ),
  8 => 
  array (
    'file' => '/home/ben/code/anjali/drupal/includes/module.inc',
    'line' => 471,
    'function' => 'call_user_func_array',
    'args' => 
    array (
      0 => 'search_cron',
      1 => 
      array (
      ),
    ),
  ),
  9 => 
  array (
    'file' => '/home/ben/code/anjali/drupal/includes/common.inc',
    'line' => 2661,
    'function' => 'module_invoke_all',
    'args' => 
    array (
      0 => 'cron',
    ),
  ),
  10 => 
  array (
    'file' => '/home/ben/code/anjali/drupal/cron.php',
    'line' => 11,
    'function' => 'drupal_cron_run',
    'args' => 
    array (
    ),
  ),
)

Comment #5

r_honey commented 12 October 2009 at 15:30

Hi turadg, can you provide the fix you committed. I have a production site using lots of multichoice questions filling up my log with "access denied" errors on each cron run.

Comment #6

patator commented 13 October 2009 at 08:31

Error cron has appeared with 4dev (new installation) or before has been installed 3.3?

Comment #7

r_honey commented 13 October 2009 at 09:35

I have a Drupal 6.13 installation with Quiz 3.3.

Obviously, I cannot use dev versions on production sites. I am requesting a patch for 3.3. I checked the latest 3.3 dev branch. I did not see anything significantly changed from 3.3 there.

On 4 however, the 3 files in multichoice module have been replaced by a ton of files. And I could not locate the hook_view function that causes these access denied errors in 4.0 multichoice.module.

Comment #8

fredklopper commented 14 October 2009 at 07:41

Subscribing

Comment #9

turadg commented 14 October 2009 at 22:10

I can't find that patch now but it's quite simple. Just take out this line:
drupal_access_denied();

That'll leave the else{} block empty, so you might want to put something in there. That's what gets presented if the view is not a page or teaser, such as in a cron-initiated search indexing pass.

Comment #10

r_honey commented 15 October 2009 at 04:17

If I leave that else block empty (I am really not sure what to put in there), would that matter or generate any further problem, except for that the multichoice questions would not be indexed??

Comment #11

benoit.borrel commented 15 October 2009 at 13:00

subscribing

Comment #12

turadg commented 15 October 2009 at 13:13

r_honey, I believe the only side-effect will be that the question is not search indexed. if you want to be safe, you could turn the elif into the else.


 /**
  * Implementation of hook_view().
  */
function multichoice_view($node, $teaser = FALSE, $page = FALSE) {
  if (!$teaser && $page) {
    $node->content['body'] = array('#value' => multichoice_render_question($node));
    return $node;
  }
  else {
    $node = node_prepare($node, $teaser);
    return $node;
  }
}

Comment #13

r_honey commented 15 October 2009 at 15:51

Thanx turadg, I updated multichoice.module as suggested by you. An immediate manual cron run after this updation completed successfully, without producing any "access denied" errors in log. I hope the 4.0 release would solve this problem.

As a side-note, I would also like to mention that I needed to hack into the Drupal core to prevent another problem caused due to the multichoice module, that I tried to discuss on a related thread mentioned below, but did not get any satisfactory reply:
http://drupal.org/node/259632#comment-2025530

This seems like due to some taxonomy term problems in multichoice nodes.

Comment #14

patator commented 17 October 2009 at 05:38

Status	File	Size
new	quiz_cron.png	43.18 KB

I could not solve the problem. Updated quiz to 6.x-4.x-dev 2009-Oct-17. In end multichoice.module added the changes as proposed turadg. Launched update. Received an error. Cron runs with errors as before.

Comment #15

turadg commented 18 October 2009 at 19:01

patator, your report is unrelated to the "access denied" bug. the code you checked out doesn't have the access denied bug. the problem is with the database update hooks. you might report it on a new ticket. if so, be sure to specify what version of the code you were running before the code update. it would also help to include the MySQL DESCRIBE TABLE output on the multichoice table.

Comment #16

hanskuiters commented 4 November 2009 at 15:18

Title:

Multichoice module causes string of access_denied() in cron and search output

» Multichoice module causes string of access_denied() in cron, search output and node preview

Another situation related to the "access denied" bug. When creating/editing a multichoise node, and hit 'preview', the same 'access denied' error occurs. Leaving the last 'else' empty, like turadg suggests, eliminates the error but leaves the preview empty.
So instead I copied two lines from the first 'if' to the last 'if'. And that gave a strange function, so I did a bit of rearrange.

/**
 * Implementation of hook_view().
 */
function multichoice_view($node, $teaser = FALSE, $page = FALSE) {
  if ($teaser) {
    $node = node_prepare($node, $teaser);
    return $node;
  }
  else {
    $node->content['body'] = array('#value' => multichoice_render_question($node));
    return $node;
  }
}