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

@@ -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');
 }
 

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']

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('<none>'));
+  $request->attributes->set(RouteObjectInterface::ROUTE_NAME, '<none>');
 
   // 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();
 }
 
 /**

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();
   }
 
   /**

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'],

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('<none>'));
     $request->attributes->set(RouteObjectInterface::ROUTE_NAME, '<none>');
     $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.

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);

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');
   }
 
   /**

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);

core/lib/Drupal/Core/StackMiddleware/Session.php

@@ -0,0 +1,63 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\StackMiddleware\Session.
+ */
+
+namespace Drupal\Core\StackMiddleware;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Session\SessionInterface;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+
+/**
+ * Wrap session logic around a HTTP request.
+ */
+class Session implements HttpKernelInterface {
+
+  /**
+   * The wrapped HTTP kernel.
+   *
+   * @var \Symfony\Component\HttpKernel\HttpKernelInterface
+   */
+  protected $httpKernel;
+
+  /**
+   * The session.
+   *
+   * @var \Symfony\Component\HttpFoundation\Session\SessionInterface
+   */
+  protected $session;
+
+  /**
+   * Constructs a Session stack middleware object.
+   *
+   * @param \Symfony\Component\HttpKernel\HttpKernelInterface $http_kernel
+   *   The decorated kernel.
+   * @param \Symfony\Component\HttpFoundation\Session\SessionInterface $session
+   *   The session.
+   */
+  public function __construct(HttpKernelInterface $http_kernel, SessionInterface $session) {
+    $this->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;
+  }
+
+}

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);

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 }

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());
   }
 
   /**

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();
         }
       }
 

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();
 }
 

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: