vert-x3 / vertx-web

HTTP web applications for Vert.x
Apache License 2.0
1.11k stars 535 forks source link

SockJS Event Bus Bridge does not keep web sessions alive for clustered session managers #2600

Open cgm-aw opened 7 months ago

cgm-aw commented 7 months ago

Version

>= 4.5.0 (older are probably also affected)

Context

When using the SockJS event bus bridge, EventBusBridgeImpl#internalHandlePing keeps the web session alive by calling Session#setAccessed However this only works for a local session storage. ClusteredSessionStoreImpl does not honor Session#lastAccessed but rather uses the TTL feature of AsyncMap.

Do you have a reproducer?

Yes please check out https://github.com/cgm-aw/vertx-sessiontimeout-reproducer

Steps to reproduce

  1. Deploy a verticle with a clustered session manager
  2. Perform a http call, you will get a web session
  3. Open a SockJS connection and utilize the event bus bridge so the periodic ping happens
  4. Wait until the configured session timeout is over
  5. Perform another http call, you will get a new web session <- the ping should have kept the original session alive

Please check out my reproducer, you can reproduce the issue and also see how the mechanism works fine with the local session storage.

Best regards

tsegismont commented 2 months ago

I spent some time looking into this and here are my findings.

Calling Session.setAccessed in EventBusBrideImpl was added in #382 (Vert.x 3.3). It hasn't been documented nor tested. It leverages an implementation detail of the LocalSessionStore to extend the lifetime of a web Session: this store uses the last accessed time to determine if session data should be removed from memory.

Other session stores (clustered session store, Redis, Infinispan) use TTL attributes of the underlying storage to expire a session. The lifetime is only extended when a session is flushed.

A session is flushed after the HTTP response headers have been sent if:

So, this behavior can be seen not only with the websocket transport but also with other transports, provided the session handler is lazy and the session is not accessed.

Note that raw WebSocket message exchanges (without SockJS) don't extend the lifetime of a web Session either.

I err on the side of closing this as won't fix. I would advise users who want to extend the lifetime of a session to make the browser send a request to the backend.

Thoughts @vietj @pmlopes ?

cgm-aw commented 2 months ago

Thank you for our analysis, @tsegismont! I would be a bit sad if you closed this with "won't fix" - as long as the web socket connection is open, it clearly means that the client is still alive. In my mind, the client would have to actively close the web socket connection (and therefore the backend must destroy the session), if the user logged out. Introducing a second keep-alive via http in parallel to the web socket keep-alive would be annoying for the client.

Best regards