diff --git a/modules/simpletest/tests/actions.test b/modules/simpletest/tests/actions.test
index 6619c24d3a79851744d92cff3d477c9bfc54f9df..5d1c52d56a027231be91c8b562f5c69abce64536 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 22f26a70482e32193725e871649108b83502ecd2..51d00403602fd8cac12792531fd034ce9611c75f 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 e810755c82ebcaa93ee75edc8e1af385dc5c2827..d6fedc64f8cf7bc8a2f6bb0fac28a574bc6901bf 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,
);
}
@@ -118,13 +87,6 @@ function trigger_menu() {
return $items;
}
-/**
- * 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.
*