[10.x] Fix RateLimiter callback return substitution (#44820) (#45611)

* Fix RateLimiter callback return substitution (#44820) This breaking update allows users to control return values explicitly without converting to bool. * Update RateLimiter.php Co-authored-by: Taylor Otwell <[email protected]>
laravel · Jan 12, 2023 · 7a57619 · 7a57619
1 parent bef6396
commit 7a57619
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 7 deletions.
diff --git a/src/Illuminate/Cache/RateLimiter.php b/src/Illuminate/Cache/RateLimiter.php
@@ -74,8 +74,12 @@ public function attempt($key, $maxAttempts, Closure $callback, $decaySeconds = 6
         if ($this->tooManyAttempts($key, $maxAttempts)) {
             return false;
         }
+
+        if (is_null($result = $callback())) {
+            $result = true;
+        }
 
-        return tap($callback() ?: true, function () use ($key, $decaySeconds) {
+        return tap($result, function () use ($key, $decaySeconds) {
             $this->hit($key, $decaySeconds);
         });
     }

diff --git a/tests/Cache/CacheRateLimiterTest.php b/tests/Cache/CacheRateLimiterTest.php
@@ -89,25 +89,45 @@ public function testAttemptsCallbackReturnsTrue()
 
         $rateLimiter = new RateLimiter($cache);
 
-        $this->assertTrue($rateLimiter->attempt('key', 1, function () use (&$executed) {
+        $rateLimiter->attempt('key', 1, function () use (&$executed) {
             $executed = true;
-        }, 1));
+        }, 1);
         $this->assertTrue($executed);
     }
 
     public function testAttemptsCallbackReturnsCallbackReturn()
     {
         $cache = m::mock(Cache::class);
-        $cache->shouldReceive('get')->once()->with('key', 0)->andReturn(0);
-        $cache->shouldReceive('add')->once()->with('key:timer', m::type('int'), 1);
-        $cache->shouldReceive('add')->once()->with('key', 0, 1)->andReturns(1);
-        $cache->shouldReceive('increment')->once()->with('key')->andReturn(1);
+        $cache->shouldReceive('get')->times(6)->with('key', 0)->andReturn(0);
+        $cache->shouldReceive('add')->times(6)->with('key:timer', m::type('int'), 1);
+        $cache->shouldReceive('add')->times(6)->with('key', 0, 1)->andReturns(1);
+        $cache->shouldReceive('increment')->times(6)->with('key')->andReturn(1);
 
         $rateLimiter = new RateLimiter($cache);
 
         $this->assertSame('foo', $rateLimiter->attempt('key', 1, function () {
             return 'foo';
         }, 1));
+
+        $this->assertSame(false, $rateLimiter->attempt('key', 1, function () {
+            return false;
+        }, 1));
+
+        $this->assertSame([], $rateLimiter->attempt('key', 1, function () {
+            return [];
+        }, 1));
+
+        $this->assertSame(0, $rateLimiter->attempt('key', 1, function () {
+            return 0;
+        }, 1));
+
+        $this->assertSame(0.0, $rateLimiter->attempt('key', 1, function () {
+            return 0.0;
+        }, 1));
+
+        $this->assertSame('', $rateLimiter->attempt('key', 1, function () {
+            return '';
+        }, 1));
     }
 
     public function testAttemptsCallbackReturnsFalse()