TQ
dev.com

Blog about software development

Subscribe

Proposal to fix a 2012 bug in Symfony

28 May 2022 - by 'Maurits van der Schee'

When your Symfony (PHP web framework) project uses AJAX requests and sessions (logging in) you may run into this 2012 bug where Symfony does not lock the session allowing for data loss on concurrent AJAX requests. I fixed the bug in 2014 in the SncRedisBundle, but that merge was reverted last year, creating an issue for some high traffic sites. In this post I propose a better solution for Symfony.

Quick workaround

The quick workaround is to use the NativeFileSessionHandler class, which uses the session storage (handler) in the php.ini (using session.save_handler and session.save_path) that does store session files on disk. As long as sessions are not working reliable (due to lack of locking) I advice to stay away from Symfony's Redis and Memcache support for session storage and use this native variant (that does support locking).

ini_set('session.save_handler', 'files');
ini_set('session.save_path', sys_get_temp_dir());

See: https://github.com/symfony/symfony/blob/6.1/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NativeFileSessionHandler.php

Performance impact

Some people argue that locking is bad for performance and they are right. It is bad to lock a session on a single action as this would prevent the same session to run another action in parallel (using another apache/php thread). You will mainly notice this is an AJAX heavy application as HTML (top level navigation) requests are serialized by your browser anyway. This locking is only needed when the action modifies the session data, which is something we do not know in advance (or do we?). Hence the following proposal, please read on.

Proposal (read-only sessions)

What I think would be an optimal solution for both performance and correctness: I suggest we look at the verb (GET or not) to indicate that the action (may) write to the session (on POST, PUT or DELETE) or when it should be read-only (GET). If the action may write to the session we open the session (identified by the session identifier) with a lock and write it back after the action completes. If it is a GET (especially when it is an AJAX request) we open the session in read-only mode and throw an exception when people write to it. Note that the session cleanup must take these GET requests (with read-only session) into account by writing an updated timestamp in the session store.

How to create read-only sessions

If you want to implement "read-only" session you shouldn't use the read_and_close flag that was added in PHP 7. That call does not update session timestamp and causes session timeouts. In PHP read-only sessions can be achieved by calling session_write_close just after session_start. Although this may feel sub-optimal it is actually isn't if your session save handler implements the "SessionUpdateTimestampHandlerInterface". If it does then updateTimestamp is called instead of write when session_write_close is called without modifications to the session data. This takes away most of the performance concerns.

See: https://stackoverflow.com/questions/37789172/php-session-randomly-dies-when-read-and-close-is-active

Two new options for session start?

I expected the read_and_close flag to NOT lock the session (which it does) and expected that it DOES update the session timestamp (which it doesn't). That's because I assumed it was added to facilitate read-only sessions. I guess it has a different use case. That being said I would have liked a read_only option in the session_start function that does update the session timestamp. The read_only option may block very briefly to ensure serialization and thus consistency. I would also like to also have a read_stale option that doesn't use locks at all (but also does update the session timestamp).

Readonly exceptions

Proposed solution: When people write to the session (or to the database) on a GET request you throw an error unless an annotation is set (to allow non-readonly behavior). This requires an alternative to the "stateless" annotation of the route, called "readonly", which is less strict and is used to mark readonly exceptions (non-readonly on GET requests or readonly on non-GET requests).

// src/Controller/MainController.php
namespace App\Controller;

use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\Routing\Annotation\Route;

class MainController extends AbstractController
{
    /**
     * @Route("/", name="homepage", readonly=false)
     */
    public function homepage()
    {
        // ...
    }
}

Note that the readonly=false annotation is part of this proposal and not actually supported by Symfony (yet).

See: https://symfony.com/doc/current/routing.html#stateless-routes

Warnings and errors write to the session

Symfony promotes the use of a session stored "FlashBag" to transfer warnings and errors from one route to the next. This allows for the "Post/Redirect/Get" (PRG) pattern, having different routes for the form retrieval and the form submission action. This may seem to simplify the controller implementation, but I'm not even sure that it does. From a performance perspective I'm not a fan of this pattern as it increases the number of session writes.

See: https://stackoverflow.com/questions/10827242/understanding-the-post-redirect-get-pattern

CSRF tokens write to the session

Cross Site Request Forgery (CSRF) protection avoids blind form submission in case of XSS vulnerabilites. This type of form submission protection uses CSRF tokens that writes to the session on every form retrieval request (in Symfony). Although that may certainly be true for Symfony, that is not a necessity for CSRF protection, as you can read here:

CSRF tokens should be generated on the server-side. They can be generated once per user session or for each request. Per-request tokens are more secure than per-session tokens as the time range for an attacker to exploit the stolen tokens is minimal. However, this may result in usability concerns.

See: https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#synchronizer-token-pattern

Conclusion

Writing reliably to the session requires session locking. Symfony does not lock sessions in it's session implementation(s), while PHP natively does. Session locking has performance impact and the number of session locks should be reduced. PHP (core) does support a form of read-only sessions by closing the session directly after starting it. Symfony on the other hand is not promoting readonly sessions with in it's Flashbag and CSRF implementations (that write to the session). Proper session locking seems to be a problem people don't understand very well. I hope this post helps.

See: https://github.com/symfony/symfony/issues/4976

Enjoy!


PS: Liked this article? Please share it on Facebook, Twitter or LinkedIn.