You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by pb...@apache.org on 2008/11/26 23:00:07 UTC

svn commit: r720996 - /struts/struts1/trunk/core/src/main/java/org/apache/struts/dispatcher/AbstractDispatcher.java

Author: pbenedict
Date: Wed Nov 26 14:00:07 2008
New Revision: 720996

URL: http://svn.apache.org/viewvc?rev=720996&view=rev
Log:
STR-3168: Leave responsibility of redelegating cancelled requests to the target

Modified:
    struts/struts1/trunk/core/src/main/java/org/apache/struts/dispatcher/AbstractDispatcher.java

Modified: struts/struts1/trunk/core/src/main/java/org/apache/struts/dispatcher/AbstractDispatcher.java
URL: http://svn.apache.org/viewvc/struts/struts1/trunk/core/src/main/java/org/apache/struts/dispatcher/AbstractDispatcher.java?rev=720996&r1=720995&r2=720996&view=diff
==============================================================================
--- struts/struts1/trunk/core/src/main/java/org/apache/struts/dispatcher/AbstractDispatcher.java (original)
+++ struts/struts1/trunk/core/src/main/java/org/apache/struts/dispatcher/AbstractDispatcher.java Wed Nov 26 14:00:07 2008
@@ -31,7 +31,6 @@
 import java.lang.reflect.Method;
 import java.util.HashMap;
 
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
@@ -77,7 +76,7 @@
     /**
      * Shared commons Logging instance among subclasses.
      */
-    protected final Log log;
+    protected transient final Log log;
 
     /**
      * The dictionary of {@link Method} objects we have introspected for this
@@ -95,21 +94,8 @@
 	methods = new HashMap();
     }
 
-    protected Object cancelled(ActionContext context) throws Exception {
-	Method method = getMethod(context, CANCELLED_METHOD_NAME);
-	return dispatchMethod(context, method, CANCELLED_METHOD_NAME);
-    }
-
     public Object dispatch(ActionContext context) throws Exception {
-	// Process cancelled requests
-	if (isCancelled(context)) {
-	    try {
-		return cancelled(context);
-	    } catch (NoSuchMethodException e) {
-		// expected and ignore
-	    }
-	}
-
+	// Resolve the method name; fallback to default if necessary
 	String methodName = resolveMethodName(context);
 	if ((methodName == null) || "".equals(methodName)) {
 	    methodName = getDefaultMethodName();
@@ -126,10 +112,13 @@
 	try {
 	    method = getMethod(context, methodName);
 	} catch (NoSuchMethodException e) {
+	    // The log message reveals the offending method name...
 	    String path = context.getActionConfig().getPath();
 	    String message = messages.getMessage(MSG_KEY_MISSING_METHOD_LOG, path, methodName);
 	    log.error(message, e);
 
+	    // ...but the exception thrown does not
+	    // See r383718 (XSS vulnerability)
 	    String userMsg = messages.getMessage(MSG_KEY_MISSING_METHOD, path);
 	    NoSuchMethodException e2 = new NoSuchMethodException(userMsg);
 	    e2.initCause(e);
@@ -152,7 +141,7 @@
      * 
      * @return The forward to which control should be transferred, or
      *         <code>null</code> if the response has been completed.
-     * @throws Exception if the application business logic throws an exception.
+     * @throws Exception if the dispatch fails with an exception
      * @see #dispatchMethod(ActionMapping, ActionForm, HttpServletRequest,
      *      HttpServletResponse, String)
      */
@@ -187,10 +176,11 @@
      * will be the target of the dispatch. This implementation caches the method
      * instance for subsequent invocations.
      * 
+     * @param context the current action context
      * @param methodName the name of the method to be introspected
      * @return the method of the specified name
      * @throws NoSuchMethodException if no such method can be found
-     * @see #resolveMethod(ActionContext, String)
+     * @see #resolveMethod(String, ActionContext)
      * @see #flushMethodCache()
      */
     protected final Method getMethod(ActionContext context, String methodName) throws NoSuchMethodException {
@@ -205,7 +195,7 @@
 	    Method method = (Method) methods.get(key);
 
 	    if (method == null) {
-		method = resolveMethod(methodName, context);
+		method = resolveMethod(context, methodName);
 		methods.put(key, method);
 	    }
 
@@ -213,24 +203,34 @@
 	}
     }
 
-    protected final Object invoke(Object target, Method method, Object[] args, String path, String name)
-	    throws Exception {
+    /**
+     * Convenience method to help dispatch the specified method. The method is
+     * invoked via reflection.
+     * 
+     * @param target the target object
+     * @param method the method of the target object
+     * @param args the arguments for the method
+     * @param path the mapping path
+     * @return the return value of the method
+     * @throws Exception if the dispatch fails with an exception
+     */
+    protected final Object invoke(Object target, Method method, Object[] args, String path) throws Exception {
 	try {
 	    return method.invoke(target, args);
 	} catch (IllegalAccessException e) {
 	    String message = messages.getMessage(MSG_KEY_DISPATCH_ERROR, path);
-	    log.error(message + ":" + name, e);
+	    log.error(message + ":" + method.getName(), e);
 	    throw e;
 	} catch (InvocationTargetException e) {
 	    // Rethrow the target exception if possible so that the
 	    // exception handling machinery can deal with it
 	    Throwable t = e.getTargetException();
 	    if (t instanceof Exception) {
-		throw ((Exception) t);
+		throw (Exception) t;
 	    } else {
 		String message = messages.getMessage(MSG_KEY_DISPATCH_ERROR, path);
-		log.error(message + ":" + name, e);
-		throw new ServletException(t);
+		log.error(message + ":" + method.getName(), e);
+		throw new Exception(t);
 	    }
 	}
     }
@@ -258,21 +258,22 @@
      * resolution is only invoked if {@link #getMethod(String)} does not find a
      * match in its method cache.
      * 
-     * @param methodName the method name to use for introspection
      * @param context the current action context
+     * @param methodName the method name to use for introspection
      * @return the method to invoke
      * @throws NoSuchMethodException if an appropriate method cannot be found
      * @see #getMethod(String)
      * @see #invoke(Object, Method, Object[], String, String)
      */
-    protected abstract Method resolveMethod(String methodName, ActionContext context) throws NoSuchMethodException;
+    protected abstract Method resolveMethod(ActionContext context, String methodName) throws NoSuchMethodException;
 
     /**
      * Decides the method name that can handle the request.
      * 
      * @param context the current action context
      * @return the method name or <code>null</code> if no name can be resolved
-     * @see #resolveMethod(String, ActionContext)
+     * @see #getDefaultMethodName()
+     * @see #resolveMethod(ActionContext, String)
      */
     protected abstract String resolveMethodName(ActionContext context);
 
@@ -280,12 +281,12 @@
      * Services the case when the dispatch fails because the method name cannot
      * be resolved. The default behavior throws an {@link IllegalStateException}.
      * Subclasses should override this to provide custom handling such as
-     * sending an HTTP 404 error.
+     * sending an HTTP 404 error or dispatching elsewhere.
      * 
      * @param context the current action context
-     * @throws IllegalStateException always unless supressed by subclass
+     * @return the return value of the dispatch
+     * @throws Exception if an error occurs
      * @see #resolveMethodName(ActionContext)
-     * @see #getDefaultMethodName()
      */
     protected Object unspecified(ActionContext context) throws Exception {
 	ActionConfig config = context.getActionConfig();