Contrib.social

Replace the usage of the super global $_SESSION

Open on Drupal.org →
Created on 4 April 2025, 3 months ago

Problem/Motivation

Follow up issue from 📌 Anonymous users - Multiple voting from one IP Needs review

Replace the usage of the super global $_SESSION

Relying on the session super global within the storage doesn't feel right. We shouldn't use that anymore. We could inject the request stack and get it from the current request a start.
It's also not isolated, no single API is responsible for this session storage. It's set in the form.
Looking at the flag module, which is quite comparable and supports per-session flags, it works quite different. it generates a session id and then stores that in a column. It's needed there, because you need to be able to query for your flags. But it might be worth considering here as well, for example you could detect bots that vote on many polls and delete those votes, if you have many active polls (if you do, storing the ids directly in the session could also grow quite a bit. But you would probably need a lot for that to become a problem).
that has it's own issues, Drupal no longer starts a session automatically, so it needs some extra work to get and manage that id (see \Drupal\flag\FlagService::ensureSession).

Source: https://git.drupalcode.org/project/poll/-/merge_requests/30#note_279119

Steps to reproduce

N/A

Proposed resolution

Replace the occurrences of $_SESSION with the session data of the current request.
Implement something like \Drupal\flag\FlagService::ensureSession

Remaining tasks

MR
In theory no extra tests are needed, everything should be green after those changes.

User interface changes

N/A

API changes

N/A

Data model changes

N/A

📌 Task
Status

Postponed

Version

2.0

Component

Code

Created by

🇧🇪Belgium BramDriesen Belgium 🇧🇪

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

Merge Requests

Comments & Activities

  • Issue created by @BramDriesen
  • Comment 3 months ago →
    🇧🇪Belgium BramDriesen Belgium 🇧🇪

    This can be taken up now.

  • Comment 3 months ago →
    🇮🇳India debrup

    I am currently working on this issue.

  • Comment 3 months ago →
    🇧🇪Belgium BramDriesen Belgium 🇧🇪

    No activity over 2 days, un-assigning. Feel free to pick this up again.

  • Merge request !78Issue #3517381: Replaced super global "$_SESSION". → (Open) created by debrup
  • Pipeline finished with Success
    about 2 months ago
    Total: 334s
    #495862
  • Pipeline finished with Success
    about 2 months ago
    Total: 270s
    #495881
  • Comment about 2 months ago →
    🇮🇳India debrup

    @bramdriesen I have replaced the $_SESSION super global with the recommended session handling in Drupal. While working I also found the issue of anonymous vote cancellation.

    Steps to Reproduce

    • Create a poll and make sure anonymous voting, cancellation and required permissions are given to anonymous user.
    • Keep the poll open as admin view. Open a incognito tab(s) in multiple separate browsers.
    • Access the poll here and cast the vote.
    • Check the result back in the admin view. The voted choice should be incremented and updated.
    • Now cancel the vote in the incognito tab.
    • Check the result in the admin view. It remains unchanged. The cancelled vote is not deleted from the database!

    Note:

    This warning also occurs in the log when cancel button is pressed.

    Warning: Trying to access array offset on false in Drupal\poll\PollVoteStorage->cancelVote() (line 131 of /app/web/modules/contrib/poll/src/PollVoteStorage.php)
  • Comment about 2 months ago →
    🇮🇳India debrup

    Apart from the above mentioned issue, the required issue of replacing the $_SESSION variable with Drupal recommended session handling has been done. @bramdriesen please review and let me know if any changes are required.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024