diff --git a/code/caching/RateLimitFilter.php b/code/caching/RateLimitFilter.php index 39663c7..e79e00d 100644 --- a/code/caching/RateLimitFilter.php +++ b/code/caching/RateLimitFilter.php @@ -34,6 +34,16 @@ class RateLimitFilter extends ContentFilter { * @var bool */ private static $lock_byuserip = false; + + /** + * Time duration (in sections) to deny further search requests after a successful search. + * Search requests within this time period while another query is in progress will be + * presented with a 429 (rate limit) + * + * @config + * @var int + */ + private static $lock_cooldown = 2; /** * Cache key prefix @@ -74,19 +84,29 @@ class RateLimitFilter extends ContentFilter { // Generate result with rate limiting enabled $limitKey = $this->getCacheKey($key); $cache = $this->getCache(); - if($cacheBegin = $cache->load($limitKey)) { - if(time() - $cacheBegin < $timeout) { + if($lockedUntil = $cache->load($limitKey)) { + if(time() < $lockedUntil) { // Politely inform visitor of limit $response = new \SS_HTTPResponse_Exception('Too Many Requests.', 429); - $response->getResponse()->addHeader('Retry-After', 1 + time() - $cacheBegin); + $response->getResponse()->addHeader('Retry-After', 1 + $lockedUntil - time()); throw $response; } } - // Generate result with rate limit locked - $cache->save(time(), $limitKey); + // Apply rate limit + $cache->save(time() + $timeout, $limitKey); + + // Generate results $result = parent::getContent($key, $callback); - $cache->remove($limitKey); + + // Reset rate limit with optional cooldown + if($cooldown = \Config::inst()->get(get_class(), 'lock_cooldown')) { + // Set cooldown on successful query execution + $cache->save(time() + $cooldown, $limitKey); + } else { + // Without cooldown simply disable lock + $cache->remove($limitKey); + } return $result; } } diff --git a/tests/VersionFeedFunctionalTest.php b/tests/VersionFeedFunctionalTest.php index 92b4d92..f3413e5 100644 --- a/tests/VersionFeedFunctionalTest.php +++ b/tests/VersionFeedFunctionalTest.php @@ -20,6 +20,7 @@ class VersionFeedFunctionalTest extends FunctionalTest { Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_timeout', 20); Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_bypage', false); Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_byuserip', false); + Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_cooldown', false); } public function tearDown() { @@ -63,7 +64,7 @@ class VersionFeedFunctionalTest extends FunctionalTest { Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_byuserip', false); $cache = SS_Cache::factory('VersionFeed_Controller'); $cache->setOption('automatic_serialization', true); - $cache->save(time() - 2, \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX); + $cache->save(time() + 10, \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX); // Test normal hit $response = $this->get($page1->RelativeLink('changes')); @@ -81,7 +82,7 @@ class VersionFeedFunctionalTest extends FunctionalTest { Versioned::get_versionnumber_by_stage('SiteTree', 'Live', $page1->ID, false) )); $key = \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX . '_' . md5($key); - $cache->save(time() - 2, $key); + $cache->save(time() + 10, $key); $response = $this->get($page1->RelativeLink('changes')); $this->assertEquals(429, $response->getStatusCode()); $this->assertGreaterThan(0, $response->getHeader('Retry-After')); @@ -92,14 +93,14 @@ class VersionFeedFunctionalTest extends FunctionalTest { // Test rate limit hit by IP Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_byuserip', true); $_SERVER['HTTP_CLIENT_IP'] = '127.0.0.1'; - $cache->save(time() - 2, \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX . '_' . md5('127.0.0.1')); + $cache->save(time() + 10, \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX . '_' . md5('127.0.0.1')); $response = $this->get($page1->RelativeLink('changes')); $this->assertEquals(429, $response->getStatusCode()); $this->assertGreaterThan(0, $response->getHeader('Retry-After')); // Test rate limit doesn't hit other IP $_SERVER['HTTP_CLIENT_IP'] = '127.0.0.20'; - $cache->save(time() - 2, \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX . '_' . md5('127.0.0.1')); + $cache->save(time() + 10, \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX . '_' . md5('127.0.0.1')); $response = $this->get($page1->RelativeLink('changes')); $this->assertEquals(200, $response->getStatusCode());