diff options
| -rw-r--r-- | modules/simpletest/tests/actions.test | 64 | ||||
| -rw-r--r-- | modules/trigger/trigger.admin.inc | 12 | ||||
| -rw-r--r-- | modules/trigger/trigger.module | 56 |
3 files changed, 79 insertions, 53 deletions
diff --git a/modules/simpletest/tests/actions.test b/modules/simpletest/tests/actions.test index 6619c24d3..5d1c52d56 100644 --- a/modules/simpletest/tests/actions.test +++ b/modules/simpletest/tests/actions.test @@ -6,7 +6,7 @@ class ActionsConfigurationTestCase extends DrupalWebTestCase { return array( 'name' => 'Actions configuration', 'description' => 'Tests complex actions configuration by adding, editing, and deleting a complex action.', - 'group' => 'System', + 'group' => 'Actions', ); } @@ -61,3 +61,65 @@ class ActionsConfigurationTestCase extends DrupalWebTestCase { $this->assertFalse($exists, t('Make sure the action is gone from the database after being deleted.')); } } + +/** + * Test actions executing in a potential loop, and make sure they abort properly. + */ +class ActionLoopTestCase extends DrupalWebTestCase { + public static function getInfo() { + return array( + 'name' => 'Actions executing in a potentially infinite loop', + 'description' => 'Tests actions executing in a loop, and makes sure they abort properly.', + 'group' => 'Actions', + ); + } + + function setUp() { + parent::setUp('dblog', 'trigger', 'actions_loop_test'); + } + + /** + * Set up a loop with 10-50 recursions, and see if it aborts properly. + */ + function testActionLoop() { + $user = $this->drupalCreateUser(array('administer actions')); + $this->drupalLogin($user); + + $hash = md5('actions_loop_test_log'); + $edit = array('aid' => $hash); + $this->drupalPost('admin/structure/trigger/actions_loop_test', $edit, t('Assign')); + + // Delete any existing watchdog messages to clear the plethora of + // "Action added" messages from when Drupal was installed. + db_delete('watchdog')->execute(); + $this->triggerActions(); + + // Clear the log again for another test, this time with a random maximum. + db_delete('watchdog')->execute(); + variable_set('actions_max_stack', mt_rand(10, 50)); + $this->triggerActions(); + } + + /** + * Create an infinite loop by causing a watchdog message to be set, + * which causes the actions to be triggered again, up to default of 35 times. + */ + protected function triggerActions() { + $this->drupalGet('<front>', array('query' => array('trigger_actions_on_watchdog' => TRUE))); + $expected = array(); + $expected[] = 'Triggering action loop'; + for ($i = 1; $i <= variable_get('actions_max_stack', 35); $i++) { + $expected[] = "Test log #$i"; + } + $expected[] = 'Stack overflow: too many calls to actions_do(). Aborting to prevent infinite recursion.'; + + $result = db_query("SELECT * FROM {watchdog} WHERE type = 'actions_loop_test' OR type = 'actions' ORDER BY timestamp"); + $loop_started = FALSE; + while ($row = db_fetch_object($result)) { + + $expected_message = array_shift($expected); + $this->assertEqual($row->message, $expected_message, t('Expected message %expected, got %message.', array('%expected' => $expected_message, '%message' => $row->message))); + } + $this->assertTrue(empty($expected), t('All expected messages found.')); + } +} diff --git a/modules/trigger/trigger.admin.inc b/modules/trigger/trigger.admin.inc index 22f26a704..51d004036 100644 --- a/modules/trigger/trigger.admin.inc +++ b/modules/trigger/trigger.admin.inc @@ -23,11 +23,13 @@ function trigger_assign($type = NULL) { $build = array(); $hooks = module_invoke_all('hook_info'); - foreach ($hooks as $module => $hook) { - if (isset($hook[$type])) { - foreach ($hook[$type] as $op => $description) { - $form_id = 'trigger_' . $type . '_' . $op . '_assign_form'; - $build[$form_id] = drupal_get_form($form_id, $type, $op, $description['runs when']); + foreach ($hooks as $module => $module_hooks) { + if ($module == $type) { + foreach ($module_hooks as $hook => $data) { + foreach ($data as $op => $description) { + $form_id = 'trigger_' . $hook . '_' . $op . '_assign_form'; + $build[$form_id] = drupal_get_form($form_id, $hook, $op, $description['runs when']); + } } } } diff --git a/modules/trigger/trigger.module b/modules/trigger/trigger.module index e810755c8..d6fedc64f 100644 --- a/modules/trigger/trigger.module +++ b/modules/trigger/trigger.module @@ -39,47 +39,15 @@ function trigger_menu() { 'title' => 'Triggers', 'description' => 'Tell Drupal when to execute actions.', 'page callback' => 'trigger_assign', - 'access callback' => 'trigger_access_check', - 'access arguments' => array('node'), - ); - // We don't use a menu wildcard here because these are tabs, - // not invisible items. - $items['admin/structure/trigger/node'] = array( - 'title' => 'Content', - 'page callback' => 'trigger_assign', - 'page arguments' => array('node'), - 'access callback' => 'trigger_access_check', - 'access arguments' => array('node'), - 'type' => MENU_LOCAL_TASK, - ); - $items['admin/structure/trigger/user'] = array( - 'title' => 'Users', - 'page callback' => 'trigger_assign', - 'page arguments' => array('user'), - 'access callback' => 'trigger_access_check', - 'access arguments' => array('user'), - 'type' => MENU_LOCAL_TASK, - ); - $items['admin/structure/trigger/comment'] = array( - 'title' => 'Comments', - 'page callback' => 'trigger_assign', - 'page arguments' => array('comment'), - 'access callback' => 'trigger_access_check', - 'access arguments' => array('comment'), - 'type' => MENU_LOCAL_TASK, - ); - $items['admin/structure/trigger/taxonomy'] = array( - 'title' => 'Taxonomy', - 'page callback' => 'trigger_assign', - 'page arguments' => array('taxonomy'), - 'access callback' => 'trigger_access_check', - 'access arguments' => array('taxonomy'), - 'type' => MENU_LOCAL_TASK, + 'access arguments' => array('administer actions'), ); + + // Explicitly define the system menu item so we can label it "cron" rather + // than "system". $items['admin/structure/trigger/cron'] = array( 'title' => 'Cron', 'page callback' => 'trigger_assign', - 'page arguments' => array('cron'), + 'page arguments' => array('system'), 'access arguments' => array('administer actions'), 'type' => MENU_LOCAL_TASK, ); @@ -88,11 +56,12 @@ function trigger_menu() { // their hooks and have actions assignable to them. $hooks = module_invoke_all('hook_info'); foreach ($hooks as $module => $hook) { - // We've already done these. - if (in_array($module, array('node', 'comment', 'user', 'system', 'taxonomy'))) { + // We've already done system.module. + if ($module == 'system') { continue; } $info = db_select('system') + ->fields('system', array('info')) ->condition('name', $module) ->execute() ->fetchField(); @@ -102,7 +71,7 @@ function trigger_menu() { 'title' => $nice_name, 'page callback' => 'trigger_assign', 'page arguments' => array($module), - 'access arguments' => array($module), + 'access arguments' => array('administer actions'), 'type' => MENU_LOCAL_TASK, ); } @@ -119,13 +88,6 @@ function trigger_menu() { } /** - * Access callback for menu system. - */ -function trigger_access_check($module) { - return (module_exists($module) && user_access('administer actions')); -} - -/** * Get the aids of actions to be executed for a hook-op combination. * * @param $hook |