FELIX-5175 - Felix incorrectly destroys sessions
- fixed the issue by converting the maxInactive time to milliseconds;
- added a simple test case to show the problem and test the fix.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1727299 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HttpSessionWrapper.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HttpSessionWrapper.java
index 1dc718e..6fdb00e 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HttpSessionWrapper.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HttpSessionWrapper.java
@@ -40,6 +40,7 @@
* The session wrapper keeps track of the internal session, manages their attributes
* separately and also handles session timeout.
*/
+@SuppressWarnings("deprecation")
public class HttpSessionWrapper implements HttpSession
{
/** All special attributes are prefixed with this prefix. */
@@ -51,10 +52,10 @@
/** The created time for the internal session (appended with context id) */
private static final String ATTR_CREATED = PREFIX + "created.";
- /** The last accessed time for the internal session (appended with context id) */
+ /** The last accessed time for the internal session (appended with context id), as Epoch time (milliseconds). */
private static final String ATTR_LAST_ACCESSED = PREFIX + "lastaccessed.";
- /** The max inactive time (appended with context id) */
+ /** The max inactive time (appended with context id), in seconds. */
private static final String ATTR_MAX_INACTIVE = PREFIX + "maxinactive.";
/** The underlying container session. */
@@ -95,19 +96,20 @@
public static Set<Long> getExpiredSessionContextIds(final HttpSession session)
{
final long now = System.currentTimeMillis();
+
final Set<Long> ids = new HashSet<Long>();
final Enumeration<String> names = session.getAttributeNames();
- while ( names.hasMoreElements() )
+ while (names.hasMoreElements())
{
final String name = names.nextElement();
- if ( name.startsWith(ATTR_LAST_ACCESSED) )
+ if (name.startsWith(ATTR_LAST_ACCESSED))
{
final String id = name.substring(ATTR_LAST_ACCESSED.length());
- final long lastAccess = (Long)session.getAttribute(name);
- final Integer maxTimeout = (Integer)session.getAttribute(ATTR_MAX_INACTIVE + id);
+ final long lastAccess = (Long) session.getAttribute(name);
+ final long maxTimeout = 1000L * ((Integer) session.getAttribute(ATTR_MAX_INACTIVE + id));
- if ( maxTimeout > 0 && lastAccess + maxTimeout < now )
+ if ((maxTimeout > 0) && (lastAccess + maxTimeout) < now)
{
ids.add(Long.valueOf(id));
}
@@ -120,10 +122,10 @@
{
final Set<Long> ids = new HashSet<Long>();
final Enumeration<String> names = session.getAttributeNames();
- while ( names.hasMoreElements() )
+ while (names.hasMoreElements())
{
final String name = names.nextElement();
- if ( name.startsWith(ATTR_LAST_ACCESSED) )
+ if (name.startsWith(ATTR_LAST_ACCESSED))
{
final String id = name.substring(ATTR_LAST_ACCESSED.length());
ids.add(Long.valueOf(id));
@@ -152,7 +154,7 @@
{
this.created = now;
this.maxTimeout = session.getMaxInactiveInterval();
- isNew = true;
+ this.isNew = true;
session.setAttribute(ATTR_CREATED + this.sessionId, this.created);
session.setAttribute(ATTR_MAX_INACTIVE + this.sessionId, this.maxTimeout);
@@ -166,7 +168,7 @@
{
this.created = (Long)session.getAttribute(ATTR_CREATED + this.sessionId);
this.maxTimeout = (Integer)session.getAttribute(ATTR_MAX_INACTIVE + this.sessionId);
- isNew = false;
+ this.isNew = false;
}
this.lastAccessed = now;
@@ -442,7 +444,6 @@
}
@Override
- @SuppressWarnings("deprecation")
public HttpSessionContext getSessionContext()
{
// no need to check validity conforming to the javadoc
diff --git a/http/base/src/test/java/org/apache/felix/http/base/internal/handler/HttpSessionWrapperTest.java b/http/base/src/test/java/org/apache/felix/http/base/internal/handler/HttpSessionWrapperTest.java
new file mode 100644
index 0000000..3904d94
--- /dev/null
+++ b/http/base/src/test/java/org/apache/felix/http/base/internal/handler/HttpSessionWrapperTest.java
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.felix.http.base.internal.handler;
+
+import static org.mockito.Mockito.*;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Set;
+
+import static org.junit.Assert.*;
+
+import javax.servlet.http.HttpSession;
+
+import org.junit.Test;
+
+/**
+ * Test cases for {@link HttpSessionWrapper}.
+ */
+public class HttpSessionWrapperTest
+{
+
+ /**
+ * FELIX-5175 - sessions are incorrectly destroyed / destroyed too soon.
+ */
+ @Test
+ public void testSessionTimeout() throws Exception
+ {
+ Set<Long> ids;
+
+ long sessionID = 123;
+ long now = System.currentTimeMillis();
+
+ HttpSession session = createMockSession(sessionID, now, 1);
+
+ ids = HttpSessionWrapper.getExpiredSessionContextIds(session);
+ assertTrue("Session should NOT be destroyed!", ids.isEmpty());
+
+ // Pretend we've accessed this session two seconds ago, which should imply it is timed out...
+ session = createMockSession(sessionID, now - 2000L, 1);
+
+ ids = HttpSessionWrapper.getExpiredSessionContextIds(session);
+ assertFalse("Session should be destroyed!", ids.isEmpty());
+ assertTrue(ids.contains(sessionID));
+ }
+
+ private HttpSession createMockSession(long sessionId, long lastAccessed, int maxInactive)
+ {
+ String attrLastAccessed = String.format("org.apache.felix.http.session.context.lastaccessed.%d", sessionId);
+ String attrMaxInactive = String.format("org.apache.felix.http.session.context.maxinactive.%d", sessionId);
+
+ HttpSession session = mock(HttpSession.class);
+ when(session.getAttributeNames()).thenReturn(Collections.enumeration(Arrays.asList(attrLastAccessed)));
+ when(session.getAttribute(eq(attrLastAccessed))).thenReturn(lastAccessed);
+ when(session.getAttribute(eq(attrMaxInactive))).thenReturn(maxInactive);
+
+ return session;
+ }
+}