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 2012/08/14 23:05:56 UTC

svn commit: r1373101 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/ java/org/apache/catalina/core/ webapps/docs/

Author: markt
Date: Tue Aug 14 21:05:55 2012
New Revision: 1373101

URL: http://svn.apache.org/viewvc?rev=1373101&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53623
Async dispatches require similar handling to RequestDispatcher.include() hence the implementation of async dispatch uses RequestDispatcher.include()
However, since RequestDispatcher provides no API for differentiating between and INCLUDE and DISPATCH some form of trickery has to be used. The previous implementation worked in most cases but failed in some cases (multiple forwards prior to dispatch, wrapping the request).
This alternative implementation calls a specific method on the RequestDispatcher implementation (we have to access the implementation class since we can't modify the servlet API) which removes any ambiguity and allows the correct handling in all cases.

Added:
    tomcat/tc7.0.x/trunk/java/org/apache/catalina/AsyncDispatcher.java
      - copied unchanged from r1373080, tomcat/trunk/java/org/apache/catalina/AsyncDispatcher.java
Modified:
    tomcat/tc7.0.x/trunk/   (props changed)
    tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/ApplicationDispatcher.java
    tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
    tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties
    tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
  Merged /tomcat/trunk:r1373080

Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/ApplicationDispatcher.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/ApplicationDispatcher.java?rev=1373101&r1=1373100&r2=1373101&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/ApplicationDispatcher.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/ApplicationDispatcher.java Tue Aug 14 21:05:55 2012
@@ -14,8 +14,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
-
 package org.apache.catalina.core;
 
 import java.io.IOException;
@@ -37,6 +35,7 @@ import javax.servlet.UnavailableExceptio
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.catalina.AsyncDispatcher;
 import org.apache.catalina.Context;
 import org.apache.catalina.Globals;
 import org.apache.catalina.InstanceEvent;
@@ -63,9 +62,7 @@ import org.apache.tomcat.util.res.String
  * @author Craig R. McClanahan
  * @version $Id$
  */
-
-final class ApplicationDispatcher
-    implements RequestDispatcher {
+final class ApplicationDispatcher implements AsyncDispatcher, RequestDispatcher {
 
     protected static final boolean STRICT_SERVLET_COMPLIANCE;
 
@@ -91,8 +88,7 @@ final class ApplicationDispatcher
         private ServletRequest request;
         private ServletResponse response;
 
-        PrivilegedForward(ServletRequest request, ServletResponse response)
-        {
+        PrivilegedForward(ServletRequest request, ServletResponse response) {
             this.request = request;
             this.response = response;
         }
@@ -109,22 +105,36 @@ final class ApplicationDispatcher
         private ServletRequest request;
         private ServletResponse response;
 
-        PrivilegedInclude(ServletRequest request, ServletResponse response)
-        {
+        PrivilegedInclude(ServletRequest request, ServletResponse response) {
             this.request = request;
             this.response = response;
         }
 
         @Override
         public Void run() throws ServletException, IOException {
-            DispatcherType type = DispatcherType.INCLUDE;
-            if (request.getDispatcherType()==DispatcherType.ASYNC) type = DispatcherType.ASYNC; 
-            doInclude(request,response,type);
+            doInclude(request, response);
             return null;
         }
     }
 
-    
+    protected class PrivilegedDispatch implements
+            PrivilegedExceptionAction<Void> {
+        private final ServletRequest request;
+        private final ServletResponse response;
+
+        PrivilegedDispatch(ServletRequest request, ServletResponse response) {
+            this.request = request;
+            this.response = response;
+        }
+
+        @Override
+        public Void run() throws ServletException, IOException {
+            doDispatch(request, response);
+            return null;
+        }
+    }
+
+
     /**
      * Used to pass state when the request dispatcher is used. Using instance
      * variables causes threading issues and state is too complex to pass and
@@ -531,15 +541,13 @@ final class ApplicationDispatcher
                 throw (IOException) e;
             }
         } else {
-            DispatcherType type = DispatcherType.INCLUDE;
-            if (request.getDispatcherType()==DispatcherType.ASYNC) type = DispatcherType.ASYNC; 
-            doInclude(request,response,type);
+            doInclude(request, response);
         }
     }
 
-    private void doInclude(ServletRequest request, ServletResponse response, DispatcherType type)
-        throws ServletException, IOException
-    {
+    private void doInclude(ServletRequest request, ServletResponse response)
+            throws ServletException, IOException {
+
         // Set up to handle the specified request and response
         State state = new State(request, response, true);
 
@@ -559,7 +567,8 @@ final class ApplicationDispatcher
             wrequest.setAttribute(Globals.NAMED_DISPATCHER_ATTR, name);
             if (servletPath != null)
                 wrequest.setServletPath(servletPath);
-            wrequest.setAttribute(Globals.DISPATCHER_TYPE_ATTR, type);
+            wrequest.setAttribute(Globals.DISPATCHER_TYPE_ATTR,
+                    DispatcherType.INCLUDE);
             wrequest.setAttribute(Globals.DISPATCHER_REQUEST_PATH_ATTR,
                     getCombinedPath());
             invoke(state.outerRequest, state.outerResponse, state);
@@ -589,7 +598,8 @@ final class ApplicationDispatcher
                 wrequest.setQueryParams(queryString);
             }
             
-            wrequest.setAttribute(Globals.DISPATCHER_TYPE_ATTR, type);
+            wrequest.setAttribute(Globals.DISPATCHER_TYPE_ATTR,
+                    DispatcherType.INCLUDE);
             wrequest.setAttribute(Globals.DISPATCHER_REQUEST_PATH_ATTR,
                     getCombinedPath());
             invoke(state.outerRequest, state.outerResponse, state);
@@ -598,6 +608,73 @@ final class ApplicationDispatcher
     }
 
 
+    @Override
+    public void dispatch(ServletRequest request, ServletResponse response)
+            throws ServletException, IOException {
+        if (Globals.IS_SECURITY_ENABLED) {
+            try {
+                PrivilegedDispatch dp = new PrivilegedDispatch(request,response);
+                AccessController.doPrivileged(dp);
+            } catch (PrivilegedActionException pe) {
+                Exception e = pe.getException();
+
+                if (e instanceof ServletException)
+                    throw (ServletException) e;
+                throw (IOException) e;
+            }
+        } else {
+            doDispatch(request, response);
+        }
+    }
+
+    private void doDispatch(ServletRequest request, ServletResponse response)
+            throws ServletException, IOException {
+
+        // Set up to handle the specified request and response
+        State state = new State(request, response, true);
+
+        // Create a wrapped response to use for this request
+        wrapResponse(state);
+
+        ApplicationHttpRequest wrequest =
+            (ApplicationHttpRequest) wrapRequest(state);
+        String contextPath = context.getPath();
+        if (requestURI != null)
+            wrequest.setAttribute(RequestDispatcher.INCLUDE_REQUEST_URI,
+                                  requestURI);
+        if (contextPath != null)
+            wrequest.setAttribute(RequestDispatcher.INCLUDE_CONTEXT_PATH,
+                                  contextPath);
+        if (servletPath != null)
+            wrequest.setAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH,
+                                  servletPath);
+        if (pathInfo != null)
+            wrequest.setAttribute(RequestDispatcher.INCLUDE_PATH_INFO,
+                                  pathInfo);
+        if (queryString != null) {
+            wrequest.setAttribute(RequestDispatcher.INCLUDE_QUERY_STRING,
+                                  queryString);
+            wrequest.setQueryParams(queryString);
+        }
+
+        wrequest.setAttribute(Globals.DISPATCHER_TYPE_ATTR,
+                DispatcherType.ASYNC);
+        wrequest.setAttribute(Globals.DISPATCHER_REQUEST_PATH_ATTR,
+                getCombinedPath());
+
+        wrequest.setContextPath(contextPath);
+        wrequest.setRequestURI(requestURI);
+        wrequest.setServletPath(servletPath);
+        wrequest.setPathInfo(pathInfo);
+        if (queryString != null) {
+            wrequest.setQueryString(queryString);
+            wrequest.setQueryParams(queryString);
+        }
+
+        invoke(state.outerRequest, state.outerResponse, state);
+    }
+
+
     // -------------------------------------------------------- Private Methods
 
 
@@ -781,6 +858,12 @@ final class ApplicationDispatcher
         if (state.wrapRequest == null)
             return;
 
+        if (state.outerRequest.isAsyncStarted()) {
+            if (!state.outerRequest.getAsyncContext().hasOriginalRequestAndResponse()) {
+                return;
+            }
+        }
+
         ServletRequest previous = null;
         ServletRequest current = state.outerRequest;
         while (current != null) {
@@ -817,6 +900,12 @@ final class ApplicationDispatcher
         if (state.wrapResponse == null)
             return;
 
+        if (state.outerRequest.isAsyncStarted()) {
+            if (!state.outerRequest.getAsyncContext().hasOriginalRequestAndResponse()) {
+                return;
+            }
+        }
+
         ServletResponse previous = null;
         ServletResponse current = state.outerResponse;
         while (current != null) {

Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=1373101&r1=1373100&r2=1373101&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Tue Aug 14 21:05:55 2012
@@ -30,7 +30,6 @@ import javax.naming.NamingException;
 import javax.servlet.AsyncContext;
 import javax.servlet.AsyncEvent;
 import javax.servlet.AsyncListener;
-import javax.servlet.DispatcherType;
 import javax.servlet.RequestDispatcher;
 import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
@@ -39,6 +38,7 @@ import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.catalina.AsyncDispatcher;
 import org.apache.catalina.Context;
 import org.apache.catalina.Globals;
 import org.apache.catalina.connector.Request;
@@ -159,24 +159,25 @@ public class AsyncContextImpl implements
             request.setAttribute(ASYNC_QUERY_STRING, request.getQueryString());
         }
         final RequestDispatcher requestDispatcher = context.getRequestDispatcher(path);
-        final HttpServletRequest servletRequest = (HttpServletRequest)getRequest();
-        final HttpServletResponse servletResponse = (HttpServletResponse)getResponse();
+        if (!(requestDispatcher instanceof AsyncDispatcher)) {
+            throw new UnsupportedOperationException(
+                    sm.getString("asyncContextImpl.noAsyncDispatcher"));
+        }
+        final AsyncDispatcher applicationDispatcher =
+                (AsyncDispatcher) requestDispatcher;
+        final HttpServletRequest servletRequest =
+                (HttpServletRequest) getRequest();
+        final HttpServletResponse servletResponse =
+                (HttpServletResponse) getResponse();
         Runnable run = new Runnable() {
             @Override
             public void run() {
                 request.getCoyoteRequest().action(ActionCode.ASYNC_DISPATCHED, null);
-                DispatcherType type = (DispatcherType)request.getAttribute(Globals.DISPATCHER_TYPE_ATTR);
                 try {
-                    //piggy back on the request dispatcher to ensure that filters etc get called.
-                    //TODO SERVLET3 - async should this be include/forward or a new dispatch type
-                    //javadoc suggests include with the type of DispatcherType.ASYNC
-                    request.setAttribute(Globals.DISPATCHER_TYPE_ATTR, DispatcherType.ASYNC);
-                    requestDispatcher.include(servletRequest, servletResponse);
+                    applicationDispatcher.dispatch(servletRequest, servletResponse);
                 }catch (Exception x) {
                     //log.error("Async.dispatch",x);
                     throw new RuntimeException(x);
-                }finally {
-                    request.setAttribute(Globals.DISPATCHER_TYPE_ATTR, type);
                 }
             }
         };

Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties?rev=1373101&r1=1373100&r2=1373101&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties Tue Aug 14 21:05:55 2012
@@ -68,6 +68,7 @@ aprListener.tooLateForFIPSMode=Cannot se
 aprListener.initializedOpenSSL=OpenSSL successfully initialized ({0})
 
 asyncContextImpl.requestEnded=The request associated with the AsyncContext has already completed processing.
+asyncContextImpl.noAsyncDispatcher=The dispatcher returned from the ServletContext does not support asynchronous dispatching
 containerBase.threadedStartFailed=A child container failed during start
 containerBase.threadedStopFailed=A child container failed during stop
 containerBase.backgroundProcess.cluster=Exception processing cluster {0} background process

Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1373101&r1=1373100&r2=1373101&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Tue Aug 14 21:05:55 2012
@@ -118,6 +118,11 @@
         (markt)
       </fix>
       <fix>
+        <bug>53623</bug>: When performing a asynchronous dispatch after series
+        of forwards, ensure that the request properties are correct for the
+        request at each stage. (markt) 
+      </fix>
+      <fix>
         <bug>53641</bug>: Correct name of HTTP header used in WebSocket
         handshake for listing the preferred protocols. (markt)
       </fix>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org