feature #38859 [HttpFoundation] Deprecate not passing a Closure together with FILTER_CALLBACK to ParameterBag::filter() (nicolas-grekas)

This PR was merged into the 5.2-dev branch.

Discussion
----------

[HttpFoundation] Deprecate not passing a `Closure` together with `FILTER_CALLBACK` to `ParameterBag::filter()`

| Q             | A
| ------------- | ---
| Branch?       | 5.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Tickets       | -
| License       | MIT
| Doc PR        | -

Using `filter_var()` with a configurable flag is risky, because of the `FILTER_CALLBACK` flag.
Restricting the type of callable that is accepted here mitigates the risk.
We did the same in Twig: https://github.com/twigphp/Twig/pull/3308

Commits
-------

d6aea288e7 [HttpFoundation] Deprecate not passing a `Closure` together with `FILTER_CALLBACK` to `ParameterBag::filter()`

UPGRADE-5.2.md

@@ -39,6 +39,11 @@ Form
  $builder->setDataMapper(new DataMapper(new PropertyPathAccessor()));
  ```
 
+HttpFoundation
+--------------
+
+ * Deprecated not passing a `Closure` together with `FILTER_CALLBACK` to `ParameterBag::filter()`; wrap your filter in a closure instead.
+
 Lock
 ----
 

UPGRADE-6.0.md

@@ -66,6 +66,7 @@ HttpFoundation
  * Removed `Response::create()`, `JsonResponse::create()`,
    `RedirectResponse::create()`, and `StreamedResponse::create()` methods (use
    `__construct()` instead)
+ * Not passing a `Closure` together with `FILTER_CALLBACK` to `ParameterBag::filter()` throws an `InvalidArgumentException`; wrap your filter in a closure instead.
 
 HttpKernel
 ----------

src/Symfony/Component/HttpFoundation/CHANGELOG.md

@@ -10,6 +10,7 @@ CHANGELOG
  * added ability to use comma separated ip addresses for `RequestMatcher::matchIps()`
  * added `Request::toArray()` to parse a JSON request body to an array
  * added `RateLimiter\RequestRateLimiterInterface` and `RateLimiter\AbstractRequestRateLimiter`
+ * deprecated not passing a `Closure` together with `FILTER_CALLBACK` to `ParameterBag::filter()`; wrap your filter in a closure instead.
 
 5.1.0
 -----

src/Symfony/Component/HttpFoundation/InputBag.php

@@ -103,6 +103,11 @@ final class InputBag extends ParameterBag
             }
         }
 
+        if ((\FILTER_CALLBACK & $filter) && !(($options['options'] ?? null) instanceof \Closure)) {
+            trigger_deprecation('symfony/http-foundation', '5.2', 'Not passing a Closure together with FILTER_CALLBACK to "%s()" is deprecated. Wrap your filter in a closure instead.', __METHOD__);
+            // throw new \InvalidArgumentException(sprintf('A Closure must be passed to "%s()" when FILTER_CALLBACK is used, "%s" given.', __METHOD__, get_debug_type($options['options'] ?? null)));
+        }
+
         return filter_var($value, $filter, $options);
     }
 }

src/Symfony/Component/HttpFoundation/ParameterBag.php

@@ -194,6 +194,11 @@ class ParameterBag implements \IteratorAggregate, \Countable
             $options['flags'] = \FILTER_REQUIRE_ARRAY;
         }
 
+        if ((\FILTER_CALLBACK & $filter) && !(($options['options'] ?? null) instanceof \Closure)) {
+            trigger_deprecation('symfony/http-foundation', '5.2', 'Not passing a Closure together with FILTER_CALLBACK to "%s()" is deprecated. Wrap your filter in a closure instead.', __METHOD__);
+            // throw new \InvalidArgumentException(sprintf('A Closure must be passed to "%s()" when FILTER_CALLBACK is used, "%s" given.', __METHOD__, get_debug_type($options['options'] ?? null)));
+        }
+
         return filter_var($value, $filter, $options);
     }
 

src/Symfony/Component/HttpFoundation/Tests/InputBagTest.php

@@ -45,6 +45,17 @@ class InputBagTest extends TestCase
         $this->assertSame([12, 8], $result);
     }
 
+    /**
+     * @group legacy
+     */
+    public function testFilterCallback()
+    {
+        $bag = new InputBag(['foo' => 'bar']);
+
+        $this->expectDeprecation('Since symfony/http-foundation 5.2: Not passing a Closure together with FILTER_CALLBACK to "Symfony\Component\HttpFoundation\InputBag::filter()" is deprecated. Wrap your filter in a closure instead.');
+        $this->assertSame('BAR', $bag->filter('foo', null, \FILTER_CALLBACK, ['options' => 'strtoupper']));
+    }
+
     /**
      * @group legacy
      */

src/Symfony/Component/HttpFoundation/Tests/ParameterBagTest.php

@@ -12,11 +12,14 @@
 namespace Symfony\Component\HttpFoundation\Tests;
 
 use PHPUnit\Framework\TestCase;
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 use Symfony\Component\HttpFoundation\Exception\BadRequestException;
 use Symfony\Component\HttpFoundation\ParameterBag;
 
 class ParameterBagTest extends TestCase
 {
+    use ExpectDeprecationTrait;
+
     public function testConstructor()
     {
         $this->testAll();
@@ -176,6 +179,17 @@ class ParameterBagTest extends TestCase
         $this->assertEquals(['bang'], $bag->filter('array', ''), '->filter() gets a value of parameter as an array');
     }
 
+    /**
+     * @group legacy
+     */
+    public function testFilterCallback()
+    {
+        $bag = new ParameterBag(['foo' => 'bar']);
+
+        $this->expectDeprecation('Since symfony/http-foundation 5.2: Not passing a Closure together with FILTER_CALLBACK to "Symfony\Component\HttpFoundation\ParameterBag::filter()" is deprecated. Wrap your filter in a closure instead.');
+        $this->assertSame('BAR', $bag->filter('foo', null, \FILTER_CALLBACK, ['options' => 'strtoupper']));
+    }
+
     public function testGetIterator()
     {
         $parameters = ['foo' => 'bar', 'hello' => 'world'];