Unchanged sessions should not be persisted on every requests

[PowerKiKi]

• [x] I was not able to find an open or closed issue matching what I'm seeing.
• [x] This is not a question. (Questions should be asked on slack (Signup for Slack here) or our forums.)

Provide a narrative description of what you are trying to accomplish.

Code to reproduce the issue

Follow the documentation to pipe the middleware early for the whole application in config/pipeline.php like so:

$app->pipe(\Zend\Expressive\Session\SessionMiddleware::class);
$app->pipeRoutingMiddleware();

And configure the session persistence implementation with zendframework/zend-expressive-session-ext (the only one published by Zend AFAIK) in config/config.php:

 $aggregator = new ConfigAggregator([
   \Zend\Expressive\Session\Ext\ConfigProvider::class,
    // [...]
], $cacheConfig['config_cache_path']);

return $aggregator->getMergedConfig();

Expected results

I expect to see a Set-Cookie header only when I write to the session (typically when a user logs in). And I expect to never see a Set-Cookie header if the session is not written to (typically most other requests after a login happened).

Actual results

Every single request has the Set-Cookie header. This has several downsides.

First, it's of course a (very) slight performance hit (on server CPU, client CPU and network transfer) for no gain at all.

Second, the default behavior of \Zend\Expressive\Session\Ext\PhpSessionPersistence is to also send cache limiting headers. So pretty much all HTTP request going through PHP become non-cacheable.

And finally when configuring a cookie_lifetime, the absolute expiry date of the cookie will be re-set on every requests rendering the lifetime effectively infinite. This might be a desired use-case for some people, but it certainly should not be the default behavior as it is now impossible to have a fixed lifetime.

Fix suggestion

If you agree with the idea, I'll submit a PR, and tests, to not call the persistence if the session is unchange. As simple as that:

diff --git src/SessionMiddleware.php src/SessionMiddleware.php
index ef77113..edafc5b 100644
--- src/SessionMiddleware.php
+++ src/SessionMiddleware.php
@@ -32,6 +32,11 @@ class SessionMiddleware implements MiddlewareInterface
     {
         $session = new LazySession($this->persistence, $request);
         $response = $handler->handle($request->withAttribute(self::SESSION_ATTRIBUTE, $session));
-        return $this->persistence->persistSession($session, $response);
+
+        if ($session->hasChanged()) {
+            $response = $this->persistence->persistSession($session, $response);
+        }
+
+        return $response;
     }
 }

[weierophinney]

First, we also publish zend-expressive-session-cache in addition to the ext-session adapter.😉

Second, while you can add session awareness globally, I'd recommend adding it only for routes where it will actually be used. Doing so solves the problems you report almost entirely.

Third, while I'm definitely open to a patch here, I have one concern: what about non-cookie based session mechanisms (e.g., using a JWT query parameter and/or header)? In such cases, there will be no cache-control headers, and the only way the client can send the token is to have it in the current request already (since nothing is persisted). If we do not send it, the client cannot provide it, which means the data is lost and a new session needs to spin up; this is arguably a worse scenario than the one you outlined!

It might make more sense to make the decision at the adapter level instead, when persistence is requested. That way the adapter can decide if the cookie should be sent.

[PowerKiKi]

Thanks for letting me know about zend-expressive-session-cache, I somehow missed it.

Adding session awareness only for routes that need it, would indeed limit the impact of the issue. But in a application where a user can log in, chances are that most, if not all, routes will need to be aware of the session to know whether a user is logged in. So in practice I don't think it would help that much.

I cannot imagine a session mechanism where there is no storage at all. It must either be stored on the server side, or on the client side. In a JWT scenario how would the client send a header if the JWT is never stored anywhere ? At the very least it must be kept in the client memory after login, and then sent back on each subsequent requests. So if the client already has the JWT in memory (or in local storage or whatever else), then what is the point for the server to re-send the exact same thing over and over again ?

Patching the adapter would be a simple solution too, but I feel it is the wrong place because it would duplicate logic in several packages whereas the decision really should be made at a higher level. zend-expressive-session-ext and zend-expressive-session-cache have very similar implementation for persistSession(), both are sending cookie and cache limiting headers even if the content never changed. So sure we could patch both of them, but then we'll have to think about it for all future implementations too. That seems unnecessarily redundant and error prone.

So, againt what should I create a PR ? one middleware or all adapters ?

[weierophinney]

I cannot imagine a session mechanism where there is no storage at all. It must either be stored on the server side, or on the client side. In a JWT scenario how would the client send a header if the JWT is never stored anywhere ? At the very least it must be kept in the client memory after login, and then sent back on each subsequent requests. So if the client already has the JWT in memory (or in local storage or whatever else), then what is the point for the server to re-send the exact same thing over and over again ?

In many cases, the JWT is appended as a query string argument to any links or form actions that go back to the application, instead of being returned as a header. (You can see this in action on sites such as Amazon, where, once logged in, you have a token appended to every URL.) The adapter then retrieves the JWT from the query string arguments, validates it, and extracts data from it. Due to the limitations of the amount of data that can be sent in a JWT, this is often limited to a user identifier, an account or order or cart identifier, etc. But the point is that it can be done, and we designed zend-expressive-session with this possibility in mind.

[PowerKiKi]

I just submitted https://github.com/zendframework/zend-expressive-session-ext/pull/44 to fix this issue.