symfony/symfony — Issue #2607: [HttpFoundation] Session management design issues
Author: drak
Created: 2011-11-10 22:08:47+00:00
State: open

I was looking as ticket #2591 and have noticed serious issues with session management that needs to be corrected.

There is a `Session` class which uses a `SessionStorageInterface`, all good so far. In order for PHP to correctly use our session management, we need to register functions (callables) for each of the specific session operations, "open, close, read, write, destroy and GC" using `session_set_save_handler`;

The problem is at the moment, only PdoSessionStorage registers any callables with `session_set_save_handler` and they are registered directly with the storage driver.

When the session is destroyed PHP calls `session_write_close()` which will in turn call whatever we registered using `session_set_save_handler`. If nothing is registered PHP will use it's own internal defaults. The problem is `Session->save()` does some work before calling the storage driver. This means there will be circumstances where the session is closed, and this bypasses the `Session->save()` tasks and heads straight for the storage driver we registered (if we did).

When PHP shuts down, it will destroy all objects in the reverse order they were created. Lastly it will try to close the session (ie save the data). If we have registered a shutdown function, PHP will call that before initiating system a shutdown (which entails destroying objects and cleaning up memory).

So the first problem is the `SessionStorageInterface` should have interfaces for read, write, open, close, destroy and gc so these can be registered with PHP's `session_set_save_handler`. Secondly, `Session->save()` does pre-shutdown work, so it needs to be outsourced to the session storage drivers, or, the storage drivers need to have the session injected so they can call `save()` before writing the session (probably ugly), or register the write handler with the session class which would be strange since the `Session` class is the developers interface to session management, and we don't want or need the storage methods in the `Session` object since it's for PHP's internals only.

`PdoSessiontorage` currently extends `NativeSessionStorage` but actually it's implementing it's own methods and registering them with `session_set_save_handler` making the interface pointless.

Specific problems
`NativeSessionStorage` uses methods which are not registered with `session_set_save_handler`.
`FilesystemSessionStorage` uses methods which are not registered with `session_set_save_handler`.

Additionally, there are no tests for the storage drivers except `FilesystemSessionStorage` which in any case don't test the driver working with sessions, it just proved it reads/write the FS.

I am quite familiar with session management so I could sort all these problems out - they need to be sorted out IMO before 2.1 is released.