The session is not being updated.

Created on 17 August 2024, about 1 year ago

Problem/Motivation

The issue that is happening is that, after updating to Drupal 10.3.0, it was noticed that the session is not being updated. So, if I set the session to 30 minutes, after the user is inactive on the site for that time, they are logged out. In Drupal 10.2.7, every time there was a request, this time was recalculated and updated.

Steps to reproduce

Create the services.yml file and change gc_maxlifetime to 120. Also, change cookie_lifetime to 120. You will be able to use the site normally, but you will be logged out after 2 minutes.

parameters:
  session.storage.options:
    cookie_samesite: Lax
    gc_maxlifetime: 120
    cookie_lifetime: 120

Proposed resolution

I think the issue is happening because it's not saving the new session.
Moreover, even forcing it to enter the if, it doesn't work.

https://git.drupalcode.org/project/drupal/-/blob/10.3.0/core/lib/Drupal/...

🐛 Bug report
Status

Needs work

Version

10.3

Component
BigPipe 

Last updated about 1 month ago

Created by

🇧🇷Brazil charlliequadros

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @charlliequadros
  • 🇧🇷Brazil charlliequadros

    I created an MR. I'm not sure if it's the best solution, but it solves the problem until someone can fix it properly. I'm available to work on this ticket if anyone can help me figure out a permanent solution to the issue.

  • Pipeline finished with Failed
    about 1 year ago
    Total: 707s
    #256382
  • Pipeline finished with Failed
    about 1 year ago
    Total: 741s
    #256381
  • My expectation is the opposite—the session should end.

  • 🇧🇷Brazil rafael maito

    Thank you, charlliequadros. This solution worked for me, adding this change the session is being updated in each request.
    Hi cilefen, Is your session never ending?

  • The session should end after gc_maxlifetime. No?

  • 🇧🇷Brazil charlliequadros

    Hi @cilefen

    In Drupal version 10.2.7, the user's session was updated with each request, meaning that as long as the user interacted with the site, the session time was refreshed, preventing it from expiring during use. However, after updating to version 10.3.0, this behavior changed: the session time is no longer updated. So, if the timeout is reached, the session will be automatically closed, regardless of what the user is doing, such as during content creation or editing, for example. What's not working after the update is the session time being refreshed while the user is interacting with the site.

  • As I mentioned my expectation is the opposite of yours.

    Anyway, perform a git bisect on a Git working copy of Drupal core to understand which commit changed this.

  • 🇧🇷Brazil charlliequadros

    Hi @cilefen,

    I followed your suggestion to identify when the issue was introduced.

    This issue arose during the resolution of this issue. https://www.drupal.org/project/drupal/issues/3414287 📌 Avoid reading session from the database multiple times during a request Needs work

    I couldn’t determine which call was removed to avoid two database requests. Was it "$request->getSession()->save();" or "$this->session->start();

  • 🇧🇷Brazil rafael maito

    Hello charlliequadros and cilefin,

    After some investigation, here is my conclusion.

    This change is related to the big pipe, here ( https://www.drupal.org/project/drupal/issues/3414287 📌 Avoid reading session from the database multiple times during a request Needs work ) is the Drupal issue. So checking this, the changes between Drupal versions 10.2.7 and 10.3.0 in BigPipe were changes to implement enhance performance and security in the delivery of dynamic content on web pages. However, might be affecting how the session is managed, particularly concerning the SSESS cookie's update timing.

    In Drupal 10.2.7, BigPipe handled session management by reopening the session during the rendering of placeholders and then closing it afterward. This ensured that the session's expiration time was updated as content was progressively delivered to the user. However, in Drupal 10.3.0, the changes of these pre- and post-rendering tasks may be disrupting the regular update process of the SSESS cookie.

    These changes could be preventing the session from being reopened or correctly managed during placeholder rendering, leading to issues with the SSESS cookie's expiration time.

    If I revert the changes made here (https://git.drupalcode.org/project/drupal/-/merge_requests/6441/diffs#6c...) in the files BigPipe.php and BigPipeResponse.php, it works as before.

  • 🇧🇷Brazil isa.bel Balneário Camboriú
  • 🇳🇿New Zealand quietone

    stack used:
    - domain module (with domain_entity on) https://www.drupal.org/project/domain
    - drupal/jsonapi_extras https://www.drupal.org/project/jsonapi_extras
    - webforms
    - Webforms Rest https://www.drupal.org/project/webform_rest

    and when I call any REST endpoint, for example webform_rest//fields?_format=json
    I get "No route found for the specified format html. Supported formats: json, XML."

    The stack looks like below

    array(
        0 => array(
            'file' => '.\\web\\core\\lib\\Drupal\\Core\\Routing\\Router.php',
            'line' => 280,
            'function' => 'filter',
            'class' => 'Drupal\\Core\\Routing\\RequestFormatRouteFilter',
            'type' => '->',
        ),
        1 => array(
            'file' => '.\\web\\core\\lib\\Drupal\\Core\\Routing\\Router.php',
            'line' => 126,
            'function' => 'applyRouteFilters',
            'class' => 'Drupal\\Core\\Routing\\Router',
            'type' => '->',
        ),
        2 => array(
            'file' => '.\\web\\core\\lib\\Drupal\\Core\\Routing\\Router.php',
            'line' => 103,
            'function' => 'matchRequest',
            'class' => 'Drupal\\Core\\Routing\\Router',
            'type' => '->',
        ),
        3 => array(
            'file' => '.\\web\\core\\lib\\Drupal\\Core\\Path\\PathValidator.php',
            'line' => 161,
            'function' => 'match',
            'class' => 'Drupal\\Core\\Routing\\Router',
            'type' => '->',
        ),
        4 => array(
            'file' => '.\\web\\core\\lib\\Drupal\\Core\\Path\\PathValidator.php',
            'line' => 122,
            'function' => 'getPathAttributes',
            'class' => 'Drupal\\Core\\Path\\PathValidator',
            'type' => '->',
        ),
        5 => array(
            'file' => '.\\web\\core\\lib\\Drupal\\Core\\Path\\PathValidator.php',
            'line' => 89,
            'function' => 'getUrl',
            'class' => 'Drupal\\Core\\Path\\PathValidator',
            'type' => '->',
        ),
        6 => array(
            'file' => '.\\web\\core\\lib\\Drupal\\Core\\Url.php',
            'line' => 427,
            'function' => 'getUrlIfValidWithoutAccessCheck',
            'class' => 'Drupal\\Core\\Path\\PathValidator',
            'type' => '->',
        ),
        7 => array(
            'file' => '.\\web\\core\\lib\\Drupal\\Core\\Url.php',
            'line' => 319,
            'function' => 'fromInternalUri',
            'class' => 'Drupal\\Core\\Url',
            'type' => '::',
        ),
        8 => array(
            'file' => '.\\web\\core\\lib\\Drupal\\Core\\Url.php',
            'line' => 221,
            'function' => 'fromUri',
            'class' => 'Drupal\\Core\\Url',
            'type' => '::',
        ),
        9 => array(
            'file' => '.\\web\\modules\\contrib\\domain_entity\\src\\HttpKernel\\DomainEntitySourcePathProcessor.php',
            'line' => 147,
            'function' => 'fromUserInput',
            'class' => 'Drupal\\Core\\Url',
            'type' => '::',
        ),
        10 => array(
            'file' => '.\\web\\core\\lib\\Drupal\\Core\\PathProcessor\\PathProcessorManager.php',
            'line' => 108,
            'function' => 'processOutbound',
            'class' => 'Drupal\\domain_entity\\HttpKernel\\DomainEntitySourcePathProcessor',
            'type' => '->',
        ),
        11 => array(
            'file' => '.\\web\\core\\lib\\Drupal\\Core\\Routing\\UrlGenerator.php',
            'line' => 388,
            'function' => 'processOutbound',
            'class' => 'Drupal\\Core\\PathProcessor\\PathProcessorManager',
            'type' => '->',
        ),
        12 => array(
            'file' => '.\\web\\core\\lib\\Drupal\\Core\\Routing\\UrlGenerator.php',
            'line' => 297,
            'function' => 'processPath',
            'class' => 'Drupal\\Core\\Routing\\UrlGenerator',
            'type' => '->',
        ),
        13 => array(
            'file' => '.\\web\\core\\lib\\Drupal\\Core\\Render\\MetadataBubblingUrlGenerator.php',
            'line' => 105,
            'function' => 'generateFromRoute',
            'class' => 'Drupal\\Core\\Routing\\UrlGenerator',
    

    As you see DomainEntitySourcePathProcessor.php invokes Url::fromUserInput() and what he tries to do is basically detect if user has access to access this route in terms of Domain Entity logic. If for example I instantly return $path value form DomainEntitySourcePathProcessor::processOutbound() not doing any logic - then all works, no error. But I cant do that, the logic must happen.

    And what happens later is that Symfony\Component\HttpFoundation\Request is recreated multiple times. But its constructor or factories does not care about _format value passed in any manner, it must be manually set, otherwise Request::duplicate() method must be used.

    I have examined all the chain few times (this problem I observe for 10 months and its time to finally solve it, client needs that).
    And my suggestion is to do change in web\core\lib\Drupal\Core\Routing\Router.php

      public function match($pathinfo): array {
        $request = Request::create($pathinfo);
        $request->getRequestFormat(\Drupal::request()->getRequestFormat()); // ADD THIS LINE
    

    works good, solves the problem and nothing broken.

    I will publish the patch soon.

    P.S. I have a feeling like the problem should be solved on some level higher, maybe where getUrlIfValidWithoutAccessCheck() should exist option for full path with ?query support in URL or maybe version with full request. But I cant formulate that right now

  • 🇬🇧United Kingdom catch
  • 🇬🇧United Kingdom catch

    The session should never be written on every single request, we have specific logic to prevent this

    See WriteSafeSessionHandler::write().

    What behaviour do you get with big pipe disabled - e.g. does the session timestamp get updated there when it doesn't for big_pipe?

    Testing locally with big_pipe enabled, when I first visit the site, my session timestamp gets update, then I click around a bit, and it doesn't get updated for a while, but then if I try again ~6-7 minutes later, the timestamp gets updated again. This is what I'd expect to happen.

    This should be the same with or without big_pipe too - or at least that was the intention of 📌 Avoid reading session from the database multiple times during a request Needs work .

    It's possible there's a bug somewhere, but please clarify the steps to reproduce if you can - the session should be updated every few minutes, not every request.

  • Status changed to Postponed: needs info 8 months ago
  • 🇩🇪Germany jan kellermann

    The same topic is discussed in #2925718. But it seems, that D11 has solved the problem if session_write_interval is set correctly.

  • 🇨🇭Switzerland znerol

    I tried to reproduce the issue with the following setup on 11.x:

    • Standard install profile.
    • Create a new user without admin privileges.
    • Login with the new user.
    • Repeatedly reload a page which triggers big pipe (e.g. /contact).

    In order to make the manual testing reproducible, I choose the following options for PHP session gc and Symfony session metadata_update_threshold (the latter is called session_write_interval in Drupal and it is a setting):

    In sites/default/services.yml:

    parameters:
      session.storage.options:
        gc_divisor: 1
        gc_maxlifetime: 30
        gc_probability: 1
    

    In sites/default/settings.php:

    $settings['session_write_interval'] = 20;
    

    The effect of the PHP gc_divisor: 1 and gc_probability: 1 is that gc is run on every request before any session data is read from the database. That means that any session record with a timestamp older than 30 seconds is removed at the start of ever request (except for requests answered by the page cache of course).

    The effect of the metadata threshold is that a session write is enforced for sessions with a timestamp older than 20 seconds.

    With this setup I can confirm that every request which hits the window between the metadata threshold (session_write_interval and the gc max lifetime will bump the effective session lifetime by 30 seconds (gc_maxlifetime). This is expected behavior.

    I can confirm that a session is removed and a user is logged out when I leave a tab open for more than 30 seconds (gc_maxlifetime) before I reload the browser (though, see the caveat at the end). That is expected behavior as well.

    In my opinion this issue can be closed. I believe that the behavior observed by the OP is a result of a gc_maxlifetime value lower/equal than session_write_interval. If there is no window between the two values, the session timestamp is only updated when new session data is written. In cases where a very short session lifetime is required, it is probably best to set session_write_interval=0. With that setting, sessions will be written on every request - no matter whether the session data has changed or not.

    For the latter test, I discovered an interesting edge case. For the test to pass there needs to be other activity which triggers the gc in between the requests in the tab left open. If the session record is still in the database when the request starts, then the session record is removed from the database (which is expected), but it is written at the end again and as a result survives the timeout (which is unexpected).

    I'm unsure whether or not this edge case needs a fix. With the probabilistic gc in use today, sites cannot rely on session records being deleted reliably after surpassing the gc max lifetime anyway.

  • 🇨🇭Switzerland znerol

    I realized, that the OP reported something different:

    In 10.2.x, the session cookie is updated if necessary on every request (not yet sure whether this is done by PHP, Symfony or Drupal).

    After 📌 Avoid reading session from the database multiple times during a request Needs work , the call to \Symfony\Component\HttpFoundation\Session\Session::save() is too late to update the session cookie on big pipe responses. Response headers are already sent when big pipe performPostSendTasks() is called. I can reproduce this behavior by adding a cookie_lifetime: 30 container parameter (and still using session_write_interval: 20).

    We could maybe fix that by adding more logic to \Drupal\Core\StackMiddleware\Session::handle(). E.g., trigger a session save unconditionally when the session timestamp is older than session_write_interval.

  • 🇨🇭Switzerland znerol

    This is a very weird issue. These are my observations so far:

    Before 📌 Avoid reading session from the database multiple times during a request Needs work (git switch --detach 4e26ae9cc3da8b8a90561716c4a53fdd6c07f8f6):

    • Session cookies are refreshed when big pipe module is enabled (e.g., standard install)
    • Session cookies are not refreshed when big pipe module is not enabled (e.g., minimal install)

    After 📌 Avoid reading session from the database multiple times during a request Needs work (git switch --detach 522404706e440106fe7e5d382ce018b0f89637d4):

    • Session cookies are not refreshed when big pipe module is enabled (e.g., standard install)
    • Session cookies are not refreshed when big pipe module is not enabled (e.g., minimal install)

    At least this is now consistently working as expected by @cilefen.

  • 🇬🇧United Kingdom AndyF

    I think I might be running into this. I was using a very low gc_maxlifetime and gc_divisor set to 1 to test the behavior of an app with a webview when the session expires, and was surprised that no amount of navigating the site from a fresh cache clear could stop me getting logged out after the time has passed.

    The session should end after gc_maxlifetime. No?

    I think the unexpected behavior is that the session ends gc_maxlifetime after the session is created, regardless of how much you use the site within that period (rather than ending a minimum of gc_maxlifetime after the last access).

  • 🇨🇭Switzerland znerol

    My observations suggest that the behavior is as follows:

    • The timestamp column on the session record in the database is refreshed when the site is used (provided that the user hits an uncached route from time to time and provided that this happens in a time window between $settings['session_write_interval'] and gc_maxlifetime #17
    • The expiry date on the cookie is never refreshed. #18 and #19.

    If that is true, then I'd recommend to keep cookie_lifetime values high and only reduce the parameters which govern the session garbage collection for sites which require expiration of idle sessions.

  • 🇬🇧United Kingdom AndyF

    I was just trying to edit my comment to say I didn't have a sufficiently low settings['session_write_interval'], but the update wouldn't submit...

    Thanks @znerol for that and your other comments, they'd already pointed me in the right direction. (Although surprisingly with a setup like in #17 I'm finding I'm still logged-in after way more than 30s, will keep playing.)

  • 🇨🇭Switzerland znerol

    (Although surprisingly with a setup like in #17 I'm finding I'm still logged-in after way more than 30s, will keep playing.)

    You might be running into the edge case documented near the end of #17. If there is only activity in one tab from one user, then the session is not garbage collected.

  • 🇬🇧United Kingdom catch

    Just a note related to #19 that since 📌 Render the navigation toolbar in a placeholder Active , big pipe previously would have been active for pretty much any request with a session, but it now won't be with warm caches. It doesn't sound like this will make any difference at all here given that big pipe is no longer accidentally mitigating the issue anyway.

Production build 0.71.5 2024