bug symfony#37265 [HttpFoundation] use InputBag for Request::$request only if data is coming from a form (nicolas-grekas)

This PR was merged into the 5.1 branch.

Discussion
----------

[HttpFoundation] use InputBag for Request::$request only if data is coming from a form

| Q             | A
| ------------- | ---
| Branch?       | 5.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix symfony#37100
| License       | MIT
| Doc PR        | -

Commits
-------

786ba10 [HttpFoundation] use InputBag for Request::$request only if data is coming from a form

Author: fabpot

src/Symfony/Component/HttpFoundation/InputBag.php

@@ -36,29 +36,18 @@ public function get(string $key, $default = null)
         $value = parent::get($key, $this);
 
         if (null !== $value && $this !== $value && !is_scalar($value) && !(\is_object($value) && method_exists($value, '__toString'))) {
-            trigger_deprecation('symfony/http-foundation', '5.1', 'Retrieving a non-string value from "%s()" is deprecated, and will throw a "%s" exception in Symfony 6.0, use "%s::all()" instead.', __METHOD__, BadRequestException::class, __CLASS__);
+            trigger_deprecation('symfony/http-foundation', '5.1', 'Retrieving a non-string value from "%s()" is deprecated, and will throw a "%s" exception in Symfony 6.0, use "%s::all($key)" instead.', __METHOD__, BadRequestException::class, __CLASS__);
         }
 
         return $this === $value ? $default : $value;
     }
 
     /**
-     * Returns the inputs.
-     *
-     * @param string|null $key The name of the input to return or null to get them all
+     * {@inheritdoc}
      */
     public function all(string $key = null): array
     {
-        if (null === $key) {
-            return $this->parameters;
-        }
-
-        $value = $this->parameters[$key] ?? [];
-        if (!\is_array($value)) {
-            throw new BadRequestException(sprintf('Unexpected value for "%s" input, expecting "array", got "%s".', $key, get_debug_type($value)));
-        }
-
-        return $value;
+        return parent::all($key);
     }
 
     /**

src/Symfony/Component/HttpFoundation/ParameterBag.php

@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\HttpFoundation;
 
+use Symfony\Component\HttpFoundation\Exception\BadRequestException;
+
 /**
  * ParameterBag is a container for key/value pairs.
  *
@@ -31,11 +33,23 @@ public function __construct(array $parameters = [])
     /**
      * Returns the parameters.
      *
+     * @param string|null $key The name of the parameter to return or null to get them all
+     *
      * @return array An array of parameters
      */
-    public function all()
+    public function all(/*string $key = null*/)
     {
-        return $this->parameters;
+        $key = \func_num_args() > 0 ? func_get_arg(0) : null;
+
+        if (null === $key) {
+            return $this->parameters;
+        }
+
+        if (!\is_array($value = $this->parameters[$key] ?? [])) {
+            throw new BadRequestException(sprintf('Unexpected value for parameter "%s": expecting "array", got "%s".', $key, get_debug_type($value)));
+        }
+
+        return $value;
     }
 
     /**

src/Symfony/Component/HttpFoundation/Request.php

@@ -85,7 +85,7 @@ class Request
     /**
      * Request body parameters ($_POST).
      *
-     * @var InputBag
+     * @var InputBag|ParameterBag
      */
     public $request;
 
@@ -268,7 +268,7 @@ public function __construct(array $query = [], array $request = [], array $attri
      */
     public function initialize(array $query = [], array $request = [], array $attributes = [], array $cookies = [], array $files = [], array $server = [], $content = null)
     {
-        $this->request = new InputBag($request);
+        $this->request = new ParameterBag($request);
         $this->query = new InputBag($query);
         $this->attributes = new ParameterBag($attributes);
         $this->cookies = new InputBag($cookies);
@@ -298,7 +298,9 @@ public static function createFromGlobals()
     {
         $request = self::createRequestFromFactory($_GET, $_POST, [], $_COOKIE, $_FILES, $_SERVER);
 
-        if (0 === strpos($request->headers->get('CONTENT_TYPE'), 'application/x-www-form-urlencoded')
+        if ($_POST) {
+            $request->request = new InputBag($_POST);
+        } elseif (0 === strpos($request->headers->get('CONTENT_TYPE'), 'application/x-www-form-urlencoded')
             && \in_array(strtoupper($request->server->get('REQUEST_METHOD', 'GET')), ['PUT', 'DELETE', 'PATCH'])
         ) {
             parse_str($request->getContent(), $data);
@@ -447,7 +449,7 @@ public function duplicate(array $query = null, array $request = null, array $att
             $dup->query = new InputBag($query);
         }
         if (null !== $request) {
-            $dup->request = new InputBag($request);
+            $dup->request = new ParameterBag($request);
         }
         if (null !== $attributes) {
             $dup->attributes = new ParameterBag($attributes);

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

@@ -13,7 +13,6 @@
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
-use Symfony\Component\HttpFoundation\Exception\BadRequestException;
 use Symfony\Component\HttpFoundation\InputBag;
 
 class InputBagTest extends TestCase
@@ -36,21 +35,6 @@ public function testGetDoesNotUseDeepByDefault()
         $this->assertNull($bag->get('foo[bar]'));
     }
 
-    public function testAllWithInputKey()
-    {
-        $bag = new InputBag(['foo' => ['bar', 'baz'], 'null' => null]);
-
-        $this->assertEquals(['bar', 'baz'], $bag->all('foo'), '->all() gets the value of a parameter');
-        $this->assertEquals([], $bag->all('unknown'), '->all() returns an empty array if a parameter is not defined');
-    }
-
-    public function testAllThrowsForNonArrayValues()
-    {
-        $this->expectException(BadRequestException::class);
-        $bag = new InputBag(['foo' => 'bar', 'null' => null]);
-        $bag->all('foo');
-    }
-
     public function testFilterArray()
     {
         $bag = new InputBag([
@@ -77,7 +61,7 @@ public function testSetWithNonStringishOrArrayIsDeprecated()
     public function testGettingANonStringValueIsDeprecated()
     {
         $bag = new InputBag(['foo' => ['a', 'b']]);
-        $this->expectDeprecation('Since symfony/http-foundation 5.1: Retrieving a non-string value from "Symfony\Component\HttpFoundation\InputBag::get()" is deprecated, and will throw a "Symfony\Component\HttpFoundation\Exception\BadRequestException" exception in Symfony 6.0, use "Symfony\Component\HttpFoundation\InputBag::all()" instead.');
+        $this->expectDeprecation('Since symfony/http-foundation 5.1: Retrieving a non-string value from "Symfony\Component\HttpFoundation\InputBag::get()" is deprecated, and will throw a "Symfony\Component\HttpFoundation\Exception\BadRequestException" exception in Symfony 6.0, use "Symfony\Component\HttpFoundation\InputBag::all($key)" instead.');
         $bag->get('foo');
     }
 

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

@@ -12,6 +12,7 @@
 namespace Symfony\Component\HttpFoundation\Tests;
 
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Exception\BadRequestException;
 use Symfony\Component\HttpFoundation\ParameterBag;
 
 class ParameterBagTest extends TestCase
@@ -27,6 +28,21 @@ public function testAll()
         $this->assertEquals(['foo' => 'bar'], $bag->all(), '->all() gets all the input');
     }
 
+    public function testAllWithInputKey()
+    {
+        $bag = new ParameterBag(['foo' => ['bar', 'baz'], 'null' => null]);
+
+        $this->assertEquals(['bar', 'baz'], $bag->all('foo'), '->all() gets the value of a parameter');
+        $this->assertEquals([], $bag->all('unknown'), '->all() returns an empty array if a parameter is not defined');
+    }
+
+    public function testAllThrowsForNonArrayValues()
+    {
+        $this->expectException(BadRequestException::class);
+        $bag = new ParameterBag(['foo' => 'bar', 'null' => null]);
+        $bag->all('foo');
+    }
+
     public function testKeys()
     {
         $bag = new ParameterBag(['foo' => 'bar']);

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

@@ -13,6 +13,8 @@
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException;
+use Symfony\Component\HttpFoundation\InputBag;
+use Symfony\Component\HttpFoundation\ParameterBag;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Session\Session;
 use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
@@ -1255,6 +1257,11 @@ public function testCreateFromGlobals($method)
     {
         $normalizedMethod = strtoupper($method);
 
+        $_POST = [];
+        $request = Request::createFromGlobals();
+        $this->assertNotInstanceOf(InputBag::class, $request->request);
+        $this->assertInstanceOf(ParameterBag::class, $request->request);
+
         $_GET['foo1'] = 'bar1';
         $_POST['foo2'] = 'bar2';
         $_COOKIE['foo3'] = 'bar3';
@@ -1267,6 +1274,8 @@ public function testCreateFromGlobals($method)
         $this->assertEquals('bar3', $request->cookies->get('foo3'), '::fromGlobals() uses values from $_COOKIE');
         $this->assertEquals(['bar4'], $request->files->get('foo4'), '::fromGlobals() uses values from $_FILES');
         $this->assertEquals('bar5', $request->server->get('foo5'), '::fromGlobals() uses values from $_SERVER');
+        $this->assertInstanceOf(InputBag::class, $request->request);
+        $this->assertInstanceOf(ParameterBag::class, $request->request);
 
         unset($_GET['foo1'], $_POST['foo2'], $_COOKIE['foo3'], $_FILES['foo4'], $_SERVER['foo5']);
 
@@ -1275,6 +1284,8 @@ public function testCreateFromGlobals($method)
         $request = RequestContentProxy::createFromGlobals();
         $this->assertEquals($normalizedMethod, $request->getMethod());
         $this->assertEquals('mycontent', $request->request->get('content'));
+        $this->assertInstanceOf(InputBag::class, $request->request);
+        $this->assertInstanceOf(ParameterBag::class, $request->request);
 
         unset($_SERVER['REQUEST_METHOD'], $_SERVER['CONTENT_TYPE']);