From 57ce2e37045e11e1a265167aa944cca36ca0503f Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Thu, 21 Mar 2024 08:45:38 +0000 Subject: [PATCH] Issue #3413153 by znerol, alexpott: Remove calls to Request::hasSession() --- core/includes/install.core.inc | 15 +++------------ .../Core/Cache/Context/SessionCacheContext.php | 6 +----- core/lib/Drupal/Core/DrupalKernel.php | 8 ++------ .../Entity/EntityDeleteMultipleAccessCheck.php | 3 --- core/lib/Drupal/Core/Form/FormBuilder.php | 3 +-- core/lib/Drupal/Core/StackMiddleware/Session.php | 2 +- .../Drupal/Core/TempStore/PrivateTempStore.php | 14 +++----------- .../lib/Drupal/Core/TempStore/SharedTempStore.php | 6 ++---- .../Core/TempStore/SharedTempStoreFactory.php | 7 +------ core/lib/Drupal/Core/Update/UpdateKernel.php | 4 +--- .../big_pipe/src/Controller/BigPipeController.php | 2 +- .../comment/src/Controller/CommentController.php | 4 +--- .../user/src/Authentication/Provider/Cookie.php | 4 ++-- core/modules/views/src/ViewExecutable.php | 2 +- core/modules/views_ui/src/ViewUI.php | 1 + .../TempStore/AnonymousPrivateTempStoreTest.php | 7 ------- .../Cache/Context/SessionCacheContextTest.php | 9 --------- .../Drupal/Tests/Core/Form/FormBuilderTest.php | 1 + 18 files changed, 22 insertions(+), 76 deletions(-) diff --git a/core/includes/install.core.inc b/core/includes/install.core.inc index bca69034fa86..5dd68cd4d5af 100644 --- a/core/includes/install.core.inc +++ b/core/includes/install.core.inc @@ -147,10 +147,7 @@ function install_drupal($class_loader, $settings = [], callable $callback = NULL // installations can send output to the browser or redirect the user to the // next page. if ($state['interactive']) { - // If a session has been initiated in this request, make sure to save it. - if (\Drupal::request()->hasSession()) { - \Drupal::request()->getSession()->save(); - } + \Drupal::request()->getSession()->save(); if ($state['parameters_changed']) { // Redirect to the correct page if the URL parameters have changed. install_goto(install_redirect_url($state)); @@ -645,10 +642,7 @@ function install_run_task($task, &$install_state) { $url = Url::fromUri('base:install.php', ['query' => $install_state['parameters'], 'script' => '']); $response = batch_process($url, clone $url); if ($response instanceof Response) { - if (\Drupal::request()->hasSession()) { - \Drupal::request()->getSession()->save(); - } - // Send the response. + \Drupal::request()->getSession()->save(); $response->send(); exit; } @@ -663,10 +657,7 @@ function install_run_task($task, &$install_state) { // @todo Replace this when we refactor the installer to use a request- // response workflow. if ($output instanceof Response) { - if (\Drupal::request()->hasSession()) { - \Drupal::request()->getSession()->save(); - } - // Send the response. + \Drupal::request()->getSession()->save(); $output->send(); exit; } diff --git a/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php b/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php index bccd0f01a469..c3af91fef32f 100644 --- a/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php +++ b/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php @@ -22,11 +22,7 @@ public static function getLabel() { * {@inheritdoc} */ public function getContext() { - $request = $this->requestStack->getCurrentRequest(); - if ($request->hasSession()) { - return Crypt::hashBase64($request->getSession()->getId()); - } - return 'none'; + return Crypt::hashBase64($this->requestStack->getSession()->getId()); } } diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php index 9c4f2fd68692..1df4fc273b7f 100644 --- a/core/lib/Drupal/Core/DrupalKernel.php +++ b/core/lib/Drupal/Core/DrupalKernel.php @@ -999,9 +999,7 @@ protected function initializeContainer() { if (($request_stack = $this->container->get('request_stack', ContainerInterface::NULL_ON_INVALID_REFERENCE))) { if ($request = $request_stack->getMainRequest()) { $subrequest = TRUE; - if ($request->hasSession()) { - $request->setSession($this->container->get('session')); - } + $request->setSession($this->container->get('session')); } } @@ -1271,9 +1269,7 @@ public function resetContainer(): ContainerInterface { if (($request_stack = $this->container->get('request_stack', ContainerInterface::NULL_ON_INVALID_REFERENCE))) { if ($request = $request_stack->getMainRequest()) { $subrequest = TRUE; - if ($request->hasSession()) { - $request->setSession($this->container->get('session')); - } + $request->setSession($this->container->get('session')); } } diff --git a/core/lib/Drupal/Core/Entity/EntityDeleteMultipleAccessCheck.php b/core/lib/Drupal/Core/Entity/EntityDeleteMultipleAccessCheck.php index dc5007a3b528..fd0d40025bcf 100644 --- a/core/lib/Drupal/Core/Entity/EntityDeleteMultipleAccessCheck.php +++ b/core/lib/Drupal/Core/Entity/EntityDeleteMultipleAccessCheck.php @@ -62,9 +62,6 @@ public function __construct(EntityTypeManagerInterface $entity_type_manager, Pri * Allowed or forbidden, neutral if tempstore is empty. */ public function access(AccountInterface $account, $entity_type_id) { - if (!$this->requestStack->getCurrentRequest()->hasSession()) { - return AccessResult::neutral(); - } $selection = $this->tempStore->get($account->id() . ':' . $entity_type_id); if (empty($selection) || !is_array($selection)) { return AccessResult::neutral(); diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php index 2331a59e1f40..2a127b7f19e5 100644 --- a/core/lib/Drupal/Core/Form/FormBuilder.php +++ b/core/lib/Drupal/Core/Form/FormBuilder.php @@ -251,8 +251,7 @@ public function buildForm($form_arg, FormStateInterface &$form_state) { $form_state->setUserInput($input); } - // @todo Remove hasSession() condition in https://www.drupal.org/i/3413153 - if ($request->hasSession() && $request->getSession()->has('batch_form_state')) { + if ($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(). $session = $request->getSession(); diff --git a/core/lib/Drupal/Core/StackMiddleware/Session.php b/core/lib/Drupal/Core/StackMiddleware/Session.php index c7778cb2dcd6..2a3be0eea646 100644 --- a/core/lib/Drupal/Core/StackMiddleware/Session.php +++ b/core/lib/Drupal/Core/StackMiddleware/Session.php @@ -52,7 +52,7 @@ public function handle(Request $request, $type = self::MAIN_REQUEST, $catch = TR $result = $this->httpKernel->handle($request, $type, $catch); - if ($type === self::MAIN_REQUEST && !$result instanceof ResponseKeepSessionOpenInterface && PHP_SAPI !== 'cli' && $request->hasSession()) { + if ($type === self::MAIN_REQUEST && !$result instanceof ResponseKeepSessionOpenInterface && PHP_SAPI !== 'cli') { $request->getSession()->save(); } diff --git a/core/lib/Drupal/Core/TempStore/PrivateTempStore.php b/core/lib/Drupal/Core/TempStore/PrivateTempStore.php index 115ff35ddd8b..fa6270959c53 100644 --- a/core/lib/Drupal/Core/TempStore/PrivateTempStore.php +++ b/core/lib/Drupal/Core/TempStore/PrivateTempStore.php @@ -121,12 +121,7 @@ public function get($key) { */ public function set($key, $value) { if ($this->currentUser->isAnonymous()) { - // Ensure that an anonymous user has a session created for them, as - // otherwise subsequent page loads will not be able to retrieve their - // tempstore data. Note this has to be done before the key is created as - // the owner is used in key creation. - $this->startSession(); - $session = $this->requestStack->getCurrentRequest()->getSession(); + $session = $this->requestStack->getSession(); if (!$session->has('core.tempstore.private.owner')) { $session->set('core.tempstore.private.owner', Crypt::randomBytesBase64()); } @@ -225,8 +220,7 @@ protected function getOwner() { $owner = $this->currentUser->id(); if ($this->currentUser->isAnonymous()) { // Check to see if an owner key exists in the session. - $this->startSession(); - $session = $this->requestStack->getCurrentRequest()->getSession(); + $session = $this->requestStack->getSession(); $owner = $session->get('core.tempstore.private.owner'); } return $owner; @@ -238,11 +232,9 @@ protected function getOwner() { * Ensures that an anonymous user has a session created for them, as * otherwise subsequent page loads will not be able to retrieve their * tempstore data. - * - * @todo when https://www.drupal.org/node/2865991 is resolved, use force - * start session API. */ protected function startSession() { + @trigger_error(__METHOD__ . "() is deprecated in drupal:10.3.0 and is removed from drupal:11.0.0. There is no replacement. See https://www.drupal.org/node/3432359", E_USER_DEPRECATED); $has_session = $this->requestStack ->getCurrentRequest() ->hasSession(); diff --git a/core/lib/Drupal/Core/TempStore/SharedTempStore.php b/core/lib/Drupal/Core/TempStore/SharedTempStore.php index 47795183783e..fe51f933d048 100644 --- a/core/lib/Drupal/Core/TempStore/SharedTempStore.php +++ b/core/lib/Drupal/Core/TempStore/SharedTempStore.php @@ -299,10 +299,8 @@ public function deleteIfOwner($key) { * This method should be called when a value is set. */ protected function ensureAnonymousSession() { - // If this is being run from the CLI then the request will not have a - // session. - if ($this->currentUser->isAnonymous() && $this->requestStack->getCurrentRequest()->hasSession()) { - $this->requestStack->getCurrentRequest()->getSession()->set('core.tempstore.shared.owner', $this->owner); + if ($this->currentUser->isAnonymous()) { + $this->requestStack->getSession()->set('core.tempstore.shared.owner', $this->owner); } } diff --git a/core/lib/Drupal/Core/TempStore/SharedTempStoreFactory.php b/core/lib/Drupal/Core/TempStore/SharedTempStoreFactory.php index c206585f6acb..58d01a2e5574 100644 --- a/core/lib/Drupal/Core/TempStore/SharedTempStoreFactory.php +++ b/core/lib/Drupal/Core/TempStore/SharedTempStoreFactory.php @@ -90,12 +90,7 @@ public function get($collection, $owner = NULL) { if (!isset($owner)) { $owner = $this->currentUser->id(); if ($this->currentUser->isAnonymous()) { - $owner = Crypt::randomBytesBase64(); - if ($this->requestStack->getCurrentRequest()->hasSession()) { - // Store a random identifier for anonymous users if the session is - // available. - $owner = $this->requestStack->getCurrentRequest()->getSession()->get('core.tempstore.shared.owner', $owner); - } + $owner = $this->requestStack->getSession()->get('core.tempstore.shared.owner', Crypt::randomBytesBase64()); } } diff --git a/core/lib/Drupal/Core/Update/UpdateKernel.php b/core/lib/Drupal/Core/Update/UpdateKernel.php index 7f2036534253..9b51f9069079 100644 --- a/core/lib/Drupal/Core/Update/UpdateKernel.php +++ b/core/lib/Drupal/Core/Update/UpdateKernel.php @@ -139,9 +139,7 @@ protected function bootSession(Request $request) { * The incoming request. */ protected function shutdownSession(Request $request) { - if ($request->hasSession()) { - $request->getSession()->save(); - } + $request->getSession()->save(); } /** diff --git a/core/modules/big_pipe/src/Controller/BigPipeController.php b/core/modules/big_pipe/src/Controller/BigPipeController.php index da95f6d9dce2..ea8b08ab47e9 100644 --- a/core/modules/big_pipe/src/Controller/BigPipeController.php +++ b/core/modules/big_pipe/src/Controller/BigPipeController.php @@ -42,7 +42,7 @@ public function setNoJsCookie(Request $request) { // the cookie was already set, yet the user is executing this controller; // - there is no session, in which case BigPipe is not enabled anyway, so it // is pointless to set this cookie. - if ($request->cookies->has(BigPipeStrategy::NOJS_COOKIE) || !$request->hasSession()) { + if ($request->cookies->has(BigPipeStrategy::NOJS_COOKIE)) { throw new AccessDeniedHttpException(); } diff --git a/core/modules/comment/src/Controller/CommentController.php b/core/modules/comment/src/Controller/CommentController.php index 14a62922e17d..6f9a0f9f1533 100644 --- a/core/modules/comment/src/Controller/CommentController.php +++ b/core/modules/comment/src/Controller/CommentController.php @@ -131,9 +131,7 @@ public function commentPermalink(Request $request, CommentInterface $comment) { $subrequest_url = $entity->toUrl()->setOption('query', ['page' => $page])->toString(TRUE); $redirect_request = Request::create($subrequest_url->getGeneratedUrl(), 'GET', $request->query->all(), $request->cookies->all(), [], $request->server->all()); // Carry over the session to the subrequest. - if ($request->hasSession()) { - $redirect_request->setSession($request->getSession()); - } + $redirect_request->setSession($request->getSession()); $request->query->set('page', $page); $response = $this->httpKernel->handle($redirect_request, HttpKernelInterface::SUB_REQUEST); if ($response instanceof CacheableResponseInterface) { diff --git a/core/modules/user/src/Authentication/Provider/Cookie.php b/core/modules/user/src/Authentication/Provider/Cookie.php index 7c07c57729ec..fb2c9cd58455 100644 --- a/core/modules/user/src/Authentication/Provider/Cookie.php +++ b/core/modules/user/src/Authentication/Provider/Cookie.php @@ -66,7 +66,7 @@ public function __construct(SessionConfigurationInterface $session_configuration * {@inheritdoc} */ public function applies(Request $request) { - $applies = $request->hasSession() && $this->sessionConfiguration->hasSession($request); + $applies = $this->sessionConfiguration->hasSession($request); if (!$applies && $request->query->has('check_logged_in')) { $domain = ltrim(ini_get('session.cookie_domain'), '.') ?: $request->getHttpHost(); $this->messenger->addMessage($this->t('To log in to this site, your browser must accept cookies from the domain %domain.', ['%domain' => $domain]), 'error'); @@ -123,7 +123,7 @@ protected function getUserFromSession(SessionInterface $session) { */ public function addCheckToUrl(ResponseEvent $event) { $response = $event->getResponse(); - if ($response instanceof RedirectResponse && $event->getRequest()->hasSession()) { + if ($response instanceof RedirectResponse) { if ($event->getRequest()->getSession()->has('check_logged_in')) { $event->getRequest()->getSession()->remove('check_logged_in'); $url = $response->getTargetUrl(); diff --git a/core/modules/views/src/ViewExecutable.php b/core/modules/views/src/ViewExecutable.php index 24109a02cfe3..a7896469b77a 100644 --- a/core/modules/views/src/ViewExecutable.php +++ b/core/modules/views/src/ViewExecutable.php @@ -720,7 +720,7 @@ public function getExposedInput() { } // If we have no input at all, check for remembered input via session. - if (empty($this->exposed_input) && $this->request->hasSession()) { + if (empty($this->exposed_input)) { $session = $this->request->getSession(); // If filters are not overridden, store the 'remember' settings on the // default display. If they are, store them on this display. This way, diff --git a/core/modules/views_ui/src/ViewUI.php b/core/modules/views_ui/src/ViewUI.php index 65983ad37301..bdf057db31e6 100644 --- a/core/modules/views_ui/src/ViewUI.php +++ b/core/modules/views_ui/src/ViewUI.php @@ -592,6 +592,7 @@ public function renderPreview($display_id, $args = []) { foreach ($args as $key => $arg) { $request->attributes->set('arg_' . $key, $arg); } + $request->setSession($request_stack->getSession()); $request_stack->push($request); // Suppress contextual links of entities within the result set during a diff --git a/core/tests/Drupal/KernelTests/Core/TempStore/AnonymousPrivateTempStoreTest.php b/core/tests/Drupal/KernelTests/Core/TempStore/AnonymousPrivateTempStoreTest.php index 9820ff89d0f0..d7d12f4e2296 100644 --- a/core/tests/Drupal/KernelTests/Core/TempStore/AnonymousPrivateTempStoreTest.php +++ b/core/tests/Drupal/KernelTests/Core/TempStore/AnonymousPrivateTempStoreTest.php @@ -3,7 +3,6 @@ namespace Drupal\KernelTests\Core\TempStore; use Drupal\KernelTests\KernelTestBase; -use Symfony\Component\HttpFoundation\Request; /** * Tests the PrivateTempStore for anonymous users. @@ -31,12 +30,6 @@ class AnonymousPrivateTempStoreTest extends KernelTestBase { */ protected function setUp(): void { parent::setUp(); - - $request = Request::create('/'); - $stack = $this->container->get('request_stack'); - $stack->pop(); - $stack->push($request); - $this->tempStore = $this->container->get('tempstore.private')->get('anonymous_private_temp_store'); } diff --git a/core/tests/Drupal/Tests/Core/Cache/Context/SessionCacheContextTest.php b/core/tests/Drupal/Tests/Core/Cache/Context/SessionCacheContextTest.php index 87993d50f98f..5d2dce80a734 100644 --- a/core/tests/Drupal/Tests/Core/Cache/Context/SessionCacheContextTest.php +++ b/core/tests/Drupal/Tests/Core/Cache/Context/SessionCacheContextTest.php @@ -93,13 +93,4 @@ public function testDifferentContextForDifferentSession() { $this->assertStringNotContainsString($session2_id, $context2, 'Session ID not contained in cache context'); } - /** - * @covers ::getContext - */ - public function testContextWithoutSessionInRequest() { - $cache_context = new SessionCacheContext($this->requestStack); - - $this->assertSame('none', $cache_context->getContext()); - } - } diff --git a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php index e3bd606f94a9..34e16dd5bae2 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php @@ -626,6 +626,7 @@ public function testGetAjaxRequest(): void { $request = new Request([FormBuilderInterface::AJAX_FORM_REQUEST => TRUE]); $request->query->set('form_id', 'different_form_id'); $request->setMethod('GET'); + $request->setSession(new Session(new MockArraySessionStorage())); $this->requestStack->push($request); $form_state = (new FormState()) -- GitLab