[RateLimiter] Be more type safe when fetching form cache

Nyholm · Nyholm · commit 4795756cc7e2 · 2020-10-21T23:51:48.000+02:00
diff --git a/src/Symfony/Component/RateLimiter/Storage/CacheStorage.php b/src/Symfony/Component/RateLimiter/Storage/CacheStorage.php
@@ -42,11 +42,12 @@ public function save(LimiterStateInterface $limiterState): void
     public function fetch(string $limiterStateId): ?LimiterStateInterface
     {
         $cacheItem = $this->pool->getItem(sha1($limiterStateId));
-        if (!$cacheItem->isHit()) {
-            return null;
+        $value = $cacheItem->get();
+        if ($value instanceof LimiterStateInterface) {
+            return $value;
         }
 
-        return $cacheItem->get();
+        return null;
     }
 
     public function delete(string $limiterStateId): void
diff --git a/src/Symfony/Component/RateLimiter/Tests/Storage/CacheStorageTest.php b/src/Symfony/Component/RateLimiter/Tests/Storage/CacheStorageTest.php
@@ -55,6 +55,18 @@ public function testFetchExistingState()
         $this->assertEquals($window, $this->storage->fetch('test'));
     }
 
+    public function testFetchExistingJunk()
+    {
+        $cacheItem = $this->createMock(CacheItemInterface::class);
+
+        $cacheItem->expects($this->any())->method('get')->willReturn('junk');
+        $cacheItem->expects($this->any())->method('isHit')->willReturn(true);
+
+        $this->pool->expects($this->any())->method('getItem')->with(sha1('test'))->willReturn($cacheItem);
+
+        $this->assertNull($this->storage->fetch('test'));
+    }
+
     public function testFetchNonExistingState()
     {
         $cacheItem = $this->createMock(CacheItemInterface::class);

Original file line number	Diff line number	Diff line change
`@@ -42,11 +42,12 @@ public function save(LimiterStateInterface $limiterState): void`
`42`	`42`	`public function fetch(string $limiterStateId): ?LimiterStateInterface`
`43`	`43`	`{`
`44`	`44`	`$cacheItem = $this->pool->getItem(sha1($limiterStateId));`
`45`		`- if (!$cacheItem->isHit()) {`
`46`		`- return null;`
	`45`	`+ $value = $cacheItem->get();`
	`46`	`+ if ($value instanceof LimiterStateInterface) {`
	`47`	`+ return $value;`
`47`	`48`	`}`
`48`	`49`
`49`		`- return $cacheItem->get();`
	`50`	`+ return null;`
`50`	`51`	`}`
`51`	`52`
`52`	`53`	`public function delete(string $limiterStateId): void`