bug #38367 [RateLimiter] Fix cache storage (use namespaced pool + remove \Serializable) (wouterj)

fabpot · fabpot · commit d0ded920e6bb · 2020-10-02T08:02:03.000+02:00
This PR was squashed before being merged into the 5.2-dev branch. Discussion ---------- [RateLimiter] Fix cache storage (use namespaced pool + remove \Serializable) | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #38365, fix #38338 | License | MIT | Doc PR | - Commits ------- 251c202 Use a dedicated cache.rate_limiter cache pool 5dc562a Use __sleep/__wakeup instead of Serializable
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
@@ -1808,7 +1808,7 @@ private function addRateLimiterSection(ArrayNodeDefinition $rootNode)
                                     ->end()
                                     ->scalarNode('cache_pool')
                                         ->info('The cache pool to use for storing the current limiter state')
-                                        ->defaultValue('cache.app')
+                                        ->defaultValue('cache.rate_limiter')
                                     ->end()
                                     ->scalarNode('storage_service')
                                         ->info('The service ID of a custom storage implementation, this precedes any configured "cache_pool"')
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -2254,6 +2254,9 @@ private function registerRateLimiterConfiguration(array $config, ContainerBuilde
 
     public static function registerRateLimiter(ContainerBuilder $container, string $name, array $limiterConfig)
     {
+        // default configuration (when used by other DI extensions)
+        $limiterConfig += ['lock_factory' => 'lock.factory', 'cache_pool' => 'cache.rate_limiter'];
+
         $limiter = $container->setDefinition($limiterId = 'limiter.'.$name, new ChildDefinition('limiter'));
 
         $limiter->addArgument(new Reference($limiterConfig['lock_factory']));
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/rate_limiter.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/rate_limiter.php
@@ -15,6 +15,10 @@
 
 return static function (ContainerConfigurator $container) {
     $container->services()
+        ->set('cache.rate_limiter')
+            ->parent('cache.app')
+            ->tag('cache.pool')
+
         ->set('limiter', Limiter::class)
             ->abstract()
             ->args([
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php
@@ -76,8 +76,6 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal
                 'strategy' => 'fixed_window',
                 'limit' => $config['max_attempts'],
                 'interval' => '1 minute',
-                'lock_factory' => 'lock.factory',
-                'cache_pool' => 'cache.app',
             ];
             FrameworkExtension::registerRateLimiter($container, $localId = '_login_local_'.$firewallName, $limiterOptions);
 
diff --git a/src/Symfony/Component/RateLimiter/LimiterStateInterface.php b/src/Symfony/Component/RateLimiter/LimiterStateInterface.php
@@ -12,11 +12,17 @@
 namespace Symfony\Component\RateLimiter;
 
 /**
+ * Representing the stored state of the limiter.
+ *
+ * Classes implementing this interface must be serializable,
+ * which is used by the storage implementations to store the
+ * object.
+ *
  * @author Wouter de Jong <wouter@wouterj.nl>
  *
  * @experimental in 5.2
  */
-interface LimiterStateInterface extends \Serializable
+interface LimiterStateInterface
 {
     public function getId(): string;
 
diff --git a/src/Symfony/Component/RateLimiter/TokenBucket.php b/src/Symfony/Component/RateLimiter/TokenBucket.php
@@ -70,15 +70,22 @@ public function getExpirationTime(): int
         return $this->rate->calculateTimeForTokens($this->burstSize);
     }
 
-    public function serialize(): string
+    /**
+     * @internal
+     */
+    public function __sleep(): array
     {
-        return serialize([$this->id, $this->tokens, $this->timer, $this->burstSize, (string) $this->rate]);
+        $this->stringRate = (string) $this->rate;
+
+        return ['id', 'tokens', 'timer', 'burstSize', 'stringRate'];
     }
 
-    public function unserialize($serialized): void
+    /**
+     * @internal
+     */
+    public function __wakeup(): void
     {
-        [$this->id, $this->tokens, $this->timer, $this->burstSize, $rate] = unserialize($serialized);
-
-        $this->rate = Rate::fromString($rate);
+        $this->rate = Rate::fromString($this->stringRate);
+        unset($this->stringRate);
     }
 }
diff --git a/src/Symfony/Component/RateLimiter/Window.php b/src/Symfony/Component/RateLimiter/Window.php
@@ -48,15 +48,13 @@ public function getHitCount(): int
         return $this->hitCount;
     }
 
-    public function serialize(): string
+    /**
+     * @internal
+     */
+    public function __sleep(): array
     {
         // $intervalInSeconds is not serialized, it should only be set
         // upon first creation of the Window.
-        return serialize([$this->id, $this->hitCount]);
-    }
-
-    public function unserialize($serialized): void
-    {
-        [$this->id, $this->hitCount] = unserialize($serialized);
+        return ['id', 'hitCount'];
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -2254,6 +2254,9 @@ private function registerRateLimiterConfiguration(array $config, ContainerBuilde`
`2254`	`2254`
`2255`	`2255`	`public static function registerRateLimiter(ContainerBuilder $container, string $name, array $limiterConfig)`
`2256`	`2256`	`{`
	`2257`	`+ // default configuration (when used by other DI extensions)`
	`2258`	`+ $limiterConfig += ['lock_factory' => 'lock.factory', 'cache_pool' => 'cache.rate_limiter'];`
	`2259`	`+`
`2257`	`2260`	`$limiter = $container->setDefinition($limiterId = 'limiter.'.$name, new ChildDefinition('limiter'));`
`2258`	`2261`
`2259`	`2262`	`$limiter->addArgument(new Reference($limiterConfig['lock_factory']));`
Original file line number	Diff line number	Diff line change
`@@ -70,15 +70,22 @@ public function getExpirationTime(): int`
`70`	`70`	`return $this->rate->calculateTimeForTokens($this->burstSize);`
`71`	`71`	`}`
`72`	`72`
`73`		`- public function serialize(): string`
	`73`	`+ /**`
	`74`	`+ * @internal`
	`75`	`+ */`
	`76`	`+ public function __sleep(): array`
`74`	`77`	`{`
`75`		`- return serialize([$this->id, $this->tokens, $this->timer, $this->burstSize, (string) $this->rate]);`
	`78`	`+ $this->stringRate = (string) $this->rate;`
	`79`	`+`
	`80`	`+ return ['id', 'tokens', 'timer', 'burstSize', 'stringRate'];`
`76`	`81`	`}`
`77`	`82`
`78`		`- public function unserialize($serialized): void`
	`83`	`+ /**`
	`84`	`+ * @internal`
	`85`	`+ */`
	`86`	`+ public function __wakeup(): void`
`79`	`87`	`{`
`80`		`- [$this->id, $this->tokens, $this->timer, $this->burstSize, $rate] = unserialize($serialized);`
`81`		`-`
`82`		`- $this->rate = Rate::fromString($rate);`
	`88`	`+ $this->rate = Rate::fromString($this->stringRate);`
	`89`	`+ unset($this->stringRate);`
`83`	`90`	`}`
`84`	`91`	`}`
Original file line number	Diff line number	Diff line change
`@@ -48,15 +48,13 @@ public function getHitCount(): int`
`48`	`48`	`return $this->hitCount;`
`49`	`49`	`}`
`50`	`50`
`51`		`- public function serialize(): string`
	`51`	`+ /**`
	`52`	`+ * @internal`
	`53`	`+ */`
	`54`	`+ public function __sleep(): array`
`52`	`55`	`{`
`53`	`56`	`// $intervalInSeconds is not serialized, it should only be set`
`54`	`57`	`// upon first creation of the Window.`
`55`		`- return serialize([$this->id, $this->hitCount]);`
`56`		`- }`
`57`		`-`
`58`		`- public function unserialize($serialized): void`
`59`		`- {`
`60`		`- [$this->id, $this->hitCount] = unserialize($serialized);`
	`58`	`+ return ['id', 'hitCount'];`
`61`	`59`	`}`
`62`	`60`	`}`