- 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

@@ -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();
 }

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.

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.

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')

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));
+  }
+}

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'])) {