- 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.

3 files changed, 54 insertions, 1 deletions

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'),
     ),
   );
 
@@ -2865,6 +2872,14 @@ function system_update_7044() {
 }
 
 /**
+ * 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));
+  }
+}