From 9334d6f100cc5cf7e687f80dca122efb7438273b Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Mon, 18 Mar 2024 09:44:54 +0000
Subject: [PATCH] Issue #3109970 by znerol, andypost, Prashant.c, daffie, Wim
Leers, dww, catch: Convert uses of $_SESSION in forms and batch
---
core/includes/batch.inc | 16 ++++++++++------
core/includes/form.inc | 14 +++++++++-----
core/lib/Drupal/Core/Form/FormBuilder.php | 8 +++++---
.../tests/src/Functional/BigPipeTest.php | 15 +++++++++++++++
.../LanguageNegotiationSession.php | 4 +---
.../EventSubscriber/SessionTestSubscriber.php | 2 +-
.../tests/src/Functional/Session/SessionTest.php | 2 +-
.../Kernel/UserAccountFormPasswordResetTest.php | 9 +--------
.../Drupal/Tests/Core/Form/FormBuilderTest.php | 4 ++++
.../Drupal/Tests/Core/Form/FormTestBase.php | 3 +++
10 files changed, 50 insertions(+), 27 deletions(-)
diff --git a/core/includes/batch.inc b/core/includes/batch.inc
index b187dcd5170a..3c80ee1d1db8 100644
--- a/core/includes/batch.inc
+++ b/core/includes/batch.inc
@@ -474,11 +474,14 @@ function _batch_finished() {
}
}
// Clean-up the session. Not needed for CLI updates.
- if (isset($_SESSION)) {
- unset($_SESSION['batches'][$batch['id']]);
- if (empty($_SESSION['batches'])) {
- unset($_SESSION['batches']);
- }
+ $session = \Drupal::request()->getSession();
+ $batches = $session->get('batches', []);
+ unset($batches[$batch['id']]);
+ if (empty($batches)) {
+ $session->remove('batches');
+ }
+ else {
+ $session->set('batches', $batches);
}
}
$_batch = $batch;
@@ -531,7 +534,8 @@ function _batch_finished() {
// form needs to be rebuilt, save the final $form_state for
// \Drupal\Core\Form\FormBuilderInterface::buildForm().
if ($_batch['form_state']->isRebuilding()) {
- $_SESSION['batch_form_state'] = $_batch['form_state'];
+ $session = \Drupal::request()->getSession();
+ $session->set('batch_form_state', $_batch['form_state']);
}
$callback = $_batch['redirect_callback'];
$_batch['source_url']->mergeOptions(['query' => ['op' => 'finish', 'id' => $_batch['id']]]);
diff --git a/core/includes/form.inc b/core/includes/form.inc
index 80a04c2cbcec..6f1025e61f9e 100644
--- a/core/includes/form.inc
+++ b/core/includes/form.inc
@@ -612,7 +612,7 @@ function template_preprocess_form_element_label(&$variables) {
* // The following keys allow for multi-step operations :
* // 'sandbox' (read / write): An array that can be freely used to
* // store persistent data between iterations. It is recommended to
- * // use this instead of $_SESSION, which is unsafe if the user
+ * // use this instead of the session, which is unsafe if the user
* // continues browsing in a separate window while the batch is processing.
* // 'finished' (write): A float number between 0 and 1 informing
* // the processing engine of the completion level for the operation.
@@ -669,11 +669,12 @@ function template_preprocess_form_element_label(&$variables) {
* $message = t('Finished with an error.');
* }
* \Drupal::messenger()->addMessage($message);
- * // Providing data for the redirected page is done through $_SESSION.
+ * // Providing data for the redirected page is done through the session.
* foreach ($results as $result) {
* $items[] = t('Loaded node %title.', array('%title' => $result));
* }
- * $_SESSION['my_batch_results'] = $items;
+ * $session = \Drupal::getRequest()->getSession();
+ * $session->set('my_batch_results', $items);
* }
* @endcode
*/
@@ -718,7 +719,7 @@ function template_preprocess_form_element_label(&$variables) {
* - finished: Name of an implementation of callback_batch_finished(). This is
* executed after the batch has completed. This should be used to perform
* any result massaging that may be needed, and possibly save data in
- * $_SESSION for display after final page redirection.
+ * the session for display after final page redirection.
* - file: Path to the file containing the definitions of the 'operations' and
* 'finished' functions, for instance if they don't reside in the main
* .module file. The path should be relative to base_path(), and thus should
@@ -952,7 +953,10 @@ function batch_process($redirect = NULL, Url $url = NULL, $redirect_callback = N
\Drupal::service(BatchStorageInterface::class)->create($batch);
// Set the batch number in the session to guarantee that it will stay alive.
- $_SESSION['batches'][$batch['id']] = TRUE;
+ $session = \Drupal::request()->getSession();
+ $batches = $session->get('batches', []);
+ $batches[$batch['id']] = TRUE;
+ $session->set('batches', $batches);
// Redirect for processing.
$query_options = $error_url->getOption('query');
diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php
index a379036cb4f3..2331a59e1f40 100644
--- a/core/lib/Drupal/Core/Form/FormBuilder.php
+++ b/core/lib/Drupal/Core/Form/FormBuilder.php
@@ -251,11 +251,13 @@ public function buildForm($form_arg, FormStateInterface &$form_state) {
$form_state->setUserInput($input);
}
- if (isset($_SESSION['batch_form_state'])) {
+ // @todo Remove hasSession() condition in https://www.drupal.org/i/3413153
+ if ($request->hasSession() && $request->getSession()->has('batch_form_state')) {
// We've been redirected here after a batch processing. The form has
// already been processed, but needs to be rebuilt. See _batch_finished().
- $form_state = $_SESSION['batch_form_state'];
- unset($_SESSION['batch_form_state']);
+ $session = $request->getSession();
+ $form_state = $session->get('batch_form_state');
+ $session->remove('batch_form_state');
return $this->rebuildForm($form_id, $form_state);
}
diff --git a/core/modules/big_pipe/tests/src/Functional/BigPipeTest.php b/core/modules/big_pipe/tests/src/Functional/BigPipeTest.php
index 59681f05c063..0c55b3884864 100644
--- a/core/modules/big_pipe/tests/src/Functional/BigPipeTest.php
+++ b/core/modules/big_pipe/tests/src/Functional/BigPipeTest.php
@@ -431,8 +431,23 @@ protected function assertBigPipePlaceholders(array $expected_big_pipe_placeholde
* Ensures CSRF tokens can be generated for the current user's session.
*/
protected function setCsrfTokenSeedInTestEnvironment() {
+ // Retrieve the CSRF token from the child site from its serialized session
+ // record in the database.
$session_data = $this->container->get('session_handler.write_safe')->read($this->getSession()->getCookie($this->getSessionName()));
$csrf_token_seed = unserialize(explode('_sf2_meta|', $session_data)[1])['s'];
+
+ // Ensure that the session is started before accessing a session bag.
+ // Otherwise the value stored in the bag is lost when subsequent session
+ // access triggers a session start automatically.
+
+ /** @var \Symfony\Component\HttpFoundation\RequestStack $request_stack */
+ $request_stack = $this->container->get('request_stack');
+ $session = $request_stack->getSession();
+ if (!$session->isStarted()) {
+ $session->start();
+ }
+
+ // Store the CSRF token in the test runners session metadata bag.
$this->container->get('session_manager.metadata_bag')->setCsrfTokenSeed($csrf_token_seed);
}
diff --git a/core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationSession.php b/core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationSession.php
index b17dd9dfa11e..365cfced6f03 100644
--- a/core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationSession.php
+++ b/core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationSession.php
@@ -88,9 +88,7 @@ public function getLangcode(Request $request = NULL) {
if ($request->query->has($param)) {
return $request->query->get($param);
}
- // @todo Remove hasSession() from condition in
- // https://www.drupal.org/node/2484991
- if ($request->hasSession() && $request->getSession()->has($param)) {
+ if ($request->getSession()->has($param)) {
return $request->getSession()->get($param);
}
}
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 38b797a8a27d..35e491cfef1a 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
@@ -13,7 +13,7 @@
class SessionTestSubscriber implements EventSubscriberInterface {
/**
- * Stores whether $_SESSION is empty at the beginning of the request.
+ * Stores whether the session is empty at the beginning of the request.
*
* @var bool
*/
diff --git a/core/modules/system/tests/src/Functional/Session/SessionTest.php b/core/modules/system/tests/src/Functional/Session/SessionTest.php
index 27e36f770d41..fa3ad0419851 100644
--- a/core/modules/system/tests/src/Functional/Session/SessionTest.php
+++ b/core/modules/system/tests/src/Functional/Session/SessionTest.php
@@ -403,7 +403,7 @@ public function assertSessionCookie(bool $sent): void {
}
/**
- * Assert whether $_SESSION is empty at the beginning of the request.
+ * Assert whether the session is empty at the beginning of the request.
*
* @internal
*/
diff --git a/core/modules/user/tests/src/Kernel/UserAccountFormPasswordResetTest.php b/core/modules/user/tests/src/Kernel/UserAccountFormPasswordResetTest.php
index 95bf84056da7..d122ebfa37e6 100644
--- a/core/modules/user/tests/src/Kernel/UserAccountFormPasswordResetTest.php
+++ b/core/modules/user/tests/src/Kernel/UserAccountFormPasswordResetTest.php
@@ -4,7 +4,6 @@
use Drupal\KernelTests\KernelTestBase;
use Drupal\user\Entity\User;
-use Symfony\Component\HttpFoundation\Session\Session;
/**
* Verifies that the password reset behaves as expected with form elements.
@@ -53,14 +52,8 @@ public function testPasswordResetToken() {
/** @var \Symfony\Component\HttpFoundation\Request $request */
$request = $this->container->get('request_stack')->getCurrentRequest();
- // @todo: Replace with $request->getSession() as soon as the session is
- // present in KernelTestBase.
- // see: https://www.drupal.org/node/2484991
- $session = new Session();
- $request->setSession($session);
-
$token = 'VALID_TOKEN';
- $session->set('pass_reset_1', $token);
+ $request->getSession()->set('pass_reset_1', $token);
// Set token in query string.
$request->query->set('pass-reset-token', $token);
diff --git a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php
index f0b3425f5b86..e3bd606f94a9 100644
--- a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php
+++ b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php
@@ -22,6 +22,8 @@
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\HttpFoundation\Session\Session;
+use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
/**
* @coversDefaultClass \Drupal\Core\Form\FormBuilder
@@ -575,6 +577,7 @@ public function testFormCacheDeletionUncached() {
*/
public function testExceededFileSize() {
$request = new Request([FormBuilderInterface::AJAX_FORM_REQUEST => TRUE]);
+ $request->setSession(new Session(new MockArraySessionStorage()));
$request_stack = new RequestStack();
$request_stack->push($request);
$this->formBuilder = $this->getMockBuilder('\Drupal\Core\Form\FormBuilder')
@@ -598,6 +601,7 @@ public function testExceededFileSize() {
public function testPostAjaxRequest(): void {
$request = new Request([FormBuilderInterface::AJAX_FORM_REQUEST => TRUE], ['form_id' => 'different_form_id']);
$request->setMethod('POST');
+ $request->setSession(new Session(new MockArraySessionStorage()));
$this->requestStack->push($request);
$form_state = (new FormState())
diff --git a/core/tests/Drupal/Tests/Core/Form/FormTestBase.php b/core/tests/Drupal/Tests/Core/Form/FormTestBase.php
index 9de2c5e1ac99..53c96350e836 100644
--- a/core/tests/Drupal/Tests/Core/Form/FormTestBase.php
+++ b/core/tests/Drupal/Tests/Core/Form/FormTestBase.php
@@ -14,6 +14,8 @@
use Drupal\Tests\UnitTestCase;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\HttpFoundation\Session\Session;
+use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
/**
* Provides a base class for testing form functionality.
@@ -184,6 +186,7 @@ protected function setUp(): void {
$this->account = $this->createMock('Drupal\Core\Session\AccountInterface');
$this->themeManager = $this->createMock('Drupal\Core\Theme\ThemeManagerInterface');
$this->request = Request::createFromGlobals();
+ $this->request->setSession(new Session(new MockArraySessionStorage()));
$this->eventDispatcher = $this->createMock('Symfony\Contracts\EventDispatcher\EventDispatcherInterface');
$this->requestStack = new RequestStack();
$this->requestStack->push($this->request);
--
GitLab