From cebfd8ec546d65958c39aec1b1fa20d487d053c9 Mon Sep 17 00:00:00 2001 From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org> Date: Thu, 3 Jul 2014 13:48:25 +0100 Subject: [PATCH] Issue #2272987 by znerol, alexpott: Fixed Do not persist session manager. --- core/authorize.php | 2 +- core/core.services.yml | 2 - core/includes/install.core.inc | 2 +- .../Core/Authentication/Provider/Cookie.php | 3 +- core/lib/Drupal/Core/DrupalKernel.php | 19 +++++++ .../Drupal/Core/Session/SessionManager.php | 51 ++++++++++++------- .../Core/Session/SessionManagerInterface.php | 15 ++++-- core/update.php | 2 +- 8 files changed, 70 insertions(+), 26 deletions(-) diff --git a/core/authorize.php b/core/authorize.php index 04c4e54f2ac0..5418007a76ff 100644 --- a/core/authorize.php +++ b/core/authorize.php @@ -49,7 +49,7 @@ * TRUE if the current user can run authorize.php, and FALSE if not. */ function authorize_access_allowed() { - \Drupal::service('session_manager')->initialize(); + \Drupal::service('session_manager')->startLazy(); return Settings::get('allow_authorize_operations', TRUE) && user_access('administer software updates'); } diff --git a/core/core.services.yml b/core/core.services.yml index 022fa7e3525c..f30cee7aaeb9 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -800,8 +800,6 @@ services: session_manager: class: Drupal\Core\Session\SessionManager arguments: ['@request_stack', '@database', '@session_manager.metadata_bag', '@settings'] - tags: - - { name: persist } session_manager.metadata_bag: class: Drupal\Core\Session\MetadataBag arguments: ['@settings'] diff --git a/core/includes/install.core.inc b/core/includes/install.core.inc index 158f8a9142df..a6fa11dd25a5 100644 --- a/core/includes/install.core.inc +++ b/core/includes/install.core.inc @@ -1451,7 +1451,7 @@ function install_load_profile(&$install_state) { * An array of information about the current installation state. */ function install_bootstrap_full() { - \Drupal::service('session_manager')->initialize(); + \Drupal::service('session_manager')->startLazy(); } /** diff --git a/core/lib/Drupal/Core/Authentication/Provider/Cookie.php b/core/lib/Drupal/Core/Authentication/Provider/Cookie.php index 167a0b0747e5..c5f23c865b77 100644 --- a/core/lib/Drupal/Core/Authentication/Provider/Cookie.php +++ b/core/lib/Drupal/Core/Authentication/Provider/Cookie.php @@ -47,7 +47,8 @@ public function applies(Request $request) { public function authenticate(Request $request) { // Global $user is deprecated, but the session system is still based on it. global $user; - if ($this->sessionManager->initialize()->isStarted()) { + $this->sessionManager->startLazy(); + if ($this->sessionManager->isStarted()) { return $user; } return NULL; diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php index 49a4d02144ba..6f2e398ab1de 100644 --- a/core/lib/Drupal/Core/DrupalKernel.php +++ b/core/lib/Drupal/Core/DrupalKernel.php @@ -684,6 +684,7 @@ protected function initializeContainer($rebuild = FALSE) { // potentially scoped. $request_scope = FALSE; $request_stack = $request = NULL; + $session_manager_state = 0; if (isset($this->container)) { if ($this->container->isScopeActive('request')) { $request_scope = TRUE; @@ -694,6 +695,18 @@ protected function initializeContainer($rebuild = FALSE) { if ($this->container->initialized('request_stack')) { $request_stack = $this->container->get('request_stack'); } + // If there is a session manager, close and save the session. + if ($this->container->initialized('session_manager')) { + $session_manager = $this->container->get('session_manager'); + if ($session_manager->isStartedLazy()) { + $session_manager_state |= 0x1; + } + if ($session_manager->isStarted()) { + $session_manager_state |= 0x2; + } + $session_manager->save(); + unset($session_manager); + } } // If the module list hasn't already been set in updateModules and we are @@ -720,6 +733,12 @@ protected function initializeContainer($rebuild = FALSE) { $this->attachSynthetic($container, $request, $request_stack, $request_scope); $this->container = $container; + if ($session_manager_state & 0x1) { + $this->container->get('session_manager')->startLazy(); + } + if ($session_manager_state & 0x2) { + $this->container->get('session_manager')->start(); + } \Drupal::setContainer($this->container); return $this->container; } diff --git a/core/lib/Drupal/Core/Session/SessionManager.php b/core/lib/Drupal/Core/Session/SessionManager.php index f656b2d7d83a..e8dc572c6cde 100644 --- a/core/lib/Drupal/Core/Session/SessionManager.php +++ b/core/lib/Drupal/Core/Session/SessionManager.php @@ -63,7 +63,7 @@ class SessionManager extends NativeSessionStorage implements SessionManagerInter * * @var bool */ - protected $lazySession; + protected $startedLazy; /** * Whether session management is enabled or temporarily disabled. @@ -89,10 +89,18 @@ class SessionManager extends NativeSessionStorage implements SessionManagerInter * The settings instance. */ public function __construct(RequestStack $request_stack, Connection $connection, SymfonyMetadataBag $metadata_bag, Settings $settings) { - parent::__construct(); + $options = array(); + $this->requestStack = $request_stack; $this->connection = $connection; - $this->setMetadataBag($metadata_bag); + + // Register the default session handler. + // @todo Extract session storage from session handler into a service. + $save_handler = new SessionHandler($this, $this->requestStack, $this->connection); + $write_check_handler = new WriteCheckSessionHandler($save_handler); + $this->setSaveHandler($write_check_handler); + + parent::__construct($options, $write_check_handler, $metadata_bag); $this->setMixedMode($settings->get('mixed_mode_sessions', FALSE)); @@ -108,14 +116,12 @@ public function __construct(RequestStack $request_stack, Connection $connection, /** * {@inheritdoc} */ - public function initialize() { + public function startLazy() { global $user; - // Register the default session handler. - // @todo Extract session storage from session handler into a service. - $save_handler = new SessionHandler($this, $this->requestStack, $this->connection); - $write_check_handler = new WriteCheckSessionHandler($save_handler); - $this->setSaveHandler($write_check_handler); + if (($this->started || $this->startedLazy) && !$this->closed) { + return $this->started; + } $is_https = $this->requestStack->getCurrentRequest()->isSecure(); $cookies = $this->requestStack->getCurrentRequest()->cookies; @@ -125,7 +131,7 @@ public function initialize() { // session is only started on demand in save(), making // anonymous users not use a session cookie unless something is stored in // $_SESSION. This allows HTTP proxies to cache anonymous pageviews. - $this->start(); + $result = $this->start(); if ($user->isAuthenticated() || !$this->isSessionObsolete()) { drupal_page_is_cacheable(FALSE); } @@ -135,17 +141,26 @@ public function initialize() { // we lazily start sessions at the end of this request, and some // processes (like drupal_get_token()) needs to know the future // session ID in advance. - $this->lazySession = TRUE; $user = new AnonymousUserSession(); $this->setId(Crypt::randomBytesBase64()); if ($is_https && $this->isMixedMode()) { $session_id = Crypt::randomBytesBase64(); $cookies->set($insecure_session_name, $session_id); } + $result = FALSE; } date_default_timezone_set(drupal_get_user_timezone()); - return $this; + $this->startedLazy = TRUE; + + return $result; + } + + /** + * {@inheritdoc} + */ + public function isStartedLazy() { + return $this->startedLazy; } /** @@ -174,7 +189,7 @@ public function start() { public function save() { global $user; - if (!$this->isEnabled()) { + if (!$this->isEnabled() || $this->isCli()) { // We don't have anything to do if we are not allowed to save the session. return; } @@ -189,7 +204,7 @@ public function save() { else { // There is session data to store. Start the session if it is not already // started. - if (!$this->isStarted()) { + if (!$this->getSaveHandler()->isActive()) { $this->start(); if ($this->requestStack->getCurrentRequest()->isSecure() && $this->isMixedMode()) { $insecure_session_name = $this->getInsecureName(); @@ -202,6 +217,8 @@ public function save() { // Write the session data. parent::save(); } + + $this->startedLazy = FALSE; } /** @@ -211,7 +228,7 @@ public function regenerate($destroy = FALSE, $lifetime = NULL) { global $user; // Nothing to do if we are not allowed to change the session. - if (!$this->isEnabled()) { + if (!$this->isEnabled() || $this->isCli()) { return; } @@ -226,7 +243,7 @@ public function regenerate($destroy = FALSE, $lifetime = NULL) { if ($is_https && $this->isMixedMode()) { $insecure_session_name = $this->getInsecureName(); - if (!isset($this->lazySession) && $cookies->has($insecure_session_name)) { + if ($this->isStarted() && $cookies->has($insecure_session_name)) { $old_insecure_session_id = $cookies->get($insecure_session_name); } $params = session_get_cookie_params(); @@ -294,7 +311,7 @@ public function regenerate($destroy = FALSE, $lifetime = NULL) { */ public function delete($uid) { // Nothing to do if we are not allowed to change the session. - if (!$this->isEnabled()) { + if (!$this->isEnabled() || $this->isCli()) { return; } $this->connection->delete('sessions') diff --git a/core/lib/Drupal/Core/Session/SessionManagerInterface.php b/core/lib/Drupal/Core/Session/SessionManagerInterface.php index dc9aa4050ad2..e7ea14d3f58f 100644 --- a/core/lib/Drupal/Core/Session/SessionManagerInterface.php +++ b/core/lib/Drupal/Core/Session/SessionManagerInterface.php @@ -15,11 +15,20 @@ interface SessionManagerInterface extends SessionStorageInterface { /** - * Initializes the session handler, starting a session if needed. + * Starts a session if appropriate cookies are on the request. * - * @return $this + * @return bool + * TRUE if the session was started. + */ + public function startLazy(); + + /** + * Determines whether the session was started lazily. + * + * @return bool + * TRUE if the session was started lazily. */ - public function initialize(); + public function isStartedLazy(); /** * Ends a specific user's session(s). diff --git a/core/update.php b/core/update.php index c7122d0035d6..f2be90de417f 100644 --- a/core/update.php +++ b/core/update.php @@ -323,7 +323,7 @@ function update_task_list($active = NULL) { $kernel->prepareLegacyRequest($request); // Determine if the current user has access to run update.php. -\Drupal::service('session_manager')->initialize(); +\Drupal::service('session_manager')->startLazy(); // Ensure that URLs generated for the home and admin pages don't have 'update.php' // in them. -- GitLab