From 7b2c3267193382cc45f8d0fd7ca4127e19da7985 Mon Sep 17 00:00:00 2001 From: Arman Hosseini <44655055+Arman-Hosseini@users.noreply.github.com> Date: Wed, 24 Jul 2019 16:11:31 +0430 Subject: [PATCH] Ensure $request->hasSession() is always checked before calling getSession() --- src/Symfony/Bridge/Twig/AppVariable.php | 8 +++----- src/Symfony/Bridge/Twig/Tests/AppVariableTest.php | 2 ++ .../FrameworkBundle/Templating/GlobalVariables.php | 6 +++--- .../Controller/ProfilerController.php | 2 +- .../EventListener/WebDebugToolbarListener.php | 3 +-- .../EventListener/AbstractTestSessionListener.php | 3 +-- .../HttpKernel/EventListener/SaveSessionListener.php | 4 ++-- .../Http/Authentication/AuthenticationUtils.php | 7 ++----- .../Security/Http/Firewall/ContextListener.php | 12 ++++++------ 9 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/Symfony/Bridge/Twig/AppVariable.php b/src/Symfony/Bridge/Twig/AppVariable.php index 21020270a0..a874e37c9b 100644 --- a/src/Symfony/Bridge/Twig/AppVariable.php +++ b/src/Symfony/Bridge/Twig/AppVariable.php @@ -112,10 +112,9 @@ class AppVariable if (null === $this->requestStack) { throw new \RuntimeException('The "app.session" variable is not available.'); } + $request = $this->getRequest(); - if ($request = $this->getRequest()) { - return $request->getSession(); - } + return $request && $request->hasSession() ? $request->getSession() : null; } /** @@ -157,8 +156,7 @@ class AppVariable public function getFlashes($types = null) { try { - $session = $this->getSession(); - if (null === $session) { + if (null === $session = $this->getSession()) { return []; } } catch (\RuntimeException $e) { diff --git a/src/Symfony/Bridge/Twig/Tests/AppVariableTest.php b/src/Symfony/Bridge/Twig/Tests/AppVariableTest.php index 53b84b2d1b..b6699e5cdc 100644 --- a/src/Symfony/Bridge/Twig/Tests/AppVariableTest.php +++ b/src/Symfony/Bridge/Twig/Tests/AppVariableTest.php @@ -51,6 +51,7 @@ class AppVariableTest extends TestCase public function testGetSession() { $request = $this->getMockBuilder('Symfony\Component\HttpFoundation\Request')->getMock(); + $request->method('hasSession')->willReturn(true); $request->method('getSession')->willReturn($session = new Session()); $this->setRequestStack($request); @@ -267,6 +268,7 @@ class AppVariableTest extends TestCase $session->method('getFlashBag')->willReturn($flashBag); $request = $this->getMockBuilder('Symfony\Component\HttpFoundation\Request')->getMock(); + $request->method('hasSession')->willReturn(true); $request->method('getSession')->willReturn($session); $this->setRequestStack($request); diff --git a/src/Symfony/Bundle/FrameworkBundle/Templating/GlobalVariables.php b/src/Symfony/Bundle/FrameworkBundle/Templating/GlobalVariables.php index 2981eb6642..5f30595635 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Templating/GlobalVariables.php +++ b/src/Symfony/Bundle/FrameworkBundle/Templating/GlobalVariables.php @@ -75,9 +75,9 @@ class GlobalVariables */ public function getSession() { - if ($request = $this->getRequest()) { - return $request->getSession(); - } + $request = $this->getRequest(); + + return $request && $request->hasSession() ? $request->getSession() : null; } /** diff --git a/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php b/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php index c8a560295d..707f8040c3 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php +++ b/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php @@ -123,7 +123,7 @@ class ProfilerController throw new NotFoundHttpException('The profiler must be enabled.'); } - if ($request->hasSession() && ($session = $request->getSession()) && $session->isStarted() && $session->getFlashBag() instanceof AutoExpireFlashBag) { + if ($request->hasSession() && ($session = $request->getSession())->isStarted() && $session->getFlashBag() instanceof AutoExpireFlashBag) { // keep current flashes for one more request if using AutoExpireFlashBag $session->getFlashBag()->setAll($session->getFlashBag()->peekAll()); } diff --git a/src/Symfony/Bundle/WebProfilerBundle/EventListener/WebDebugToolbarListener.php b/src/Symfony/Bundle/WebProfilerBundle/EventListener/WebDebugToolbarListener.php index 1541c7113b..fbcdb67826 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/EventListener/WebDebugToolbarListener.php +++ b/src/Symfony/Bundle/WebProfilerBundle/EventListener/WebDebugToolbarListener.php @@ -88,8 +88,7 @@ class WebDebugToolbarListener implements EventSubscriberInterface } if ($response->headers->has('X-Debug-Token') && $response->isRedirect() && $this->interceptRedirects && 'html' === $request->getRequestFormat()) { - $session = $request->getSession(); - if (null !== $session && $session->isStarted() && $session->getFlashBag() instanceof AutoExpireFlashBag) { + if ($request->hasSession() && ($session = $request->getSession())->isStarted() && $session->getFlashBag() instanceof AutoExpireFlashBag) { // keep current flashes for one more request if using AutoExpireFlashBag $session->getFlashBag()->setAll($session->getFlashBag()->peekAll()); } diff --git a/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php index 054695e6f2..86f179add7 100644 --- a/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php +++ b/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php @@ -46,8 +46,7 @@ abstract class AbstractTestSessionListener implements EventSubscriberInterface } // bootstrap the session - $session = $this->getSession(); - if (!$session) { + if (!$session = $this->getSession()) { return; } diff --git a/src/Symfony/Component/HttpKernel/EventListener/SaveSessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/SaveSessionListener.php index b14153ad3c..3b105ced34 100644 --- a/src/Symfony/Component/HttpKernel/EventListener/SaveSessionListener.php +++ b/src/Symfony/Component/HttpKernel/EventListener/SaveSessionListener.php @@ -30,8 +30,8 @@ class SaveSessionListener implements EventSubscriberInterface return; } - $session = $event->getRequest()->getSession(); - if ($session && $session->isStarted()) { + $request = $event->getRequest(); + if ($request->hasSession() && ($session = $request->getSession())->isStarted()) { $session->save(); } } diff --git a/src/Symfony/Component/Security/Http/Authentication/AuthenticationUtils.php b/src/Symfony/Component/Security/Http/Authentication/AuthenticationUtils.php index fbdc0bc5eb..af7e4919c3 100644 --- a/src/Symfony/Component/Security/Http/Authentication/AuthenticationUtils.php +++ b/src/Symfony/Component/Security/Http/Authentication/AuthenticationUtils.php @@ -38,12 +38,11 @@ class AuthenticationUtils public function getLastAuthenticationError($clearSession = true) { $request = $this->getRequest(); - $session = $request->getSession(); $authenticationException = null; if ($request->attributes->has(Security::AUTHENTICATION_ERROR)) { $authenticationException = $request->attributes->get(Security::AUTHENTICATION_ERROR); - } elseif (null !== $session && $session->has(Security::AUTHENTICATION_ERROR)) { + } elseif ($request->hasSession() && ($session = $request->getSession())->has(Security::AUTHENTICATION_ERROR)) { $authenticationException = $session->get(Security::AUTHENTICATION_ERROR); if ($clearSession) { @@ -65,9 +64,7 @@ class AuthenticationUtils return $request->attributes->get(Security::LAST_USERNAME, ''); } - $session = $request->getSession(); - - return null === $session ? '' : $session->get(Security::LAST_USERNAME, ''); + return $request->hasSession() ? $request->getSession()->get(Security::LAST_USERNAME, '') : ''; } /** diff --git a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php index 16cdc8f9e2..e58b2e3f7d 100644 --- a/src/Symfony/Component/Security/Http/Firewall/ContextListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/ContextListener.php @@ -90,7 +90,7 @@ class ContextListener implements ListenerInterface } $request = $event->getRequest(); - $session = $request->hasPreviousSession() ? $request->getSession() : null; + $session = $request->hasPreviousSession() && $request->hasSession() ? $request->getSession() : null; if (null === $session || null === $token = $session->get($this->sessionKey)) { $this->tokenStorage->setToken(null); @@ -137,14 +137,14 @@ class ContextListener implements ListenerInterface $this->dispatcher->removeListener(KernelEvents::RESPONSE, [$this, 'onKernelResponse']); $this->registered = false; - $session = $request->getSession(); + $token = $this->tokenStorage->getToken(); - if ((null === $token = $this->tokenStorage->getToken()) || $this->trustResolver->isAnonymous($token)) { - if ($request->hasPreviousSession()) { - $session->remove($this->sessionKey); + if (null === $token || $this->trustResolver->isAnonymous($token)) { + if ($request->hasPreviousSession() && $request->hasSession()) { + $request->getSession()->remove($this->sessionKey); } } else { - $session->set($this->sessionKey, serialize($token)); + $request->getSession()->set($this->sessionKey, serialize($token)); if (null !== $this->logger) { $this->logger->debug('Stored the security token in the session.', ['key' => $this->sessionKey]);