You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2013/02/13 10:28:59 UTC
svn commit: r1445517 - in /tomcat/trunk: java/javax/servlet/http/
java/org/apache/catalina/connector/ java/org/apache/catalina/core/
java/org/apache/catalina/ha/session/ java/org/apache/catalina/session/
java/org/apache/catalina/websocket/ webapps/docs/
Author: markt
Date: Wed Feb 13 09:28:58 2013
New Revision: 1445517
URL: http://svn.apache.org/r1445517
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54552
Servlet 3.1
Implement HttpSessionIdListener and HttpServletRequest#changeSessionId()
Patch provided by Nick Williams.
Added:
tomcat/trunk/java/javax/servlet/http/HttpSessionIdListener.java (with props)
Modified:
tomcat/trunk/java/javax/servlet/http/HttpServletRequest.java
tomcat/trunk/java/javax/servlet/http/HttpServletRequestWrapper.java
tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties
tomcat/trunk/java/org/apache/catalina/connector/Request.java
tomcat/trunk/java/org/apache/catalina/connector/RequestFacade.java
tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java
tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java
tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
tomcat/trunk/java/org/apache/catalina/websocket/WsHttpServletRequestWrapper.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/javax/servlet/http/HttpServletRequest.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/javax/servlet/http/HttpServletRequest.java?rev=1445517&r1=1445516&r2=1445517&view=diff
==============================================================================
--- tomcat/trunk/java/javax/servlet/http/HttpServletRequest.java (original)
+++ tomcat/trunk/java/javax/servlet/http/HttpServletRequest.java Wed Feb 13 09:28:58 2013
@@ -383,6 +383,17 @@ public interface HttpServletRequest exte
public HttpSession getSession();
/**
+ * Changes the session ID of the session associated with this request. This
+ * method does not create a new session object it only changes the ID of the
+ * current session.
+ *
+ * @return the new session ID allocated to the session
+ * @see HttpSessionIdListener
+ * @since Servlet 3.1
+ */
+ public String changeSessionId();
+
+ /**
* Checks whether the requested session ID is still valid.
*
* @return <code>true</code> if this request has an id for a valid session
Modified: tomcat/trunk/java/javax/servlet/http/HttpServletRequestWrapper.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/javax/servlet/http/HttpServletRequestWrapper.java?rev=1445517&r1=1445516&r2=1445517&view=diff
==============================================================================
--- tomcat/trunk/java/javax/servlet/http/HttpServletRequestWrapper.java (original)
+++ tomcat/trunk/java/javax/servlet/http/HttpServletRequestWrapper.java Wed Feb 13 09:28:58 2013
@@ -239,6 +239,15 @@ public class HttpServletRequestWrapper e
}
/**
+ * The default behavior of this method is to call changeSessionId() on the
+ * wrapped request object.
+ */
+ @Override
+ public String changeSessionId() {
+ return this._getHttpServletRequest().changeSessionId();
+ }
+
+ /**
* The default behavior of this method is to return
* isRequestedSessionIdValid() on the wrapped request object.
*/
Added: tomcat/trunk/java/javax/servlet/http/HttpSessionIdListener.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/javax/servlet/http/HttpSessionIdListener.java?rev=1445517&view=auto
==============================================================================
--- tomcat/trunk/java/javax/servlet/http/HttpSessionIdListener.java (added)
+++ tomcat/trunk/java/javax/servlet/http/HttpSessionIdListener.java Wed Feb 13 09:28:58 2013
@@ -0,0 +1,41 @@
+/*
+ * 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 javax.servlet.http;
+
+import java.util.EventListener;
+
+/**
+ * Implementations of this interface are notified when an {@link HttpSession}'s
+ * ID changes. To receive notification events, the implementation class must be
+ * configured in the deployment descriptor for the web application, annotated
+ * with {@link javax.servlet.annotation.WebListener} or registered by calling an
+ * addListener method on the {@link javax.servlet.ServletContext}.
+ *
+ * @see HttpSessionEvent
+ * @see HttpServletRequest#changeSessionId()
+ * @since Servlet 3.1
+ */
+public interface HttpSessionIdListener extends EventListener {
+
+ /**
+ * Notification that a session ID has been changed.
+ *
+ * @param se the notification event
+ * @param oldSessionId the old session ID
+ */
+ public void sessionIdChanged(HttpSessionEvent se, String oldSessionId);
+}
Propchange: tomcat/trunk/java/javax/servlet/http/HttpSessionIdListener.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified: tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties?rev=1445517&r1=1445516&r2=1445517&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties Wed Feb 13 09:28:58 2013
@@ -52,6 +52,7 @@ coyoteResponse.setBufferSize.ise=Cannot
coyoteRequest.getInputStream.ise=getReader() has already been called for this request
coyoteRequest.getReader.ise=getInputStream() has already been called for this request
coyoteRequest.sessionCreateCommitted=Cannot create a session after the response has been committed
+coyoteRequest.changeSessionId=Cannot change session ID. There is no session associated with this request.
coyoteRequest.setAttribute.namenull=Cannot call setAttribute with a null name
coyoteRequest.attributeEvent=Exception thrown by attributes event listener
coyoteRequest.parseParameters=Exception thrown whilst processing POSTed parameters
Modified: tomcat/trunk/java/org/apache/catalina/connector/Request.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Request.java?rev=1445517&r1=1445516&r2=1445517&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Wed Feb 13 09:28:58 2013
@@ -2362,6 +2362,30 @@ public class Request
}
}
+ /**
+ * Changes the session ID of the session associated with this request.
+ *
+ * @return the old session ID before it was changed
+ * @see javax.servlet.http.HttpSessionIdListener
+ * @since Servlet 3.1
+ */
+ @Override
+ public String changeSessionId() {
+
+ Session session = this.getSessionInternal(false);
+ if (session == null) {
+ throw new IllegalStateException(
+ sm.getString("coyoteRequest.changeSessionId"));
+ }
+
+ Manager manager = this.getContext().getManager();
+ manager.changeSessionId(session);
+
+ String newSessionId = session.getId();
+ this.changeSessionId(newSessionId);
+
+ return newSessionId;
+ }
/**
* Return the session associated with this Request, creating one
Modified: tomcat/trunk/java/org/apache/catalina/connector/RequestFacade.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/RequestFacade.java?rev=1445517&r1=1445516&r2=1445517&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/RequestFacade.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/RequestFacade.java Wed Feb 13 09:28:58 2013
@@ -910,6 +910,16 @@ public class RequestFacade implements Ht
return getSession(true);
}
+ @Override
+ public String changeSessionId() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
+ return request.changeSessionId();
+ }
@Override
public boolean isRequestedSessionIdValid() {
Modified: tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java?rev=1445517&r1=1445516&r2=1445517&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java Wed Feb 13 09:28:58 2013
@@ -50,6 +50,7 @@ import javax.servlet.SessionCookieConfig
import javax.servlet.SessionTrackingMode;
import javax.servlet.descriptor.JspConfigDescriptor;
import javax.servlet.http.HttpSessionAttributeListener;
+import javax.servlet.http.HttpSessionIdListener;
import javax.servlet.http.HttpSessionListener;
import org.apache.catalina.Container;
@@ -1282,6 +1283,7 @@ public class ApplicationContext
if (t instanceof ServletContextAttributeListener ||
t instanceof ServletRequestListener ||
t instanceof ServletRequestAttributeListener ||
+ t instanceof HttpSessionIdListener ||
t instanceof HttpSessionAttributeListener) {
context.addApplicationEventListener(t);
match = true;
@@ -1319,6 +1321,7 @@ public class ApplicationContext
listener instanceof ServletRequestListener ||
listener instanceof ServletRequestAttributeListener ||
listener instanceof HttpSessionListener ||
+ listener instanceof HttpSessionIdListener ||
listener instanceof HttpSessionAttributeListener) {
return listener;
}
Modified: tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardContext.java?rev=1445517&r1=1445516&r2=1445517&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardContext.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardContext.java Wed Feb 13 09:28:58 2013
@@ -65,6 +65,7 @@ import javax.servlet.ServletRequestListe
import javax.servlet.ServletSecurityElement;
import javax.servlet.descriptor.JspConfigDescriptor;
import javax.servlet.http.HttpSessionAttributeListener;
+import javax.servlet.http.HttpSessionIdListener;
import javax.servlet.http.HttpSessionListener;
import org.apache.catalina.Authenticator;
@@ -4632,6 +4633,7 @@ public class StandardContext extends Con
if ((results[i] instanceof ServletContextAttributeListener)
|| (results[i] instanceof ServletRequestAttributeListener)
|| (results[i] instanceof ServletRequestListener)
+ || (results[i] instanceof HttpSessionIdListener)
|| (results[i] instanceof HttpSessionAttributeListener)) {
eventListeners.add(results[i]);
}
Modified: tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java?rev=1445517&r1=1445516&r2=1445517&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java (original)
+++ tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java Wed Feb 13 09:28:58 2013
@@ -26,6 +26,9 @@ import java.util.ArrayList;
import java.util.Date;
import java.util.Iterator;
+import javax.servlet.http.HttpSessionEvent;
+import javax.servlet.http.HttpSessionIdListener;
+
import org.apache.catalina.Cluster;
import org.apache.catalina.Container;
import org.apache.catalina.Context;
@@ -1471,6 +1474,30 @@ public class DeltaManager extends Cluste
getContext().fireContainerEvent(Context.CHANGE_SESSION_ID_EVENT,
new String[] {msg.getSessionID(), newSessionID});
}
+
+ if (notifySessionListenersOnReplication) {
+ Object listeners[] = getContext().
+ getApplicationEventListeners();
+ if (listeners != null && listeners.length > 0) {
+ HttpSessionEvent event =
+ new HttpSessionEvent(session.getSession());
+
+ for(Object listener : listeners) {
+ if (!(listener instanceof HttpSessionIdListener))
+ continue;
+
+ HttpSessionIdListener idListener =
+ (HttpSessionIdListener)listener;
+ try {
+ idListener.
+ sessionIdChanged(event, msg.getSessionID());
+ } catch (Throwable t) {
+ log.error(sm.getString(
+ "standardSession.sessionEvent"), t);
+ }
+ }
+ }
+ }
}
}
Modified: tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java?rev=1445517&r1=1445516&r2=1445517&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java (original)
+++ tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java Wed Feb 13 09:28:58 2013
@@ -33,6 +33,9 @@ import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicLong;
+import javax.servlet.http.HttpSessionEvent;
+import javax.servlet.http.HttpSessionIdListener;
+
import org.apache.catalina.Container;
import org.apache.catalina.Context;
import org.apache.catalina.Engine;
@@ -762,6 +765,25 @@ public abstract class ManagerBase extend
String newId = session.getIdInternal();
context.fireContainerEvent(Context.CHANGE_SESSION_ID_EVENT,
new String[] {oldId, newId});
+
+ Object listeners[] = context.getApplicationEventListeners();
+ if (listeners != null && listeners.length > 0) {
+ HttpSessionEvent event =
+ new HttpSessionEvent(session.getSession());
+
+ for(Object listener : listeners) {
+ if (!(listener instanceof HttpSessionIdListener))
+ continue;
+
+ HttpSessionIdListener idListener =
+ (HttpSessionIdListener)listener;
+ try {
+ idListener.sessionIdChanged(event, oldId);
+ } catch (Throwable t) {
+ log.error(sm.getString("standardSession.sessionEvent"), t);
+ }
+ }
+ }
}
Modified: tomcat/trunk/java/org/apache/catalina/websocket/WsHttpServletRequestWrapper.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/websocket/WsHttpServletRequestWrapper.java?rev=1445517&r1=1445516&r2=1445517&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/websocket/WsHttpServletRequestWrapper.java (original)
+++ tomcat/trunk/java/org/apache/catalina/websocket/WsHttpServletRequestWrapper.java Wed Feb 13 09:28:58 2013
@@ -362,6 +362,11 @@ public class WsHttpServletRequestWrapper
}
@Override
+ public String changeSessionId() {
+ return getRequest().changeSessionId();
+ }
+
+ @Override
public boolean isRequestedSessionIdValid() {
return getRequest().isRequestedSessionIdValid();
}
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1445517&r1=1445516&r2=1445517&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Wed Feb 13 09:28:58 2013
@@ -111,6 +111,12 @@
Port storeconfig functionality, which can persist to server.xml and
context.xml runtime container configuration changes. (remm)
</add>
+ <add>
+ <bug>54552</bug>: Servlet 3.1. Implement
+ <code>HttpSessionIdListener</code> and
+ <code>HttpServletRequest#changeSessionId()</code>. Patch provided by
+ Nick Williams. (markt)
+ </add>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1445517 - in /tomcat/trunk: java/javax/servlet/http/ java/org/apache/catalina/connector/ java/org/apache/catalina/core/ java/org/apache/catalina/ha/session/ java/org/apache/catalina/session/ java/org/apache/catalina/websocket/ webapps/...
Posted by Nick Williams <ni...@nicholaswilliams.net>.
On Feb 13, 2013, at 4:26 AM, Keiichi Fujino wrote:
> 2013/2/13 <ma...@apache.org>:
>> Author: markt
>> Date: Wed Feb 13 09:28:58 2013
>> New Revision: 1445517
>>
>> URL: http://svn.apache.org/r1445517
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54552
>> Servlet 3.1
>> Implement HttpSessionIdListener and HttpServletRequest#changeSessionId()
>> Patch provided by Nick Williams.
>>
> I think that this code itself is not a problem at all. (I think prety good.)
Thanks! I was a bit nervous about my first code contribution. :-)
> However I have a idea of code improvement.
>
> There seems to be a duplicate code on ManagerBase and DeltaManager.
I struggled with that, too. There's a tellNew() method in StandardSession that is responsible for firing the Session.SESSION_CREATED_EVENT event and notifying HttpSessionListeners, and I thought about implementing a similar tellChangedSessionId method in StandardSession. However, since the code for firing the Context.CHANGE_SESSION_ID_EVENT internal event was also duplicated in ManagerBase and DeltaManager, I decided to stick with doing the same and keeping the code alongside those event firings. I would not disagree that the code should be centralized…in both cases.
> e.g.
> By introducing a following method to ManagerBase and DeltaManager,
> we might be able to avoid code duplication.
>
> ==
> changeSessionId(Session session, String newId)
> changeSessionId(Session session, String newId,
> boolean notifySessionListeners, boolean notifyContainerListeners)
> ==
If you are going to add a method to prevent code duplication, I would recommend it be something like StandardSession.tellChangedSessionId(String, String, boolean, boolean) (perhaps right below the tellNew() method) instead of in the ManagerBase class.
>
> And furthermore, we are changing sessionId in JvmRouteBinderValve.
> Change sessionid of JvmRouteBinderValve is completely different from
> Manager#changeSessionId.
> By using new changeSessionId method, will be able to change sessionId
> in a same way.
> As a result, JvmRouteSessionIDBinderListener will be unnecessary.
I took a look at JvmRouteSessionIDBinderListener and I do not believe it is changing the session ID in the same way (though I could certainly be wrong). It looks to me like it is involved in binding the JVMRoute to the end of the session ID in order to support clustering? I do not think that is the same thing as this.
Other areas in the code that "change" the session ID (changeSessionIdOnAuthentication, change replicated from another cluster, HttpServletRequest.changeSessionId()) call the changeSessionId(Session) method on the Manager, which changes the sessionId, does NOT call tellNew(), fires Context.CHANGE_SESSION_ID_EVENT and (now) notifies HttpSessionIdListeners. However, JvmRouteSessionIDBinderListener does it differently: it calls setId() directly on the session, DOES call tellNew() (is this right?) and bypasses the Context.CHANGE_SESSION_ID_EVENT and listeners by not calling Manager.changeSessionId(Session).
It could be that this was done this way intentionally, or not. I don't know. I'd like to understand it better. Someone more in-the-know than I can hopefully chime in.
>
> I'm going to fix these improvements If there is no objections from anyone.
> Any objections and comment?
If my code can be improved, I am always happy for someone to do it!
Nick
Re: svn commit: r1445517 - in /tomcat/trunk: java/javax/servlet/http/
java/org/apache/catalina/connector/ java/org/apache/catalina/core/
java/org/apache/catalina/ha/session/ java/org/apache/catalina/session/
java/org/apache/catalina/websocket/ webapps/docs/
Posted by Keiichi Fujino <kf...@apache.org>.
2013/2/13 <ma...@apache.org>:
> Author: markt
> Date: Wed Feb 13 09:28:58 2013
> New Revision: 1445517
>
> URL: http://svn.apache.org/r1445517
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54552
> Servlet 3.1
> Implement HttpSessionIdListener and HttpServletRequest#changeSessionId()
> Patch provided by Nick Williams.
>
I think that this code itself is not a problem at all. (I think prety good.)
However I have a idea of code improvement.
There seems to be a duplicate code on ManagerBase and DeltaManager.
e.g.
By introducing a following method to ManagerBase and DeltaManager,
we might be able to avoid code duplication.
==
changeSessionId(Session session, String newId)
changeSessionId(Session session, String newId,
boolean notifySessionListeners, boolean notifyContainerListeners)
==
And furthermore, we are changing sessionId in JvmRouteBinderValve.
Change sessionid of JvmRouteBinderValve is completely different from
Manager#changeSessionId.
By using new changeSessionId method, will be able to change sessionId
in a same way.
As a result, JvmRouteSessionIDBinderListener will be unnecessary.
I'm going to fix these improvements If there is no objections from anyone.
Any objections and comment?
--
Keiichi.Fujino
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org