[5.5] Make the requests throttler less aggressive

src/Illuminate/Cache/RateLimiter.php

@@ -37,35 +37,19 @@ public function __construct(Cache $cache)
      */
     public function tooManyAttempts($key, $maxAttempts, $decayMinutes = 1)
     {
-        if ($this->cache->has($key.':lockout')) {
-            return true;
-        }
-
         if ($this->attempts($key) >= $maxAttempts) {
-            $this->lockout($key, $decayMinutes);
+            if ($this->cache->has($key.':timer')) {
+                return true;
+            } else {
+                $this->resetAttempts($key);
 
-            $this->resetAttempts($key);
-
-            return true;
+                return false;
+            }
         }
 
         return false;
     }
 
-    /**
-     * Add the lockout key to the cache.
-     *
-     * @param  string  $key
-     * @param  int  $decayMinutes
-     * @return void
-     */
-    protected function lockout($key, $decayMinutes)
-    {
-        $this->cache->add(
-            $key.':lockout', $this->availableAt($decayMinutes * 60), $decayMinutes
-        );
-    }
-
     /**
      * Increment the counter for a given key for a given decay time.
      *
@@ -75,6 +59,10 @@ protected function lockout($key, $decayMinutes)
      */
     public function hit($key, $decayMinutes = 1)
     {
+        $this->cache->add($key.':timer', $this->availableAt($decayMinutes * 60),
+            $decayMinutes
+        );
+
         $added = $this->cache->add($key, 0, $decayMinutes);
 
         $hits = (int) $this->cache->increment($key);
@@ -123,7 +111,7 @@ public function retriesLeft($key, $maxAttempts)
     }
 
     /**
-     * Clear the hits and lockout for the given key.
+     * Clear the hits and lockout timer for the given key.
      *
      * @param  string  $key
      * @return void
@@ -132,7 +120,7 @@ public function clear($key)
     {
         $this->resetAttempts($key);
 
-        $this->cache->forget($key.':lockout');
+        $this->cache->forget($key.':timer');
     }
 
     /**
@@ -143,6 +131,6 @@ public function clear($key)
      */
     public function availableIn($key)
     {
-        return $this->cache->get($key.':lockout') - $this->currentTime();
+        return $this->cache->get($key.':timer') - $this->currentTime();
     }
 }

tests/Cache/CacheRateLimiterTest.php

@@ -17,28 +17,18 @@ public function tearDown()
     public function testTooManyAttemptsReturnTrueIfAlreadyLockedOut()
     {
         $cache = m::mock(Cache::class);
-        $cache->shouldReceive('has')->once()->with('key:lockout')->andReturn(true);
+        $cache->shouldReceive('get')->once()->with('key', 0)->andReturn(1);
+        $cache->shouldReceive('has')->once()->with('key:timer')->andReturn(true);
         $cache->shouldReceive('add')->never();
         $rateLimiter = new RateLimiter($cache);
 
         $this->assertTrue($rateLimiter->tooManyAttempts('key', 1, 1));
     }
 
-    public function testTooManyAttemptsReturnsTrueIfMaxAttemptsExceeded()
-    {
-        $cache = m::mock(Cache::class);
-        $cache->shouldReceive('get')->once()->with('key', 0)->andReturn(10);
-        $cache->shouldReceive('has')->once()->with('key:lockout')->andReturn(false);
-        $cache->shouldReceive('add')->once()->with('key:lockout', m::type('int'), 1);
-        $cache->shouldReceive('forget')->once()->with('key');
-        $rateLimiter = new RateLimiter($cache);
-
-        $this->assertTrue($rateLimiter->tooManyAttempts('key', 1, 1));
-    }
-
     public function testHitProperlyIncrementsAttemptCount()
     {
         $cache = m::mock(Cache::class);
+        $cache->shouldReceive('add')->once()->with('key:timer', m::type('int'), 1)->andReturn(true);
         $cache->shouldReceive('add')->once()->with('key', 0, 1)->andReturn(true);
         $cache->shouldReceive('increment')->once()->with('key')->andReturn(1);
         $rateLimiter = new RateLimiter($cache);
@@ -49,6 +39,7 @@ public function testHitProperlyIncrementsAttemptCount()
     public function testHitHasNoMemoryLeak()
     {
         $cache = m::mock(Cache::class);
+        $cache->shouldReceive('add')->once()->with('key:timer', m::type('int'), 1)->andReturn(true);
         $cache->shouldReceive('add')->once()->with('key', 0, 1)->andReturn(false);
         $cache->shouldReceive('increment')->once()->with('key')->andReturn(1);
         $cache->shouldReceive('put')->once()->with('key', 1, 1);
@@ -70,7 +61,7 @@ public function testClearClearsTheCacheKeys()
     {
         $cache = m::mock(Cache::class);
         $cache->shouldReceive('forget')->once()->with('key');
-        $cache->shouldReceive('forget')->once()->with('key:lockout');
+        $cache->shouldReceive('forget')->once()->with('key:timer');
         $rateLimiter = new RateLimiter($cache);
 
         $rateLimiter->clear('key');

tests/Integration/Http/ThrottleRequestsTest.php

@@ -0,0 +1,57 @@
+<?php
+
+namespace Illuminate\Tests\Integration\Http;
+
+use Illuminate\Support\Carbon;
+use Orchestra\Testbench\TestCase;
+use Illuminate\Support\Facades\Route;
+use Illuminate\Routing\Middleware\ThrottleRequests;
+
+/**
+ * @group integration
+ */
+class ThrottleRequestsTest extends TestCase
+{
+    protected function getEnvironmentSetUp($app)
+    {
+        $app['config']->set('cache.default', 'redis');
+    }
+
+    public function setup()
+    {
+        parent::setup();
+
+        resolve('redis')->flushall();
+    }
+
+    public function test_lock_opens_immediately_after_decay()
+    {
+        Route::get('/', function () {
+            return 'yes';
+        })->middleware(ThrottleRequests::class.':2,1');
+
+        $response = $this->withoutExceptionHandling()->get('/');
+        $this->assertEquals('yes', $response->getContent());
+        $this->assertEquals(2, $response->headers->get('X-RateLimit-Limit'));
+        $this->assertEquals(1, $response->headers->get('X-RateLimit-Remaining'));
+
+        $response = $this->withoutExceptionHandling()->get('/');
+        $this->assertEquals('yes', $response->getContent());
+        $this->assertEquals(2, $response->headers->get('X-RateLimit-Limit'));
+        $this->assertEquals(0, $response->headers->get('X-RateLimit-Remaining'));
+
+        Carbon::setTestNow(
+            Carbon::now()->addSeconds(58)
+        );
+
+        try {
+            $response = $this->withoutExceptionHandling()->get('/');
+        } catch (\Throwable $e) {
+            $this->assertEquals(429, $e->getStatusCode());
+            $this->assertEquals(2, $e->getHeaders()['X-RateLimit-Limit']);
+            $this->assertEquals(0, $e->getHeaders()['X-RateLimit-Remaining']);
+            $this->assertEquals(2, $e->getHeaders()['Retry-After']);
+            $this->assertEquals(now()->timestamp + 2, $e->getHeaders()['X-RateLimit-Reset']);
+        }
+    }
+}