From b926f5d868c7000019f205bd9bae371c329571aa Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Sat, 7 Feb 2015 10:57:50 +0000 Subject: [PATCH] Issue #2229145 by znerol, neclimdul, larowlan, joelpittet, almaudoh: Register symfony session components in the DIC and inject the session service into the request object --- core/authorize.php | 1 - core/core.services.yml | 16 ++++- core/includes/install.core.inc | 21 ++++--- .../Core/Authentication/Provider/Cookie.php | 25 ++------ core/lib/Drupal/Core/Batch/BatchStorage.php | 20 +++--- core/lib/Drupal/Core/DrupalKernel.php | 15 ++++- .../DefaultExceptionHtmlSubscriber.php | 5 ++ .../Installer/InstallerServiceProvider.php | 3 - .../Drupal/Core/Session/SessionManager.php | 2 +- .../Drupal/Core/StackMiddleware/Session.php | 63 +++++++++++++++++++ .../src/Controller/CommentController.php | 4 ++ .../session_test/session_test.services.yml | 1 - .../EventSubscriber/SessionTestSubscriber.php | 18 +----- core/modules/user/src/Entity/User.php | 2 +- core/modules/user/user.module | 8 ++- core/modules/user/user.services.yml | 1 - 16 files changed, 136 insertions(+), 69 deletions(-) create mode 100644 core/lib/Drupal/Core/StackMiddleware/Session.php diff --git a/core/authorize.php b/core/authorize.php index d033b7efb28..dbab02f39ad 100644 --- a/core/authorize.php +++ b/core/authorize.php @@ -51,7 +51,6 @@ const MAINTENANCE_MODE = 'update'; * TRUE if the current user can run authorize.php, and FALSE if not. */ function authorize_access_allowed() { - \Drupal::service('session_manager')->start(); return Settings::get('allow_authorize_operations', TRUE) && \Drupal::currentUser()->hasPermission('administer software updates'); } diff --git a/core/core.services.yml b/core/core.services.yml index 9f5d6fc325b..9fe832403b2 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -475,6 +475,11 @@ services: arguments: ['@kernel'] tags: - { name: http_middleware, priority: 100 } + http_middleware.session: + class: Drupal\Core\StackMiddleware\Session + arguments: ['@session'] + tags: + - { name: http_middleware, priority: 50 } language_manager: class: Drupal\Core\Language\LanguageManager arguments: ['@language.default'] @@ -993,7 +998,7 @@ services: arguments: ['@module_handler', '@cache.discovery', '@language_manager', '@cache_tags.invalidator'] batch.storage: class: Drupal\Core\Batch\BatchStorage - arguments: ['@database', '@session_manager', '@csrf_token'] + arguments: ['@database', '@session', '@csrf_token'] tags: - { name: backend_overridable } replica_database_ignore__subscriber: @@ -1089,6 +1094,15 @@ services: session_configuration: class: Drupal\Core\Session\SessionConfiguration arguments: ['%session.storage.options%'] + session: + class: Symfony\Component\HttpFoundation\Session\Session + arguments: ['@session_manager', '@session.attribute_bag', '@session.flash_bag'] + session.flash_bag: + class: Symfony\Component\HttpFoundation\Session\Flash\FlashBag + public: false + session.attribute_bag: + class: Symfony\Component\HttpFoundation\Session\Attribute\AttributeBag + public: false session_manager: class: Drupal\Core\Session\SessionManager arguments: ['@request_stack', '@database', '@session_manager.metadata_bag', '@session_configuration'] diff --git a/core/includes/install.core.inc b/core/includes/install.core.inc index f98c64d53bf..34187f2c50b 100644 --- a/core/includes/install.core.inc +++ b/core/includes/install.core.inc @@ -21,9 +21,11 @@ use Drupal\Core\Extension\ExtensionDiscovery; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Url; use Drupal\language\Entity\ConfigurableLanguage; +use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Routing\Route; use GuzzleHttp\Exception\RequestException; @@ -396,7 +398,6 @@ function install_begin_request($class_loader, &$install_state) { $kernel->setSitePath($site_path); $kernel->boot(); $container = $kernel->getContainer(); - $container->get('request_stack')->push($request); // Register the file translation service. if (isset($GLOBALS['config']['locale.settings']['translation']['path'])) { @@ -442,13 +443,15 @@ function install_begin_request($class_loader, &$install_state) { if ($profile && !$module_handler->moduleExists($profile)) { $module_handler->addProfile($profile, $install_state['profiles'][$profile]->getPath()); } - // After setting up a custom and finite module list in a custom low-level - // bootstrap like here, ensure to use ModuleHandler::loadAll() so that - // ModuleHandler::isLoaded() returns TRUE, since that is a condition being - // checked by other subsystems (e.g., the theme system). - $module_handler->loadAll(); - $kernel->prepareLegacyRequest($request); + // Load all modules and perform request related initialization. + $kernel->preHandle($request); + + // Initialize a route on this legacy request similar to + // \Drupal\Core\DrupalKernel::prepareLegacyRequest() since normal routing + // will not happen. + $request->attributes->set(RouteObjectInterface::ROUTE_OBJECT, new Route('')); + $request->attributes->set(RouteObjectInterface::ROUTE_NAME, ''); // Prepare for themed output. We need to run this at the beginning of the // page request to avoid a different theme accidentally getting set. (We also @@ -593,7 +596,7 @@ function install_run_task($task, &$install_state) { $response = batch_process($url, clone $url); if ($response instanceof Response) { // Save $_SESSION data from batch. - \Drupal::service('session_manager')->save(); + \Drupal::service('session')->save(); // Send the response. $response->send(); exit; @@ -1548,7 +1551,7 @@ function install_load_profile(&$install_state) { * An array of information about the current installation state. */ function install_bootstrap_full() { - \Drupal::service('session_manager')->start(); + \Drupal::service('session')->start(); } /** diff --git a/core/lib/Drupal/Core/Authentication/Provider/Cookie.php b/core/lib/Drupal/Core/Authentication/Provider/Cookie.php index 5f88c4789ce..e36275f2d59 100644 --- a/core/lib/Drupal/Core/Authentication/Provider/Cookie.php +++ b/core/lib/Drupal/Core/Authentication/Provider/Cookie.php @@ -17,28 +17,11 @@ use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; */ class Cookie implements AuthenticationProviderInterface { - /** - * The session manager. - * - * @var \Drupal\Core\Session\SessionManagerInterface - */ - protected $sessionManager; - - /** - * Constructs a new Cookie authentication provider instance. - * - * @param \Drupal\Core\Session\SessionManagerInterface $session_manager - * The session manager. - */ - public function __construct(SessionManagerInterface $session_manager) { - $this->sessionManager = $session_manager; - } - /** * {@inheritdoc} */ public function applies(Request $request) { - return TRUE; + return $request->hasSession(); } /** @@ -47,10 +30,11 @@ class Cookie implements AuthenticationProviderInterface { public function authenticate(Request $request) { // Global $user is deprecated, but the session system is still based on it. global $user; - $this->sessionManager->start(); - if ($this->sessionManager->isStarted()) { + + if ($request->getSession()->start()) { return $user; } + return NULL; } @@ -58,7 +42,6 @@ class Cookie implements AuthenticationProviderInterface { * {@inheritdoc} */ public function cleanup(Request $request) { - $this->sessionManager->save(); } /** diff --git a/core/lib/Drupal/Core/Batch/BatchStorage.php b/core/lib/Drupal/Core/Batch/BatchStorage.php index da0fe94764d..5742fd2dbcf 100644 --- a/core/lib/Drupal/Core/Batch/BatchStorage.php +++ b/core/lib/Drupal/Core/Batch/BatchStorage.php @@ -8,7 +8,7 @@ namespace Drupal\Core\Batch; use Drupal\Core\Database\Connection; -use Drupal\Core\Session\SessionManager; +use Symfony\Component\HttpFoundation\Session\SessionInterface; use Drupal\Core\Access\CsrfTokenGenerator; class BatchStorage implements BatchStorageInterface { @@ -21,11 +21,11 @@ class BatchStorage implements BatchStorageInterface { protected $connection; /** - * The session manager. + * The session. * - * @var \Drupal\Core\Session\SessionManager + * @var \Symfony\Component\HttpFoundation\Session\SessionInterface */ - protected $sessionManager; + protected $session; /** * The CSRF token generator. @@ -39,14 +39,14 @@ class BatchStorage implements BatchStorageInterface { * * @param \Drupal\Core\Database\Connection $connection * The database connection. - * @param \Drupal\Core\Session\SessionManager $session_manager - * The session manager. + * @param \Symfony\Component\HttpFoundation\Session\SessionInterface $session + * The session. * @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token * The CSRF token generator. */ - public function __construct(Connection $connection, SessionManager $session_manager, CsrfTokenGenerator $csrf_token) { + public function __construct(Connection $connection, SessionInterface $session, CsrfTokenGenerator $csrf_token) { $this->connection = $connection; - $this->sessionManager = $session_manager; + $this->session = $session; $this->csrfToken = $csrf_token; } @@ -55,7 +55,7 @@ class BatchStorage implements BatchStorageInterface { */ public function load($id) { // Ensure that a session is started before using the CSRF token generator. - $this->sessionManager->start(); + $this->session->start(); $batch = $this->connection->query("SELECT batch FROM {batch} WHERE bid = :bid AND token = :token", array( ':bid' => $id, ':token' => $this->csrfToken->get($id), @@ -100,7 +100,7 @@ class BatchStorage implements BatchStorageInterface { */ public function create(array $batch) { // Ensure that a session is started before using the CSRF token generator. - $this->sessionManager->start(); + $this->session->start(); $this->connection->insert('batch') ->fields(array( 'bid' => $batch['id'], diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php index bc36c15fc08..bfa2c980128 100644 --- a/core/lib/Drupal/Core/DrupalKernel.php +++ b/core/lib/Drupal/Core/DrupalKernel.php @@ -571,8 +571,9 @@ class DrupalKernel implements DrupalKernelInterface, TerminableInterface { public function prepareLegacyRequest(Request $request) { $this->boot(); $this->preHandle($request); - // Enter the request scope so that current_user service is available for - // locale/translation sake. + // Setup services which are normally initialized from within stack + // middleware or during the request kernel event. + $request->setSession($this->container->get('session')); $request->attributes->set(RouteObjectInterface::ROUTE_OBJECT, new Route('')); $request->attributes->set(RouteObjectInterface::ROUTE_NAME, ''); $this->container->get('request_stack')->push($request); @@ -718,6 +719,16 @@ class DrupalKernel implements DrupalKernelInterface, TerminableInterface { if ($session_manager_started) { $this->container->get('session_manager')->start(); } + + // The request stack is preserved accross container rebuilds. Reinject the + // new session into the master request if one was present before. + if (($request_stack = $this->container->get('request_stack', ContainerInterface::NULL_ON_INVALID_REFERENCE))) { + if ($request = $request_stack->getMasterRequest()) { + if ($request->hasSession()) { + $request->setSession($this->container->get('session')); + } + } + } \Drupal::setContainer($this->container); // If needs dumping flag was set, dump the container. diff --git a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php index 9535abf5b6f..f757aa7ee1b 100644 --- a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php @@ -111,6 +111,11 @@ class DefaultExceptionHtmlSubscriber extends HttpExceptionSubscriberBase { // Persist the 'exception' attribute to the subrequest. $sub_request->attributes->set('exception', $request->attributes->get('exception')); + // Carry over the session to the subrequest. + if ($session = $request->getSession()) { + $sub_request->setSession($session); + } + $response = $this->httpKernel->handle($sub_request, HttpKernelInterface::SUB_REQUEST); $response->setStatusCode($status_code); $event->setResponse($response); diff --git a/core/lib/Drupal/Core/Installer/InstallerServiceProvider.php b/core/lib/Drupal/Core/Installer/InstallerServiceProvider.php index 96b5e73de25..83a7845156c 100644 --- a/core/lib/Drupal/Core/Installer/InstallerServiceProvider.php +++ b/core/lib/Drupal/Core/Installer/InstallerServiceProvider.php @@ -59,9 +59,6 @@ class InstallerServiceProvider implements ServiceProviderInterface, ServiceModif // @todo Convert installer steps into routes; add an installer.routing.yml. $definition = $container->getDefinition('router.builder'); $definition->setClass('Drupal\Core\Installer\InstallerRouteBuilder'); - - // Remove dependencies on Drupal's default session handling. - $container->removeDefinition('authentication.cookie'); } /** diff --git a/core/lib/Drupal/Core/Session/SessionManager.php b/core/lib/Drupal/Core/Session/SessionManager.php index c354cec8250..2a143b73822 100644 --- a/core/lib/Drupal/Core/Session/SessionManager.php +++ b/core/lib/Drupal/Core/Session/SessionManager.php @@ -338,7 +338,7 @@ class SessionManager extends NativeSessionStorage implements SessionManagerInter // Ignore attribute bags when they do not contain any data. foreach ($this->bags as $bag) { $key = $bag->getStorageKey(); - $mask[$key] = empty($_SESSION[$key]); + $mask[$key] = !empty($_SESSION[$key]); } return array_intersect_key($mask, $_SESSION); diff --git a/core/lib/Drupal/Core/StackMiddleware/Session.php b/core/lib/Drupal/Core/StackMiddleware/Session.php new file mode 100644 index 00000000000..b45a16ec7a2 --- /dev/null +++ b/core/lib/Drupal/Core/StackMiddleware/Session.php @@ -0,0 +1,63 @@ +httpKernel = $http_kernel; + $this->session = $session; + } + + /** + * {@inheritdoc} + */ + public function handle(Request $request, $type = self::MASTER_REQUEST, $catch = TRUE) { + if ($type === self::MASTER_REQUEST) { + $request->setSession($this->session); + } + + $result = $this->httpKernel->handle($request, $type, $catch); + + if ($type === self::MASTER_REQUEST && $request->hasSession()) { + $request->getSession()->save(); + } + + return $result; + } + +} diff --git a/core/modules/comment/src/Controller/CommentController.php b/core/modules/comment/src/Controller/CommentController.php index 84fbf72f0a2..69be36aa03e 100644 --- a/core/modules/comment/src/Controller/CommentController.php +++ b/core/modules/comment/src/Controller/CommentController.php @@ -130,6 +130,10 @@ class CommentController extends ControllerBase { // @todo: Cleaner sub request handling. $redirect_request = Request::create($entity->url(), 'GET', $request->query->all(), $request->cookies->all(), array(), $request->server->all()); $redirect_request->query->set('page', $page); + // Carry over the session to the subrequest. + if ($session = $request->getSession()) { + $redirect_request->setSession($session); + } // @todo: Convert the pager to use the request object. $request->query->set('page', $page); return $this->httpKernel->handle($redirect_request, HttpKernelInterface::SUB_REQUEST); diff --git a/core/modules/system/tests/modules/session_test/session_test.services.yml b/core/modules/system/tests/modules/session_test/session_test.services.yml index 281b09d2165..8ef2e204dec 100644 --- a/core/modules/system/tests/modules/session_test/session_test.services.yml +++ b/core/modules/system/tests/modules/session_test/session_test.services.yml @@ -1,6 +1,5 @@ services: session_test.subscriber: class: Drupal\session_test\EventSubscriber\SessionTestSubscriber - arguments: ['@session_manager'] tags: - { name: event_subscriber } diff --git a/core/modules/system/tests/modules/session_test/src/EventSubscriber/SessionTestSubscriber.php b/core/modules/system/tests/modules/session_test/src/EventSubscriber/SessionTestSubscriber.php index 9029093c64d..a17cd5a4822 100644 --- a/core/modules/system/tests/modules/session_test/src/EventSubscriber/SessionTestSubscriber.php +++ b/core/modules/system/tests/modules/session_test/src/EventSubscriber/SessionTestSubscriber.php @@ -7,7 +7,6 @@ namespace Drupal\session_test\EventSubscriber; -use Drupal\Core\Session\SessionManagerInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpKernel\KernelEvents; @@ -19,13 +18,6 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent; */ class SessionTestSubscriber implements EventSubscriberInterface { - /** - * The session manager. - * - * @var \Drupal\Core\Session\SessionManagerInterface - */ - protected $sessionManager; - /** * Stores whether $_SESSION is empty at the beginning of the request. * @@ -33,13 +25,6 @@ class SessionTestSubscriber implements EventSubscriberInterface { */ protected $emptySession; - /** - * Constructs a new session test subscriber. - */ - public function __construct(SessionManagerInterface $session_manager) { - $this->sessionManager = $session_manager; - } - /** * Set header for session testing. * @@ -47,7 +32,8 @@ class SessionTestSubscriber implements EventSubscriberInterface { * The Event to process. */ public function onKernelRequestSessionTest(GetResponseEvent $event) { - $this->emptySession = (int) !$this->sessionManager->start(); + $session = $event->getRequest()->getSession(); + $this->emptySession = (int) !($session && $session->start()); } /** diff --git a/core/modules/user/src/Entity/User.php b/core/modules/user/src/Entity/User.php index 8be2e5e2e44..a2c3a4703c2 100644 --- a/core/modules/user/src/Entity/User.php +++ b/core/modules/user/src/Entity/User.php @@ -128,7 +128,7 @@ class User extends ContentEntityBase implements UserInterface { if ($this->pass->value != $this->original->pass->value) { $session_manager->delete($this->id()); if ($this->id() == \Drupal::currentUser()->id()) { - $session_manager->regenerate(); + \Drupal::service('session')->migrate(); } } diff --git a/core/modules/user/user.module b/core/modules/user/user.module index 86c063d436f..e09ff0310eb 100644 --- a/core/modules/user/user.module +++ b/core/modules/user/user.module @@ -609,7 +609,7 @@ function user_login_finalize(UserInterface $account) { // This is called before hook_user_login() in case one of those functions // fails or incorrectly does a redirect which would leave the old session // in place. - \Drupal::service('session_manager')->regenerate(); + \Drupal::service('session')->migrate(); \Drupal::moduleHandler()->invokeAll('user_login', array($account)); } @@ -840,7 +840,7 @@ function _user_cancel($edit, $account, $method) { function _user_cancel_session_regenerate() { // Regenerate the users session instead of calling session_destroy() as we // want to preserve any messages that might have been set. - \Drupal::service('session_manager')->regenerate(); + \Drupal::service('session')->migrate(); } /** @@ -1484,6 +1484,10 @@ function user_logout() { \Drupal::moduleHandler()->invokeAll('user_logout', array($user)); // Destroy the current session, and reset $user to the anonymous user. + // Note: In Symfony the session is intended to be destroyed with + // Session::invalidate(). Regrettably this method is currently broken and may + // lead to the creation of spurious session records in the database. + // @see https://github.com/symfony/symfony/issues/12375 session_destroy(); } diff --git a/core/modules/user/user.services.yml b/core/modules/user/user.services.yml index 3b6e4516e63..8a4f4f05ab4 100644 --- a/core/modules/user/user.services.yml +++ b/core/modules/user/user.services.yml @@ -17,7 +17,6 @@ services: - { name: access_check, applies_to: _user_is_logged_in } authentication.cookie: class: Drupal\Core\Authentication\Provider\Cookie - arguments: ['@session_manager'] tags: - { name: authentication_provider, priority: 0 } cache_context.user: