Merge branch '2.8' into 3.2

* 2.8: Fixed pathinfo calculation for requests starting with a question mark. [Security] simplify the SwitchUserListenerTest
2017-03-22 13:39:24 -07:00 · 2017-03-22 13:39:24 -07:00 · f29664893d
commit f29664893d
parent 04fcac74b1 89bb89538b
4 changed files with 121 additions and 149 deletions
--- a/src/Symfony/Component/HttpFoundation/Request.php
+++ b/src/Symfony/Component/HttpFoundation/Request.php
@ -1774,6 +1774,9 @@ class Request

        // Does the baseUrl have anything in common with the request_uri?
        $requestUri = $this->getRequestUri();
+        if ($requestUri !== '' && $requestUri[0] !== '/') {
+            $requestUri = '/'.$requestUri;
+        }

        if ($baseUrl && false !== $prefix = $this->getUrlencodedPrefix($requestUri, $baseUrl)) {
            // full $baseUrl matches
@ -1846,9 +1849,12 @@ class Request
        }

        // Remove the query string from REQUEST_URI
-        if ($pos = strpos($requestUri, '?')) {
+        if (false !== $pos = strpos($requestUri, '?')) {
            $requestUri = substr($requestUri, 0, $pos);
        }
+        if ($requestUri !== '' && $requestUri[0] !== '/') {
+            $requestUri = '/'.$requestUri;
+        }

        $pathInfo = substr($requestUri, strlen($baseUrl));
        if (null !== $baseUrl && (false === $pathInfo || '' === $pathInfo)) {
--- a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
+++ b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
@ -1281,6 +1281,12 @@ class RequestTest extends TestCase
        $request->initialize(array(), array(), array(), array(), array(), $server);

        $this->assertEquals('/path%20test/info', $request->getPathInfo());
+
+        $server = array();
+        $server['REQUEST_URI'] = '?a=b';
+        $request->initialize(array(), array(), array(), array(), array(), $server);
+
+        $this->assertEquals('/', $request->getPathInfo());
    }

    public function testGetParameterPrecedence()
@ -2110,6 +2116,61 @@ class RequestTest extends TestCase
            array('CONNECT', false),
        );
    }
+
+    public function nonstandardRequestsData()
+    {
+        return array(
+            array('',  '', '/', 'http://host:8080/', ''),
+            array('/', '', '/', 'http://host:8080/', ''),
+
+            array('hello/app.php/x',  '', '/x', 'http://host:8080/hello/app.php/x', '/hello', '/hello/app.php'),
+            array('/hello/app.php/x', '', '/x', 'http://host:8080/hello/app.php/x', '/hello', '/hello/app.php'),
+
+            array('',      'a=b', '/', 'http://host:8080/?a=b'),
+            array('?a=b',  'a=b', '/', 'http://host:8080/?a=b'),
+            array('/?a=b', 'a=b', '/', 'http://host:8080/?a=b'),
+
+            array('x',      'a=b', '/x', 'http://host:8080/x?a=b'),
+            array('x?a=b',  'a=b', '/x', 'http://host:8080/x?a=b'),
+            array('/x?a=b', 'a=b', '/x', 'http://host:8080/x?a=b'),
+
+            array('hello/x',  '', '/x', 'http://host:8080/hello/x', '/hello'),
+            array('/hello/x', '', '/x', 'http://host:8080/hello/x', '/hello'),
+
+            array('hello/app.php/x',      'a=b', '/x', 'http://host:8080/hello/app.php/x?a=b', '/hello', '/hello/app.php'),
+            array('hello/app.php/x?a=b',  'a=b', '/x', 'http://host:8080/hello/app.php/x?a=b', '/hello', '/hello/app.php'),
+            array('/hello/app.php/x?a=b', 'a=b', '/x', 'http://host:8080/hello/app.php/x?a=b', '/hello', '/hello/app.php'),
+        );
+    }
+
+    /**
+     * @dataProvider nonstandardRequestsData
+     */
+    public function testNonstandardRequests($requestUri, $queryString, $expectedPathInfo, $expectedUri, $expectedBasePath = '', $expectedBaseUrl = null)
+    {
+        if (null === $expectedBaseUrl) {
+            $expectedBaseUrl = $expectedBasePath;
+        }
+
+        $server = array(
+            'HTTP_HOST' => 'host:8080',
+            'SERVER_PORT' => '8080',
+            'QUERY_STRING' => $queryString,
+            'PHP_SELF' => '/hello/app.php',
+            'SCRIPT_FILENAME' => '/some/path/app.php',
+            'REQUEST_URI' => $requestUri,
+        );
+
+        $request = new Request(array(), array(), array(), array(), array(), $server);
+
+        $this->assertEquals($expectedPathInfo, $request->getPathInfo());
+        $this->assertEquals($expectedUri, $request->getUri());
+        $this->assertEquals($queryString, $request->getQueryString());
+        $this->assertEquals(8080, $request->getPort());
+        $this->assertEquals('host:8080', $request->getHttpHost());
+        $this->assertEquals($expectedBaseUrl, $request->getBaseUrl());
+        $this->assertEquals($expectedBasePath, $request->getBasePath());
+    }
 }

 class RequestContentProxy extends Request
--- a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php
+++ b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php
@ -12,6 +12,13 @@
 namespace Symfony\Component\Security\Http\Tests\Firewall;

 use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
+use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
+use Symfony\Component\Security\Core\Role\SwitchUserRole;
+use Symfony\Component\Security\Core\User\User;
 use Symfony\Component\Security\Http\Event\SwitchUserEvent;
 use Symfony\Component\Security\Http\Firewall\SwitchUserListener;
 use Symfony\Component\Security\Http\SecurityEvents;
@ -32,14 +39,12 @@ class SwitchUserListenerTest extends TestCase

    protected function setUp()
    {
-        $this->tokenStorage = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface')->getMock();
+        $this->tokenStorage = new TokenStorage();
        $this->userProvider = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserProviderInterface')->getMock();
        $this->userChecker = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserCheckerInterface')->getMock();
        $this->accessDecisionManager = $this->getMockBuilder('Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface')->getMock();
-        $this->request = $this->getMockBuilder('Symfony\Component\HttpFoundation\Request')->getMock();
-        $this->request->query = $this->getMockBuilder('Symfony\Component\HttpFoundation\ParameterBag')->getMock();
-        $this->request->server = $this->getMockBuilder('Symfony\Component\HttpFoundation\ServerBag')->getMock();
-        $this->event = $this->getEvent($this->request);
+        $this->request = new Request();
+        $this->event = new GetResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $this->request, HttpKernelInterface::MASTER_REQUEST);
    }

    /**
@ -53,13 +58,11 @@ class SwitchUserListenerTest extends TestCase

    public function testEventIsIgnoredIfUsernameIsNotPassedWithTheRequest()
    {
-        $this->request->expects($this->any())->method('get')->with('_switch_user')->will($this->returnValue(null));
-
-        $this->event->expects($this->never())->method('setResponse');
-        $this->tokenStorage->expects($this->never())->method('setToken');
-
        $listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager);
        $listener->handle($this->event);
+
+        $this->assertNull($this->event->getResponse());
+        $this->assertNull($this->tokenStorage->getToken());
    }

    /**
@ -67,10 +70,10 @@ class SwitchUserListenerTest extends TestCase
     */
    public function testExitUserThrowsAuthenticationExceptionIfOriginalTokenCannotBeFound()
    {
-        $token = $this->getToken(array($this->getMockBuilder('Symfony\Component\Security\Core\Role\RoleInterface')->getMock()));
+        $token = new UsernamePasswordToken('username', '', 'key', array('ROLE_FOO'));

-        $this->tokenStorage->expects($this->any())->method('getToken')->will($this->returnValue($token));
-        $this->request->expects($this->any())->method('get')->with('_switch_user')->will($this->returnValue('_exit'));
+        $this->tokenStorage->setToken($token);
+        $this->request->query->set('_switch_user', '_exit');

        $listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager);
        $listener->handle($this->event);
@ -78,29 +81,19 @@ class SwitchUserListenerTest extends TestCase

    public function testExitUserUpdatesToken()
    {
-        $originalToken = $this->getToken();
-        $role = $this->getMockBuilder('Symfony\Component\Security\Core\Role\SwitchUserRole')
-            ->disableOriginalConstructor()
-            ->getMock();
-        $role->expects($this->any())->method('getSource')->will($this->returnValue($originalToken));
+        $originalToken = new UsernamePasswordToken('username', '', 'key', array());
+        $this->tokenStorage->setToken(new UsernamePasswordToken('username', '', 'key', array(new SwitchUserRole('ROLE_PREVIOUS', $originalToken))));

-        $this->tokenStorage->expects($this->any())
-            ->method('getToken')
-            ->will($this->returnValue($this->getToken(array($role))));
-
-        $this->request->expects($this->any())->method('get')->with('_switch_user')->will($this->returnValue('_exit'));
-        $this->request->expects($this->any())->method('getUri')->will($this->returnValue('/'));
-        $this->request->query->expects($this->once())->method('remove', '_switch_user');
-        $this->request->query->expects($this->any())->method('all')->will($this->returnValue(array()));
-        $this->request->server->expects($this->once())->method('set')->with('QUERY_STRING', '');
-
-        $this->tokenStorage->expects($this->once())
-            ->method('setToken')->with($originalToken);
-        $this->event->expects($this->once())
-            ->method('setResponse')->with($this->isInstanceOf('Symfony\Component\HttpFoundation\RedirectResponse'));
+        $this->request->query->set('_switch_user', '_exit');

        $listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager);
        $listener->handle($this->event);
+
+        $this->assertSame(array(), $this->request->query->all());
+        $this->assertSame('', $this->request->server->get('QUERY_STRING'));
+        $this->assertInstanceOf('Symfony\Component\HttpFoundation\RedirectResponse', $this->event->getResponse());
+        $this->assertSame($this->request->getUri(), $this->event->getResponse()->getTargetUrl());
+        $this->assertSame($originalToken, $this->tokenStorage->getToken());
    }

    public function testExitUserDispatchesEventWithRefreshedUser()
@ -113,38 +106,9 @@ class SwitchUserListenerTest extends TestCase
            ->method('refreshUser')
            ->with($originalUser)
            ->willReturn($refreshedUser);
-        $originalToken = $this->getToken();
-        $originalToken
-            ->expects($this->any())
-            ->method('getUser')
-            ->willReturn($originalUser);
-        $role = $this
-            ->getMockBuilder('Symfony\Component\Security\Core\Role\SwitchUserRole')
-            ->disableOriginalConstructor()
-            ->getMock();
-        $role->expects($this->any())->method('getSource')->willReturn($originalToken);
-        $this
-            ->tokenStorage
-            ->expects($this->any())
-            ->method('getToken')
-            ->willReturn($this->getToken(array($role)));
-        $this
-            ->request
-            ->expects($this->any())
-            ->method('get')
-            ->with('_switch_user')
-            ->willReturn('_exit');
-        $this
-            ->request
-            ->expects($this->any())
-            ->method('getUri')
-            ->willReturn('/');
-        $this
-            ->request
-            ->query
-            ->expects($this->any())
-            ->method('all')
-            ->will($this->returnValue(array()));
+        $originalToken = new UsernamePasswordToken($originalUser, '', 'key');
+        $this->tokenStorage->setToken(new UsernamePasswordToken('username', '', 'key', array(new SwitchUserRole('ROLE_PREVIOUS', $originalToken))));
+        $this->request->query->set('_switch_user', '_exit');

        $dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')->getMock();
        $dispatcher
@ -166,41 +130,9 @@ class SwitchUserListenerTest extends TestCase
            ->userProvider
            ->expects($this->never())
            ->method('refreshUser');
-        $originalToken = $this->getToken();
-        $originalToken
-            ->expects($this->any())
-            ->method('getUser')
-            ->willReturn($originalUser);
-        $role = $this
-            ->getMockBuilder('Symfony\Component\Security\Core\Role\SwitchUserRole')
-            ->disableOriginalConstructor()
-            ->getMock();
-        $role
-            ->expects($this->any())
-            ->method('getSource')
-            ->willReturn($originalToken);
-        $this
-            ->tokenStorage
-            ->expects($this->any())
-            ->method('getToken')
-            ->willReturn($this->getToken(array($role)));
-        $this
-            ->request
-            ->expects($this->any())
-            ->method('get')
-            ->with('_switch_user')
-            ->willReturn('_exit');
-        $this
-            ->request
-            ->query
-            ->expects($this->any())
-            ->method('all')
-            ->will($this->returnValue(array()));
-        $this
-            ->request
-            ->expects($this->any())
-            ->method('getUri')
-            ->willReturn('/');
+        $originalToken = new UsernamePasswordToken($originalUser, '', 'key');
+        $this->tokenStorage->setToken(new UsernamePasswordToken('username', '', 'key', array(new SwitchUserRole('ROLE_PREVIOUS', $originalToken))));
+        $this->request->query->set('_switch_user', '_exit');

        $dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')->getMock();
        $dispatcher
@ -217,10 +149,10 @@ class SwitchUserListenerTest extends TestCase
     */
    public function testSwitchUserIsDisallowed()
    {
-        $token = $this->getToken(array($this->getMockBuilder('Symfony\Component\Security\Core\Role\RoleInterface')->getMock()));
+        $token = new UsernamePasswordToken('username', '', 'key', array('ROLE_FOO'));

-        $this->tokenStorage->expects($this->any())->method('getToken')->will($this->returnValue($token));
-        $this->request->expects($this->any())->method('get')->with('_switch_user')->will($this->returnValue('kuba'));
+        $this->tokenStorage->setToken($token);
+        $this->request->query->set('_switch_user', 'kuba');

        $this->accessDecisionManager->expects($this->once())
            ->method('decide')->with($token, array('ROLE_ALLOWED_TO_SWITCH'))
@ -232,17 +164,11 @@ class SwitchUserListenerTest extends TestCase

    public function testSwitchUser()
    {
-        $token = $this->getToken(array($this->getMockBuilder('Symfony\Component\Security\Core\Role\RoleInterface')->getMock()));
-        $user = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock();
-        $user->expects($this->any())->method('getRoles')->will($this->returnValue(array()));
+        $token = new UsernamePasswordToken('username', '', 'key', array('ROLE_FOO'));
+        $user = new User('username', 'password', array());

-        $this->tokenStorage->expects($this->any())->method('getToken')->will($this->returnValue($token));
-        $this->request->expects($this->any())->method('get')->with('_switch_user')->will($this->returnValue('kuba'));
-        $this->request->query->expects($this->once())->method('remove', '_switch_user');
-        $this->request->query->expects($this->any())->method('all')->will($this->returnValue(array()));
-
-        $this->request->expects($this->any())->method('getUri')->will($this->returnValue('/'));
-        $this->request->server->expects($this->once())->method('set')->with('QUERY_STRING', '');
+        $this->tokenStorage->setToken($token);
+        $this->request->query->set('_switch_user', 'kuba');

        $this->accessDecisionManager->expects($this->once())
            ->method('decide')->with($token, array('ROLE_ALLOWED_TO_SWITCH'))
@ -253,25 +179,26 @@ class SwitchUserListenerTest extends TestCase
            ->will($this->returnValue($user));
        $this->userChecker->expects($this->once())
            ->method('checkPostAuth')->with($user);
-        $this->tokenStorage->expects($this->once())
-            ->method('setToken')->with($this->isInstanceOf('Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken'));

        $listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager);
        $listener->handle($this->event);
+
+        $this->assertSame(array(), $this->request->query->all());
+        $this->assertSame('', $this->request->server->get('QUERY_STRING'));
+        $this->assertInstanceOf('Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken', $this->tokenStorage->getToken());
    }

    public function testSwitchUserKeepsOtherQueryStringParameters()
    {
-        $token = $this->getToken(array($this->getMockBuilder('Symfony\Component\Security\Core\Role\RoleInterface')->getMock()));
-        $user = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock();
-        $user->expects($this->any())->method('getRoles')->will($this->returnValue(array()));
+        $token = new UsernamePasswordToken('username', '', 'key', array('ROLE_FOO'));
+        $user = new User('username', 'password', array());

-        $this->tokenStorage->expects($this->any())->method('getToken')->will($this->returnValue($token));
-        $this->request->expects($this->any())->method('get')->with('_switch_user')->will($this->returnValue('kuba'));
-        $this->request->query->expects($this->once())->method('remove', '_switch_user');
-        $this->request->query->expects($this->any())->method('all')->will($this->returnValue(array('page' => 3, 'section' => 2)));
-        $this->request->expects($this->any())->method('getUri')->will($this->returnValue('/'));
-        $this->request->server->expects($this->once())->method('set')->with('QUERY_STRING', 'page=3&section=2');
+        $this->tokenStorage->setToken($token);
+        $this->request->query->replace(array(
+            '_switch_user' => 'kuba',
+            'page' => 3,
+            'section' => 2,
+        ));

        $this->accessDecisionManager->expects($this->once())
            ->method('decide')->with($token, array('ROLE_ALLOWED_TO_SWITCH'))
@ -282,33 +209,11 @@ class SwitchUserListenerTest extends TestCase
            ->will($this->returnValue($user));
        $this->userChecker->expects($this->once())
            ->method('checkPostAuth')->with($user);
-        $this->tokenStorage->expects($this->once())
-            ->method('setToken')->with($this->isInstanceOf('Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken'));

        $listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager);
        $listener->handle($this->event);
-    }

-    private function getEvent($request)
-    {
-        $event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent')
-            ->disableOriginalConstructor()
-            ->getMock();
-
-        $event->expects($this->any())
-            ->method('getRequest')
-            ->will($this->returnValue($request));
-
-        return $event;
-    }
-
-    private function getToken(array $roles = array())
-    {
-        $token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock();
-        $token->expects($this->any())
-            ->method('getRoles')
-            ->will($this->returnValue($roles));
-
-        return $token;
+        $this->assertSame('page=3&section=2', $this->request->server->get('QUERY_STRING'));
+        $this->assertInstanceOf('Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken', $this->tokenStorage->getToken());
    }
 }
--- a/src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php
+++ b/src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php
@ -144,7 +144,7 @@ class HttpUtilsTest extends TestCase
        // Plus must not decoded to space
        $this->assertTrue($utils->checkRequestPath($this->getRequest('/foo+bar'), '/foo+bar'));
        // Checking unicode
-        $this->assertTrue($utils->checkRequestPath($this->getRequest(urlencode('/вход')), '/вход'));
+        $this->assertTrue($utils->checkRequestPath($this->getRequest('/'.urlencode('вход')), '/вход'));
    }

    public function testCheckRequestPathWithUrlMatcherAndResourceNotFound()