AFAICT there is no API to start a session. The best way appears to be to store something in $_SESSION: see http://drupal.stackexchange.com/questions/163142/what-is-the-right-way-t...

But that means that every module that might want to ensure a session is started needs to store junk data in $_SESSION. This is not ideal (and indeed the patch at #2488116: Document the session subsystem says the amount of data in the session should be kept to a minimum).

There should instead be a forceStart() method on SessionManager.

Issue fork drupal-2865991

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm at OSInet for Carrefour commented

Actually, for the Flag use case in FlagService::populateFlaggerDefaults() where this "need" appears, creating the session for all anonymous visitors is a performance hog because, while it makes perfect sense to create sessions for anon users to keep track of their invididual likes; it means that the mere fact of visiting a page with a flag on a node causes the entire site visit after that not to remain anonymous and pierce caches, including proxies, because the session cookie is opaque.

The technique we've used on similar needs (keeping track of anonymous info) is to add a non-opaque cookie (with added checksum if security is needed), and only create it when data needs to be stored in it, preserving cacheability for other users.

joachim’s picture

joachim CreditAttribution: joachim commented

> Actually, for the Flag use case in FlagService::populateFlaggerDefaults() where this "need" appears, creating the session for all anonymous visitors is a performance hog because,

I've just had a look at the code, and I see what you mean -- we're starting a session just for looking at a flag link. I agree that's wrong -- could you file a bug report on Flag please?

However, that doesn't change that there is a need for this API. In the Flag case, once an anon user clicks a flag link, we need to make sure there is a session for them, and there should be a better way than setting $_SESSION['some irrelevant key'] = TRUE.

fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm at OSInet commented

Followup added as #2894095: Enabling a flag adds session cookies to anonymous users.

Back to this issue: strictly speaking, I wonder what could be the purpose of having a session without having anything in it, considering how sessions cost in performance in the general case. What would it bring to the module starting such a session without content ?

joachim’s picture

joachim CreditAttribution: joachim commented

> I wonder what could be the purpose of having a session without having anything in it, considering how sessions cost in performance in the general case. What would it bring to the module starting such a session without content ?

In the Flag use case, we don't need anything in the session. We just need it to exist and have an ID that persists. The current code just puts junk in to force that:

        // Ensure something is in $_SESSION, otherwise the session ID will
        // not persist.
        // TODO: Replace this with something cleaner once core provides it.
        // See https://www.drupal.org/node/2865991.
        $_SESSION['flag'] = TRUE;

        $this->sessionManager->start();

As you can guess, we're never going to use $_SESSION['flag'] = TRUE;. We're not going to need to check for it. We only had to put it there to cause a session to properly start.

fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm at OSInet commented

Thanks for the answer, but to me it still remains unclear: what is the value of having this anonymous user session id until there is anything actually stored for that session ? I don't see where in the Flag code base that ID is used until some flagging action is actually performed, at which point is seems normal to start the session.

joachim’s picture

joachim CreditAttribution: joachim commented

> until some flagging action is actually performed, at which point is seems normal to start the session.

Yup, that's it. When a flagging action is performed by an anonymous user, a session should be started.

The point of this issue is that I think there should be a cleaner way of starting the session than doing this:

        $_SESSION['made-up-key'] = "junk I don't care about";
joachim’s picture

joachim CreditAttribution: joachim commented

> what is the value of having this anonymous user session id until there is anything actually stored for that session

It occurs to me belatedly that perhaps the bit we're misunderstanding each other on is that Flag doesn't need to store any data in the actual session. It only needs a session ID which it saves as a field value on the Flagging entity.

fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm at OSInet commented

This is perfectly clear, thanks, not a problem.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

znerol’s picture

znerol CreditAttribution: znerol commented
Parent issue: » #2371629: [meta] Finalize Session and User Authentication API
znerol’s picture

znerol CreditAttribution: znerol commented
Issue tags: +ContributionWeekend2020, +ContributionWeekendCH

Drupal cleans up empty sessions (and associated cookies) for the reasons fgm already stated. Even if there would be an API to forcibly start a session, the only way to keep it open across request would be to store something in it (even if it is a keep-it-open flag.

The flag module should generate an identifier whenever the first flag was set by an anonymous user and store that in the session as well as in the flag entity. On subsequent requests, this identifier can be used to locate existing flag entities as well as to associate new ones. As soon as the user clears the last flag, the module needs to clear the identifier from the session.

IMHO this is a wontfix.

joachim’s picture

joachim CreditAttribution: joachim commented

> Even if there would be an API to forcibly start a session, the only way to keep it open across request would be to store something in it (even if it is a keep-it-open flag.

Right. So this issue would provide a clear, simple API that takes care of storing one thing in the session to keep it open. At the moment, modules are all having to dump their arbitrary junk into sessions.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

As @joachim said #17 ++

core provides shared temp storage but no common API to bound it to session
looks it needs summary update

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

OTOH Probably core could suggest to use Symfony session native way to contrib

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

Probably I could move masquerade session flag to session bad to better control flag operations and logging

znerol’s picture

znerol CreditAttribution: znerol commented

Right. So this issue would provide a clear, simple API that takes care of storing one thing in the session to keep it open. At the moment, modules are all having to dump their arbitrary junk into sessions.

To be clear. The thing which intends to enforce a session also is responsible to release that enforcement as soon as it is not necessary anymore. Thus, client code needs its own key/identifier anyway. Please let's just keep to the symfony API (i.e., the SessionInterface). If you want to make it pretty for the flags module, then I suggest trying to add a custom AttributeBag which you use to abstract that.

joachim’s picture

joachim CreditAttribution: joachim commented

> The thing which intends to enforce a session also is responsible to release that enforcement as soon as it is not necessary anymore.

I don't really understand this point. With the use cases I've seen for this, the thing which wants a session wants the session to remain as long as PHP/the site cares to keep it for. There's no release.

> Please let's just keep to the symfony API (i.e., the SessionInterface).

The Symfony API doesn't start sessions if they're empty.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

FYI used to try custom bag approach at today's sprint with #3042321-24: Last access by a user should not change after masquerade as that user (D8)

It's really tricky sometimes to access stored data when session reports is not started

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Related issues: +#3042321: Last access by a user should not change after masquerade as that user (D8)

First feeling is that \Drupal\Core\Session\MetadataBag needs method like [get|set|unset]thirstPartyFlag()

znerol’s picture

znerol CreditAttribution: znerol commented

the thing which wants a session wants the session to remain as long as PHP/the site cares to keep it for. There's no release.

Keeping anonymous sessions open for no reason renders caches ineffective (both HTTP proxies as well as the browser). This is definitely not something we want to encourage in core.

First feeling is that \Drupal\Core\Session\MetadataBag needs method like [get|set|unset]thirstPartyFlag()

The MetadataBag is exempted when checking whether a session is empty in SessionManager::isSessionObsolete(). Thus if a session contains nothing more than metadata, the session is destroyed and the session cookie is unset.

znerol’s picture

znerol CreditAttribution: znerol commented

@andypost: Opened #3108967: Introduce a container tag to register session bags.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

Looks there's nothing left to fix as sessions will use "bags" and no longer care about start/stop state

joachim’s picture

joachim CreditAttribution: joachim commented

> The Symfony API doesn't start sessions if they're empty.

Is this no longer the case?

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

voleger made their first commit to this issue’s fork.

voleger opened merge request !2386

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at GOLEMS GABB, Drupal Ukraine Community for Drupal Ukraine Community, GOLEMS GABB commented
Status: Active » Needs review

So before closing the issue, references from the codebase need to be removed.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Version: 9.5.x-dev » 10.1.x-dev

@voleger please rebase MR on 10.1.x

joachim’s picture

joachim CreditAttribution: joachim commented
Status: Needs review » Needs work

> The Symfony API doesn't start sessions if they're empty.

I still think this isn't the case.

I just tried

$this->session_manager->start();

and

$this->session_manager->getId();

and

$this->session_manager->save();

and none of them created an entry in the sessions table.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Issue tags: +Needs issue summary update

MR removes the TODO and sessions should start automatically

joachim’s picture

joachim CreditAttribution: joachim commented
Issue tags: -Needs issue summary update

I don't understand the MR or #40.

The method startSession() in the MR is protected, so not part of the API. It only gets called when you set some data in the private tempstore, which is exactly the problem described by this issue -- that you have to set some data in order to start a session.

znerol’s picture

znerol CreditAttribution: znerol commented
Issue tags: -ContributionWeekend2020, -ContributionWeekendCH
Related issues: +#3413153: Remove calls to Request::hasSession()

The current MR in #3413153: Remove calls to Request::hasSession() removes PrivateTempStore::startSession() entirely.

I suggest we close this issue here as a won't fix.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs work » Closed (won't fix)

As commited already #3413153: Remove calls to Request::hasSession()