Details
Description
System.out.println(request.getSession().getId());
request.getSession().invalidate();
System.out.println(request.getSession().getId());
getSession() although return the same session after the invalidation.
Attachments
Activity
ok,thanks
ignite.xml:
<bean id="session.cfg" class="org.apache.ignite.configuration.IgniteConfiguration">
<property name="gridName" value="sessionGrid" />
<property name="metricsLogFrequency" value="0" />
<property name="gridLogger" >
<bean class="org.apache.ignite.logger.slf4j.Slf4jLogger" />
</property>
<property name="cacheConfiguration">
<bean class="org.apache.ignite.configuration.CacheConfiguration">
<!-- Cache mode. -->
<property name="name" value="webSession" />
<property name="cacheMode" value="PARTITIONED" />
<property name="backups" value="1" />
</bean>
</property>
<property name="discoverySpi">
<bean class="org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi">
<property name="ipFinder">
<bean class="org.apache.ignite.spi.discovery.tcp.ipfinder.vm.TcpDiscoveryVmIpFinder">
<property name="addresses">
<list>
<value>localhost</value>
</list>
</property>
</bean>
</property>
</bean>
</property>
</bean>
web.xml:
<listener>
<listener-class>org.apache.ignite.startup.servlet.ServletContextListenerStartup</listener-class>
</listener>
<filter>
<filter-name>IgniteWebSessionsFilter</filter-name>
<filter-class>org.apache.ignite.cache.websession.WebSessionFilter</filter-class>
</filter>
<filter-mapping>
<filter-name>IgniteWebSessionsFilter</filter-name>
<url-pattern>/*</url-pattern>
</filter-mapping>
<context-param>
<param-name>IgniteConfigurationFilePath</param-name>
<param-value>/var/www/ignite/ignite-session.xml</param-value>
</context-param>
<context-param>
<param-name>IgniteWebSessionsGridName</param-name>
<param-value>sessionGrid</param-value>
</context-param>
<context-param>
<param-name>IgniteWebSessionsCacheName</param-name>
<param-value>webSession</param-value>
</context-param>
System.out.println(request.getSession().getId()); request.getSession().invalidate(); System.out.println(request.getSession().getId());
Attributes are correctly nullified, but after the current session is invalidated, a new one with a different id has to be created (and that does not happen).
Another problem is a bunch of methods should throw IllegalStateException on invalidaded session (http://docs.oracle.com/javaee/7/api/javax/servlet/http/HttpSession.html)
The fix invalidates a session within a request, and enables the following scenario
// returns a valid session request.getSession(); // invalidates the session request.getSession().invalidate(); // now the session is null request.getSession(false); // creates a new session request.getSession()
vkulichenko Can I ask you to review it and mute IgniteWebSessionSelfTestSuite$WebSessionTransactionalSelfTest.testInvalidatedSession (or give me permissions)?
Roman,
Sorry for the delay. I will take a look at this today or tomorrow.
Roman,
Looks good,
but:
1) Is there any reason to store isValid at writeExternal? I see that cache.remove(ses) happens at ses.invalidate().
I removed isValid storage and see no test failures.
2) Seems that "GenuineSession" should be invalidated at ses.invalidate, not at next getSession() request. (super.invalidate() should be invoked)
3) WebSession ses at RequestWrapper should be marked as volatile
4) getSession at RequestWrapper should return invalid session instead of null
Anton,
Thanks for your comments! I have made the changes.
1) -> You are absolutely right. Externalizing isValid is not needed here – by the current logic the session is completely deleted from the cache, and when it is not found a new session is created. I think I will need to make isValid transient then.
2) -> HttpSession is an interface. I could not find a cleaner way, so I invalidate the gunuine session on every subsequent getSession() if it is invalid.
3) -> Good point, thank you!
4) -> I was thinking of it too. But doing some investigations and looking at users' code at github, it looks like many users expects null. Tomcat produces null too. Just want to be sure users won't be surprised switching to Ignite. What do you think?
Roman,
2) I see that gunuine session is a Websession constructor param, possible we have to store it as a field and invalidate at ses.invaliate()? Or we can invalidate it somehow using sesId? What do you think?
4) Sounds good in this case.
Anton,
I changed the implementation by storing a genuine session, as you advised. Looks nice and simple to me.
isValid's use becomes trivial – just to follow specifications and produce IllegalStateException when invalid.
Roman,
1) I think that "if (cache.get(ses.getId()) == null) {" should be replaced by isValid() check because cache.get if much more heavy than reading volatile.
2) Also new session should be created on demand at any cases, not only if current invalidated? Possible previous one should be invalidated in case new one demanded.
Anton,
1) Yes, you are right. I have reverted this change.
2) According to the specs, it is created only when there is no session and create is true. http://docs.oracle.com/javaee/7/api/javax/servlet/http/HttpServletRequest.html#getSession-boolean-
Roman,
Now looks good to me.
Please change genuineSession to ses according to https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules and check TeamCity.
P.s. I'm not an expert in servlets, so in case you're not sure at some changes lets ask someone to review result of our discussions
Thank you! Maybe Valentin has some comments?
It seems session replication has more issues. I will open other tickets if I can confirm.
Also,IgniteWebSessionSelfTestSuite$WebSessionTransactionalSelfTest.testInvalidatedSession will have to be muted. Whom should I ask?
Roman,
it's not a good case to mute untill it contains fail("https://issues.apache.org/jira/browse/IGNITE-*");
Someone, for example me, will mute that when see such failure.
P.s. ask Val about review by email.
Anton,
Got it. It contains fail(...) for transactional mode related to a jira issue. Let's see if Val has any comments, and someone can mute it after I merge.
Changes make sense to me. Couple of questions though:
- Why not all methods are guarded by if (!isValid) check? Is it on purpose?
- Why the test doesn't work in transactional cache?
Thank you!
1) methods are guarded according to the HttpSession specs. Probably the only left are s/getMaxInactiveInterval() and getId() but specs don't say how it should behave when the session is invalid, it says they must return a value. All the rest are deprecated and call guarded methods.
http://docs.oracle.com/javaee/7/api/javax/servlet/http/HttpSession.html
2) I guess it's the same IGNITE-810 which doesn't work for testRestarts()
A quick test invalidates session for me.
Can you please provide simple configurations you use to reproduce it?