Bug in Undertow InMemorySessionManager

[dvayanu]

Hello,

I was recently investigating an issue with sessions in our application and tracked it down to InMemorySessionManager. In the local class SessionImpl there is a method getAttribute:

@Override
public Object getAttribute(final String name) {

   if (invalid) {

   throw UndertowMessages.MESSAGES.sessionNotFound(sessionId);

  }

  bumpTimeout();

   return attributes.get(name);

}

The call to bumpTimeout() would prevent session from expiring. However, this is against the spec (7.5) and common sense, since the session should only be dependent on user requests, not some crazy code accessing attributes.

Is there any particular logic here? I checked tomcat and it's logic (also different via lastAccessTimestamp) is not influenced by accessing the attributes. Since jboss used to used tomcat as servlet engine prior to 8, why change in such a harsh way?

Or am I just misreading the code?

regards

Leon

[dvayanu]

Just wanted to make clear what are the consequences of this behaviour. One of my clients is using wildfly to host a web-shop. In the web-shop the basket will be saved in a session. However, prices do change from time to time, so there is a process that runs through all the sessions, looks at all the baskets and check if prices in the basket have to be adjusted. This leads effectively to session which would never expire and servers blowing up.

[mchoma]

Are you refering to Java Servlet 3.1 Specification chapter 7.5 Session Timeouts? Could you write down which sentence you have in mind exactly?

But yes I have got your point. But this is general problem bumpTimeout is also in setAttribute, removeAttribute, getAttributeNames, getId. Question here is how should be "user interaction" detected reliably.

swd847 what do you think?

[jstourac]

Martin, I would correct the list of methods where 'bumpTimeout' is actually used in InMemorySessionManager to following: createSession(), setMaxInactiveInterval(), getAttribute(), getAttributeNames(), setAttribute(), removeAttribute(). From this list usage in following methods is suspicious: getAttribute(), getAttributeNames(), setAttribute(), removeAttribute().

All occurrences were added by this commit [2] with initial session timeout implementation.

The truth is the Servlet 4.0, section 7.5 [3] specification (Servlet 3.1 is almost identical) specifies that timeout depends on user activity only:

"This means that the only mechanism that can be used to indicate when a client is no longer active is a time out period."

User interaction by common sense should mean some client request. Yeah, it looks like a bug to me, but swd847 should take a look.

[1] undertow/InMemorySessionManager.java at master · undertow-io/undertow · GitHub

[2] Implement session timeout · undertow-io/undertow@be768b6 · GitHub

[3] https://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_FINAL.pdf

[adamkrajcik]

I created JIRAs to track progress on this: WFLY-11115[1] UNDERTOW-1419[2]

[1] [WFLY-11115] bumpTimeout method usage in InMemorySessionManager - JBoss Issue Tracker 

[2] [UNDERTOW-1419] bumpTimeout method usage in InMemorySessionManager - JBoss Issue Tracker