feature #16007 [HttpFoundation] deprecate finding deep items in request parameters (xabbuh)

This PR was merged into the 2.8 branch.

Discussion
----------

[HttpFoundation] deprecate finding deep items in request parameters

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

This is the same as #14203 but using the PropertyAccess component in the Security HTTP component to query nested request parameters and is rebased on the `2.8` branch.

Commits
-------

47fba88 deprecate finding deep items in request parameters

src/Symfony/Component/HttpFoundation/CHANGELOG.md

@@ -1,6 +1,12 @@
 CHANGELOG
 =========
 
+2.8.0
+-----
+
+ * Finding deep items in `ParameterBag::get()` is deprecated since version 2.8 and
+   will be removed in 3.0.
+
 2.6.0
 -----
 

src/Symfony/Component/HttpFoundation/ParameterBag.php

@@ -78,7 +78,9 @@ class ParameterBag implements \IteratorAggregate, \Countable
     /**
      * Returns a parameter by name.
      *
-     * @param string $path    The key
+     * Note: Finding deep items is deprecated since version 2.8, to be removed in 3.0.
+     *
+     * @param string $key     The key
      * @param mixed  $default The default value if the parameter key does not exist
      * @param bool   $deep    If true, a path like foo[bar] will find deeper items
      *
@@ -86,21 +88,25 @@ class ParameterBag implements \IteratorAggregate, \Countable
      *
      * @throws \InvalidArgumentException
      */
-    public function get($path, $default = null, $deep = false)
+    public function get($key, $default = null, $deep = false)
     {
-        if (!$deep || false === $pos = strpos($path, '[')) {
-            return array_key_exists($path, $this->parameters) ? $this->parameters[$path] : $default;
+        if (true === $deep) {
+            @trigger_error('Using paths to find deeper items in '.__METHOD__.' is deprecated since version 2.8 and will be removed in 3.0. Filter the returned value in your own code instead.', E_USER_DEPRECATED);
         }
 
-        $root = substr($path, 0, $pos);
+        if (!$deep || false === $pos = strpos($key, '[')) {
+            return array_key_exists($key, $this->parameters) ? $this->parameters[$key] : $default;
+        }
+
+        $root = substr($key, 0, $pos);
         if (!array_key_exists($root, $this->parameters)) {
             return $default;
         }
 
         $value = $this->parameters[$root];
         $currentKey = null;
-        for ($i = $pos, $c = strlen($path); $i < $c; ++$i) {
-            $char = $path[$i];
+        for ($i = $pos, $c = strlen($key); $i < $c; ++$i) {
+            $char = $key[$i];
 
             if ('[' === $char) {
                 if (null !== $currentKey) {

src/Symfony/Component/HttpFoundation/Request.php

@@ -714,6 +714,8 @@ class Request
      * It is better to explicitly get request parameters from the appropriate
      * public property instead (query, attributes, request).
      *
+     * Note: Finding deep items is deprecated since version 2.8, to be removed in 3.0.
+     *
      * @param string $key     the key
      * @param mixed  $default the default value
      * @param bool   $deep    is parameter deep in multidimensional array
@@ -722,6 +724,10 @@ class Request
      */
     public function get($key, $default = null, $deep = false)
     {
+        if (true === $deep) {
+            @trigger_error('Using paths to find deeper items in '.__METHOD__.' is deprecated since version 2.8 and will be removed in 3.0. Filter the returned value in your own code instead.', E_USER_DEPRECATED);
+        }
+
         if ($this !== $result = $this->query->get($key, $this, $deep)) {
             return $result;
         }

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

@@ -86,6 +86,7 @@ class ParameterBagTest extends \PHPUnit_Framework_TestCase
     }
 
     /**
+     * @group legacy
      * @dataProvider getInvalidPaths
      * @expectedException \InvalidArgumentException
      */
@@ -106,6 +107,9 @@ class ParameterBagTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    /**
+     * @group legacy
+     */
     public function testGetDeep()
     {
         $bag = new ParameterBag(array('foo' => array('bar' => array('moo' => 'boo'))));

src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationFailureHandler.php

@@ -17,6 +17,7 @@ use Psr\Log\LoggerInterface;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
 use Symfony\Component\Security\Core\Security;
 use Symfony\Component\Security\Http\HttpUtils;
+use Symfony\Component\Security\Http\ParameterBagUtils;
 
 /**
  * Class with the default authentication failure handling logic.
@@ -82,7 +83,7 @@ class DefaultAuthenticationFailureHandler implements AuthenticationFailureHandle
      */
     public function onAuthenticationFailure(Request $request, AuthenticationException $exception)
     {
-        if ($failureUrl = $request->get($this->options['failure_path_parameter'], null, true)) {
+        if ($failureUrl = ParameterBagUtils::getRequestParameterValue($request, $this->options['failure_path_parameter'])) {
             $this->options['failure_path'] = $failureUrl;
         }
 

src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationSuccessHandler.php

@@ -14,6 +14,7 @@ namespace Symfony\Component\Security\Http\Authentication;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Security\Http\HttpUtils;
+use Symfony\Component\Security\Http\ParameterBagUtils;
 
 /**
  * Class with the default authentication success handling logic.
@@ -108,7 +109,7 @@ class DefaultAuthenticationSuccessHandler implements AuthenticationSuccessHandle
             return $this->options['default_target_path'];
         }
 
-        if ($targetUrl = $request->get($this->options['target_path_parameter'], null, true)) {
+        if ($targetUrl = ParameterBagUtils::getRequestParameterValue($request, $this->options['target_path_parameter'])) {
             return $targetUrl;
         }
 

src/Symfony/Component/Security/Http/Firewall/LogoutListener.php

@@ -24,6 +24,7 @@ use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
 use Symfony\Component\Security\Http\HttpUtils;
 use Symfony\Component\Security\Http\Logout\LogoutHandlerInterface;
 use Symfony\Component\Security\Http\Logout\LogoutSuccessHandlerInterface;
+use Symfony\Component\Security\Http\ParameterBagUtils;
 
 /**
  * LogoutListener logout users.
@@ -98,7 +99,7 @@ class LogoutListener implements ListenerInterface
         }
 
         if (null !== $this->csrfTokenManager) {
-            $csrfToken = $request->get($this->options['csrf_parameter'], null, true);
+            $csrfToken = ParameterBagUtils::getRequestParameterValue($request, $this->options['csrf_parameter']);
 
             if (false === $this->csrfTokenManager->isTokenValid(new CsrfToken($this->options['intention'], $csrfToken))) {
                 throw new LogoutException('Invalid CSRF token.');

src/Symfony/Component/Security/Http/Firewall/SimpleFormAuthenticationListener.php

@@ -26,6 +26,7 @@ use Symfony\Component\Security\Core\Authentication\SimpleFormAuthenticatorInterf
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
 use Symfony\Component\Security\Core\Security;
 use Symfony\Component\Security\Http\HttpUtils;
+use Symfony\Component\Security\Http\ParameterBagUtils;
 use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface;
 use Psr\Log\LoggerInterface;
 
@@ -101,7 +102,7 @@ class SimpleFormAuthenticationListener extends AbstractAuthenticationListener
     protected function attemptAuthentication(Request $request)
     {
         if (null !== $this->csrfTokenManager) {
-            $csrfToken = $request->get($this->options['csrf_parameter'], null, true);
+            $csrfToken = ParameterBagUtils::getRequestParameterValue($request, $this->options['csrf_parameter']);
 
             if (false === $this->csrfTokenManager->isTokenValid(new CsrfToken($this->options['intention'], $csrfToken))) {
                 throw new InvalidCsrfTokenException('Invalid CSRF token.');
@@ -109,11 +110,11 @@ class SimpleFormAuthenticationListener extends AbstractAuthenticationListener
         }
 
         if ($this->options['post_only']) {
-            $username = trim($request->request->get($this->options['username_parameter'], null, true));
-            $password = $request->request->get($this->options['password_parameter'], null, true);
+            $username = trim(ParameterBagUtils::getParameterBagValue($request->request, $this->options['username_parameter']));
+            $password = ParameterBagUtils::getParameterBagValue($request->request, $this->options['password_parameter']);
         } else {
-            $username = trim($request->get($this->options['username_parameter'], null, true));
-            $password = $request->get($this->options['password_parameter'], null, true);
+            $username = trim(ParameterBagUtils::getRequestParameterValue($request, $this->options['username_parameter']));
+            $password = ParameterBagUtils::getRequestParameterValue($request, $this->options['password_parameter']);
         }
 
         $request->getSession()->set(Security::LAST_USERNAME, $username);

src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php

@@ -19,6 +19,7 @@ use Symfony\Component\Security\Csrf\CsrfToken;
 use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
 use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface;
 use Symfony\Component\Security\Http\Authentication\AuthenticationSuccessHandlerInterface;
+use Symfony\Component\Security\Http\ParameterBagUtils;
 use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface;
 use Symfony\Component\Security\Http\HttpUtils;
 use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
@@ -76,7 +77,7 @@ class UsernamePasswordFormAuthenticationListener extends AbstractAuthenticationL
     protected function attemptAuthentication(Request $request)
     {
         if (null !== $this->csrfTokenManager) {
-            $csrfToken = $request->get($this->options['csrf_parameter'], null, true);
+            $csrfToken = ParameterBagUtils::getRequestParameterValue($request, $this->options['csrf_parameter']);
 
             if (false === $this->csrfTokenManager->isTokenValid(new CsrfToken($this->options['intention'], $csrfToken))) {
                 throw new InvalidCsrfTokenException('Invalid CSRF token.');
@@ -84,11 +85,11 @@ class UsernamePasswordFormAuthenticationListener extends AbstractAuthenticationL
         }
 
         if ($this->options['post_only']) {
-            $username = trim($request->request->get($this->options['username_parameter'], null, true));
-            $password = $request->request->get($this->options['password_parameter'], null, true);
+            $username = trim(ParameterBagUtils::getParameterBagValue($request->request, $this->options['username_parameter']));
+            $password = ParameterBagUtils::getParameterBagValue($request->request, $this->options['password_parameter']);
         } else {
-            $username = trim($request->get($this->options['username_parameter'], null, true));
-            $password = $request->get($this->options['password_parameter'], null, true);
+            $username = trim(ParameterBagUtils::getRequestParameterValue($request, $this->options['username_parameter']));
+            $password = ParameterBagUtils::getRequestParameterValue($request, $this->options['password_parameter']);
         }
 
         $request->getSession()->set(Security::LAST_USERNAME, $username);

src/Symfony/Component/Security/Http/ParameterBagUtils.php

@@ -0,0 +1,96 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Http;
+
+use Symfony\Component\HttpFoundation\ParameterBag;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\PropertyAccess\Exception\AccessException;
+use Symfony\Component\PropertyAccess\Exception\InvalidArgumentException;
+use Symfony\Component\PropertyAccess\PropertyAccess;
+
+/**
+ * @internal
+ */
+final class ParameterBagUtils
+{
+    private static $propertyAccessor;
+
+    /**
+     * Returns a "parameter" value.
+     *
+     * Paths like foo[bar] will be evaluated to find deeper items in nested data structures.
+     *
+     * @param ParameterBag $parameters The parameter bag
+     * @param string       $path       The key
+     *
+     * @return mixed
+     *
+     * @throws InvalidArgumentException when the given path is malformed
+     */
+    public static function getParameterBagValue(ParameterBag $parameters, $path)
+    {
+        if (false === $pos = strpos($path, '[')) {
+            return $parameters->get($path);
+        }
+
+        $root = substr($path, 0, $pos);
+
+        if (null === $value = $parameters->get($root)) {
+            return;
+        }
+
+        if (null === self::$propertyAccessor) {
+            self::$propertyAccessor = PropertyAccess::createPropertyAccessor();
+        }
+
+        try {
+            return self::$propertyAccessor->getValue($value, substr($path, $pos));
+        } catch (AccessException $e) {
+            return;
+        }
+    }
+
+    /**
+     * Returns a request "parameter" value.
+     *
+     * Paths like foo[bar] will be evaluated to find deeper items in nested data structures.
+     *
+     * @param Request $request The request
+     * @param string  $path    The key
+     *
+     * @return mixed
+     *
+     * @throws InvalidArgumentException when the given path is malformed
+     */
+    public static function getRequestParameterValue(Request $request, $path)
+    {
+        if (false === $pos = strpos($path, '[')) {
+            return $request->get($path);
+        }
+
+        $root = substr($path, 0, $pos);
+
+        if (null === $value = $request->get($root)) {
+            return;
+        }
+
+        if (null === self::$propertyAccessor) {
+            self::$propertyAccessor = PropertyAccess::createPropertyAccessor();
+        }
+
+        try {
+            return self::$propertyAccessor->getValue($value, substr($path, $pos));
+        } catch (AccessException $e) {
+            return;
+        }
+    }
+}

src/Symfony/Component/Security/Http/RememberMe/AbstractRememberMeServices.php

@@ -23,6 +23,7 @@ use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Cookie;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\Security\Http\ParameterBagUtils;
 
 /**
  * Base class implementing the RememberMeServicesInterface.
@@ -319,7 +320,7 @@ abstract class AbstractRememberMeServices implements RememberMeServicesInterface
             return true;
         }
 
-        $parameter = $request->get($this->options['remember_me_parameter'], null, true);
+        $parameter = ParameterBagUtils::getRequestParameterValue($request, $this->options['remember_me_parameter']);
 
         if (null === $parameter && null !== $this->logger) {
             $this->logger->debug('Did not send remember-me cookie.', array('parameter' => $this->options['remember_me_parameter']));

src/Symfony/Component/Security/Http/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php

@@ -145,7 +145,7 @@ class DefaultAuthenticationFailureHandlerTest extends \PHPUnit_Framework_TestCas
     public function testFailurePathCanBeOverwrittenWithRequest()
     {
         $this->request->expects($this->once())
-            ->method('get')->with('_failure_path', null, true)
+            ->method('get')->with('_failure_path', null, false)
             ->will($this->returnValue('/auth/login'));
 
         $this->httpUtils->expects($this->once())
@@ -155,12 +155,25 @@ class DefaultAuthenticationFailureHandlerTest extends \PHPUnit_Framework_TestCas
         $handler->onAuthenticationFailure($this->request, $this->exception);
     }
 
+    public function testFailurePathCanBeOverwrittenWithNestedAttributeInRequest()
+    {
+        $this->request->expects($this->once())
+            ->method('get')->with('_failure_path', null, false)
+            ->will($this->returnValue(array('value' => '/auth/login')));
+
+        $this->httpUtils->expects($this->once())
+            ->method('createRedirectResponse')->with($this->request, '/auth/login');
+
+        $handler = new DefaultAuthenticationFailureHandler($this->httpKernel, $this->httpUtils, array('failure_path_parameter' => '_failure_path[value]'), $this->logger);
+        $handler->onAuthenticationFailure($this->request, $this->exception);
+    }
+
     public function testFailurePathParameterCanBeOverwritten()
     {
         $options = array('failure_path_parameter' => '_my_failure_path');
 
         $this->request->expects($this->once())
-            ->method('get')->with('_my_failure_path', null, true)
+            ->method('get')->with('_my_failure_path', null, false)
             ->will($this->returnValue('/auth/login'));
 
         $this->httpUtils->expects($this->once())

src/Symfony/Component/Security/Http/Tests/Authentication/DefaultAuthenticationSuccessHandlerTest.php

@@ -68,6 +68,20 @@ class DefaultAuthenticationSuccessHandlerTest extends \PHPUnit_Framework_TestCas
         $this->assertSame($response, $result);
     }
 
+    public function testTargetPathIsPassedAsNestedParameterWithRequest()
+    {
+        $this->request->expects($this->once())
+            ->method('get')->with('_target_path')
+            ->will($this->returnValue(array('value' => '/dashboard')));
+
+        $response = $this->expectRedirectResponse('/dashboard');
+
+        $handler = new DefaultAuthenticationSuccessHandler($this->httpUtils, array('target_path_parameter' => '_target_path[value]'));
+        $result = $handler->onAuthenticationSuccess($this->request, $this->token);
+
+        $this->assertSame($response, $result);
+    }
+
     public function testTargetPathParameterIsCustomised()
     {
         $options = array('target_path_parameter' => '_my_target_path');

src/Symfony/Component/Security/Http/composer.json

@@ -20,7 +20,8 @@
         "symfony/security-core": "~2.8",
         "symfony/event-dispatcher": "~2.1|~3.0.0",
         "symfony/http-foundation": "~2.4|~3.0.0",
-        "symfony/http-kernel": "~2.4|~3.0.0"
+        "symfony/http-kernel": "~2.4|~3.0.0",
+        "symfony/property-access": "~2.3|~3.0.0"
     },
     "require-dev": {
         "symfony/phpunit-bridge": "~2.7|~3.0.0",