From 2dc3c05a2b40653f10bd57e76007de22b6468a8f Mon Sep 17 00:00:00 2001 From: Dries Buytaert Date: Sun, 18 Oct 2009 11:34:45 +0000 Subject: - Patch #589126 by mfb: fixed bug with user module using a flood window of 6 hours, but flood events more than 1 hour old being deleted by cron. Improved API documentation, and added tests. --- includes/common.inc | 8 +++++++- modules/contact/contact.pages.inc | 4 ++-- modules/system/system.install | 15 +++++++++++++++ modules/system/system.module | 2 +- modules/system/system.test | 38 ++++++++++++++++++++++++++++++++++++++ modules/user/user.module | 4 ++-- 6 files changed, 65 insertions(+), 6 deletions(-) diff --git a/includes/common.inc b/includes/common.inc index bcc07fe29..5ecdcddf6 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -1563,10 +1563,15 @@ function valid_url($url, $absolute = FALSE) { * * @param $name * The name of an event. + * @param $window + * Optional number of seconds before this event expires. Defaults to 3600 (1 + * hour). Typically uses the same value as the flood_is_allowed() $window + * parameter. Expired events are purged on cron run to prevent the flood table + * from growing indefinitely. * @param $identifier * Optional identifier (defaults to the current user's IP address). */ -function flood_register_event($name, $identifier = NULL) { +function flood_register_event($name, $window = 3600, $identifier = NULL) { if (!isset($identifier)) { $identifier = ip_address(); } @@ -1575,6 +1580,7 @@ function flood_register_event($name, $identifier = NULL) { 'event' => $name, 'identifier' => $identifier, 'timestamp' => REQUEST_TIME, + 'expiration' => REQUEST_TIME + $window, )) ->execute(); } diff --git a/modules/contact/contact.pages.inc b/modules/contact/contact.pages.inc index fa5606448..72603a39d 100644 --- a/modules/contact/contact.pages.inc +++ b/modules/contact/contact.pages.inc @@ -154,7 +154,7 @@ function contact_site_form_submit($form, &$form_state) { drupal_mail('contact', 'page_autoreply', $from, $language, $values, $to); } - flood_register_event('contact'); + flood_register_event('contact', variable_get('contact_threshold_window', 3600)); watchdog('mail', '%sender-name (@sender-from) sent an e-mail regarding %category.', array('%sender-name' => $values['name'], '@sender-from' => $from, '%category' => $values['category']['category'])); // Jump to home page rather than back to contact page to avoid @@ -278,7 +278,7 @@ function contact_personal_form_submit($form, &$form_state) { drupal_mail('contact', 'user_copy', $from, $language, $values, $from); } - flood_register_event('contact'); + flood_register_event('contact', variable_get('contact_threshold_window', 3600)); watchdog('mail', '%sender-name (@sender-from) sent %recipient-name an e-mail.', array('%sender-name' => $values['name'], '@sender-from' => $from, '%recipient-name' => $values['recipient']->name)); // Jump to the contacted user's profile page. diff --git a/modules/system/system.install b/modules/system/system.install index 6ba6d1ff9..38753a045 100644 --- a/modules/system/system.install +++ b/modules/system/system.install @@ -898,10 +898,17 @@ function system_schema() { 'not null' => TRUE, 'default' => 0, ), + 'expiration' => array( + 'description' => 'Expiration timestamp. Expired events are purged on cron run.', + 'type' => 'int', + 'not null' => TRUE, + 'default' => 0, + ), ), 'primary key' => array('fid'), 'indexes' => array( 'allow' => array('event', 'identifier', 'timestamp'), + 'purge' => array('expiration'), ), ); @@ -2864,6 +2871,14 @@ function system_update_7044() { db_delete('sequences')->condition('value', $max, '<'); } +/** + * Add expiration field to the {flood} table. + */ +function system_update_7044() { + db_add_field('flood', 'expiration', array('description' => 'Expiration timestamp. Expired events are purged on cron run.', 'type' => 'int', 'not null' => TRUE, 'default' => 0)); + db_add_index('flood', 'purge', array('expiration')); +} + /** * @} End of "defgroup updates-6.x-to-7.x" * The next series of updates should start at 8000. diff --git a/modules/system/system.module b/modules/system/system.module index 5c1abf524..80d22d26b 100644 --- a/modules/system/system.module +++ b/modules/system/system.module @@ -2597,7 +2597,7 @@ function system_get_module_admin_tasks($module) { function system_cron() { // Cleanup the flood. db_delete('flood') - ->condition('timestamp', REQUEST_TIME - 3600, '<') + ->condition('expiration', REQUEST_TIME, '<') ->execute(); // Cleanup the batch table. db_delete('batch') diff --git a/modules/system/system.test b/modules/system/system.test index 4f684e876..3f66d677b 100644 --- a/modules/system/system.test +++ b/modules/system/system.test @@ -1503,3 +1503,41 @@ class UpdateScriptFunctionalTest extends DrupalWebTestCase { $this->assertResponse(200); } } + +/** + * Functional tests for the flood control mechanism. + */ +class FloodFunctionalTest extends DrupalWebTestCase { + public static function getInfo() { + return array( + 'name' => 'Flood control mechanism', + 'description' => 'Functional tests for the flood control mechanism.', + 'group' => 'System', + ); + } + + /** + * Test flood control mechanism clean-up. + */ + function testCleanUp() { + $threshold = 1; + $window_expired = -1; + $name = 'flood_test_cleanup'; + + // Register expired event. + flood_register_event($name, $window_expired); + // Verify event is not allowed. + $this->assertFalse(flood_is_allowed($name, $threshold)); + // Run cron and verify event is now allowed. + $this->cronRun(); + $this->assertTrue(flood_is_allowed($name, $threshold)); + + // Register unexpired event. + flood_register_event($name); + // Verify event is not allowed. + $this->assertFalse(flood_is_allowed($name, $threshold)); + // Run cron and verify event is still not allowed. + $this->cronRun(); + $this->assertFalse(flood_is_allowed($name, $threshold)); + } +} diff --git a/modules/user/user.module b/modules/user/user.module index 241e6ee84..cf10cfe34 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -1801,10 +1801,10 @@ function user_login_authenticate_validate($form, &$form_state) { function user_login_final_validate($form, &$form_state) { if (empty($form_state['uid'])) { // Always register an IP-based failed login event. - flood_register_event('failed_login_attempt_ip'); + flood_register_event('failed_login_attempt_ip', variable_get('user_failed_login_ip_window', 3600)); // Register a per-user failed login event. if (isset($form_state['flood_control_user_identifier'])) { - flood_register_event('failed_login_attempt_user', $form_state['flood_control_user_identifier']); + flood_register_event('failed_login_attempt_user', variable_get('user_failed_login_user_window', 21600), $form_state['flood_control_user_identifier']); } if (isset($form_state['flood_control_triggered'])) { -- cgit v1.2.3