Details
-
Bug
-
Status: Closed
-
Normal
-
Resolution: Fixed
-
None
-
None
Description
org.onehippo.forge.hst.spring.support.session.servlet.HttpSessionDelegatingSession simply delegates getAttribute(), setAttribute() and removeAttribute() to the underlying HttpSession.
This makes a side effect in the following situation:
- The spring session implementations such as org.onehippo.forge.hst.spring.support.session.servlet.HttpSessionDelegatingSession are wrapped by org.springframework.session.web.http.HttpSessionAdapter which implements javax.servlet.http.HttpSession. So, applications get access to this wrapper instance.
- If an application sets a HttpSession attribute through the adapter, and if the attribute implements javax.servlet.http.HttpSessionBindingListener, then the adapter first sets the attribute first to the underlying spring session, and then second invoke HttpSessionBindingListener#valueBound(...) method afterward.
In a corner case [1], if the attribute object implementing HttpSessionBindingListener invokes HttpSession#removeAttribute(name) for some (security?) reason, in case that the attribute is already set in the underlying HttpSession, then the setAttribute call ends up removing it.
Solution is as follows:
- As spring-session assumes that org.springframework.session.Session and org.springframework.session.SessionRepository are responsible for internal session data management, not as equivalent to an HttpSession implementation such as org.springframework.session.web.http.HttpSessionAdapter or org.apache.catalina.session.StandardSession [2], org.onehippo.forge.hst.spring.support.session.servlet.HttpSessionDelegatingSession should not use the same HttpSession attribute name and value.
- Instead, it can store a single map as single HttpSession attribute, and store/retrieve/remove from the map instead. As a result, HttpSession attributes are not directly shared.
- This is actually how the default spring-session implementation, org.springframework.session.MapSession, works.
[1] For example, org.onehippo.cms7.services.cmscontext.CmsContextServiceImpl.CmsSessionContextImpl implements valueBound() like the following:
@Override public void valueBound(final HttpSessionBindingEvent event) { if (session == null && SESSION_KEY.equals(event.getName())) { session = event.getSession(); } else { // don't allow storing this instance under any other session attribute and/or any other session event.getSession().removeAttribute(event.getName()); } }
So, it invokes HttpSession#remove(...) when the value object triggering the eent is being set, in case CmsSessionContextImpl was already set in the HttpSession. So, it is never able to set the context attribute with the current org.onehippo.forge.hst.spring.support.session.servlet.HttpSessionDelegatingSession implementation in spring-session.
[2] An interesting different between org.springframework.session.web.http.HttpSessionAdapter or org.apache.catalina.session.StandardSession is that the former stores the new attribute before invoking #valueBound() whereas the latter does in the reversed order. So, with the latter, the corner case in org.onehippo.cms7.services.cmscontext.CmsContextServiceImpl.CmsSessionContextImpl could be working.