From 47fba88123fcc797444a21023f8c817a2166a076 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Fri, 3 Apr 2015 18:13:34 +0200 Subject: [PATCH] deprecate finding deep items in request parameters --- .../Component/HttpFoundation/CHANGELOG.md | 6 ++ .../Component/HttpFoundation/ParameterBag.php | 20 ++-- .../Component/HttpFoundation/Request.php | 6 ++ .../HttpFoundation/Tests/ParameterBagTest.php | 4 + .../DefaultAuthenticationFailureHandler.php | 3 +- .../DefaultAuthenticationSuccessHandler.php | 3 +- .../Security/Http/Firewall/LogoutListener.php | 3 +- .../SimpleFormAuthenticationListener.php | 11 ++- ...namePasswordFormAuthenticationListener.php | 11 ++- .../Security/Http/ParameterBagUtils.php | 96 +++++++++++++++++++ .../RememberMe/AbstractRememberMeServices.php | 3 +- ...efaultAuthenticationFailureHandlerTest.php | 17 +++- ...efaultAuthenticationSuccessHandlerTest.php | 14 +++ .../Component/Security/Http/composer.json | 3 +- 14 files changed, 176 insertions(+), 24 deletions(-) create mode 100644 src/Symfony/Component/Security/Http/ParameterBagUtils.php diff --git a/src/Symfony/Component/HttpFoundation/CHANGELOG.md b/src/Symfony/Component/HttpFoundation/CHANGELOG.md index dcdeb4ebf9..9f48e8252d 100644 --- a/src/Symfony/Component/HttpFoundation/CHANGELOG.md +++ b/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 ----- diff --git a/src/Symfony/Component/HttpFoundation/ParameterBag.php b/src/Symfony/Component/HttpFoundation/ParameterBag.php index ecc0de1070..c61c4f23f4 100644 --- a/src/Symfony/Component/HttpFoundation/ParameterBag.php +++ b/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) { diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php index 542987a8b9..7c8732a705 100644 --- a/src/Symfony/Component/HttpFoundation/Request.php +++ b/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; } diff --git a/src/Symfony/Component/HttpFoundation/Tests/ParameterBagTest.php b/src/Symfony/Component/HttpFoundation/Tests/ParameterBagTest.php index 16fedbf83a..94416549d2 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/ParameterBagTest.php +++ b/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')))); diff --git a/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationFailureHandler.php b/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationFailureHandler.php index f8004d638b..3e63992639 100644 --- a/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationFailureHandler.php +++ b/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; } diff --git a/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationSuccessHandler.php b/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationSuccessHandler.php index 5fa7071c64..078a366384 100644 --- a/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationSuccessHandler.php +++ b/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; } diff --git a/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php b/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php index 96f568534d..6211ee0323 100644 --- a/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php +++ b/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.'); diff --git a/src/Symfony/Component/Security/Http/Firewall/SimpleFormAuthenticationListener.php b/src/Symfony/Component/Security/Http/Firewall/SimpleFormAuthenticationListener.php index 4733b6ac26..fedaa4e62f 100644 --- a/src/Symfony/Component/Security/Http/Firewall/SimpleFormAuthenticationListener.php +++ b/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); diff --git a/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php b/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php index 07ab85ae7e..d20ab19f62 100644 --- a/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php +++ b/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); diff --git a/src/Symfony/Component/Security/Http/ParameterBagUtils.php b/src/Symfony/Component/Security/Http/ParameterBagUtils.php new file mode 100644 index 0000000000..eed5421f8f --- /dev/null +++ b/src/Symfony/Component/Security/Http/ParameterBagUtils.php @@ -0,0 +1,96 @@ + + * + * 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; + } + } +} diff --git a/src/Symfony/Component/Security/Http/RememberMe/AbstractRememberMeServices.php b/src/Symfony/Component/Security/Http/RememberMe/AbstractRememberMeServices.php index 16810bd17d..04d95ea3a5 100644 --- a/src/Symfony/Component/Security/Http/RememberMe/AbstractRememberMeServices.php +++ b/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'])); diff --git a/src/Symfony/Component/Security/Http/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php b/src/Symfony/Component/Security/Http/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php index 82b5533658..2ed8872755 100644 --- a/src/Symfony/Component/Security/Http/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php +++ b/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()) diff --git a/src/Symfony/Component/Security/Http/Tests/Authentication/DefaultAuthenticationSuccessHandlerTest.php b/src/Symfony/Component/Security/Http/Tests/Authentication/DefaultAuthenticationSuccessHandlerTest.php index 4d1847d0b7..2c22da607c 100644 --- a/src/Symfony/Component/Security/Http/Tests/Authentication/DefaultAuthenticationSuccessHandlerTest.php +++ b/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'); diff --git a/src/Symfony/Component/Security/Http/composer.json b/src/Symfony/Component/Security/Http/composer.json index 43dff2e23d..c4a4349acf 100644 --- a/src/Symfony/Component/Security/Http/composer.json +++ b/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",