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 2007/01/20 04:07:37 UTC
svn commit: r498053 - in /tomcat/container/tc5.5.x/catalina: build.xml
src/share/org/apache/catalina/core/StandardWrapper.java
Author: markt
Date: Fri Jan 19 19:07:36 2007
New Revision: 498053
URL: http://svn.apache.org/viewvc?view=rev&rev=498053
Log:
Put the fix back for 39088 now the build is fixed. Sorry for the noise.
Modified:
tomcat/container/tc5.5.x/catalina/build.xml
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/StandardWrapper.java
Modified: tomcat/container/tc5.5.x/catalina/build.xml
URL: http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/build.xml?view=diff&rev=498053&r1=498052&r2=498053
==============================================================================
--- tomcat/container/tc5.5.x/catalina/build.xml (original)
+++ tomcat/container/tc5.5.x/catalina/build.xml Fri Jan 19 19:07:36 2007
@@ -30,6 +30,7 @@
<!-- Dependent JARs and files -->
<property name="ant.jar" value="${ant.home}/lib/ant.jar"/>
<property name="servlet-api.jar" value="${api.home}/jsr154/dist/lib/servlet-api.jar"/>
+ <property name="jsp-api.jar" value="${api.home}/jsr152/dist/lib/jsp-api.jar"/>
<property name="tomcat-util.jar"
value="${tomcat-util.home}/build/lib/tomcat-util.jar"/>
<property name="tomcat-coyote.jar"
@@ -72,6 +73,7 @@
<pathelement location="${mail.jar}"/>
<pathelement location="${regexp.jar}"/>
<pathelement location="${servlet-api.jar}"/>
+ <pathelement location="${jsp-api.jar}"/>
<pathelement location="${xercesImpl.jar}"/>
<pathelement location="${xml-apis.jar}"/>
<pathelement location="${classes.dir}"/>
@@ -101,6 +103,7 @@
<pathelement location="${mail.jar}"/>
<pathelement location="${regexp.jar}"/>
<pathelement location="${servlet-api.jar}"/>
+ <pathelement location="${jsp-api.jar}"/>
<pathelement location="${xercesImpl.jar}"/>
<pathelement location="${xml-apis.jar}"/>
<pathelement location="${classes.dir}"/>
Modified: tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/StandardWrapper.java
URL: http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/StandardWrapper.java?view=diff&rev=498053&r1=498052&r2=498053
==============================================================================
--- tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/StandardWrapper.java (original)
+++ tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/StandardWrapper.java Fri Jan 19 19:07:36 2007
@@ -28,6 +28,8 @@
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
+import java.sql.SQLException;
+
import javax.servlet.Servlet;
import javax.servlet.ServletConfig;
import javax.servlet.ServletContext;
@@ -36,6 +38,7 @@
import javax.servlet.ServletResponse;
import javax.servlet.SingleThreadModel;
import javax.servlet.UnavailableException;
+import javax.servlet.jsp.JspException;
import javax.management.ListenerNotFoundException;
import javax.management.MBeanNotificationInfo;
import javax.management.Notification;
@@ -56,7 +59,6 @@
import org.apache.catalina.security.SecurityUtil;
import org.apache.catalina.util.Enumerator;
import org.apache.catalina.util.InstanceSupport;
-import org.apache.tomcat.util.IntrospectionUtils;
import org.apache.tomcat.util.log.SystemLogHandler;
import org.apache.commons.modeler.Registry;
@@ -632,23 +634,33 @@
* @param e The servlet exception
*/
public static Throwable getRootCause(ServletException e) {
- Throwable rootCause = e;
- Throwable rootCauseCheck = null;
- // Extra aggressive rootCause finding
- do {
- try {
- rootCauseCheck = (Throwable)IntrospectionUtils.getProperty
- (rootCause, "rootCause");
- if (rootCause == rootCauseCheck)
- rootCauseCheck = null;
- else if (rootCauseCheck != null)
- rootCause = rootCauseCheck;
+ Throwable rootCause = e.getRootCause();
+ return findRootCause(e, rootCause);
+ }
- } catch (ClassCastException ex) {
- rootCauseCheck = null;
- }
- } while (rootCauseCheck != null);
- return rootCause;
+ /*
+ * Work through the root causes using specific methods for well known types
+ * and getCause() for the rest. Stop when the next rootCause is null or
+ * an exception is found that has itself as its own rootCause.
+ */
+ private static final Throwable findRootCause(Throwable theException,
+ Throwable theRootCause) {
+
+ Throwable deeperRootCause = null;
+
+ if (theRootCause == null || theRootCause == theException) {
+ deeperRootCause = theException;
+ } else if (theRootCause instanceof ServletException) {
+ deeperRootCause = ((ServletException) theRootCause).getRootCause();
+ } else if (theRootCause instanceof JspException) {
+ deeperRootCause = ((JspException) theRootCause).getRootCause();
+ } else if (theRootCause instanceof SQLException) {
+ deeperRootCause = ((SQLException) theRootCause).getNextException();
+ } else {
+ deeperRootCause = theRootCause.getCause();
+ }
+
+ return deeperRootCause;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r498053 - in /tomcat/container/tc5.5.x/catalina:
build.xml src/share/org/apache/catalina/core/StandardWrapper.java
Posted by Tim Funk <fu...@joedog.org>.
I guess one could do this:
Create instance variable called JspException which is initialized to
protected jspExceptionClazz =
Class.forName("javax.servlet.jsp.JspException");
Then findRootCause could be this (missing the exception checking) - in
this version theRootCause is unneeded but can be kept for binary compat.
Sorry for the line wraps
private static final Throwable findRootCause(Throwable theException,
Throwable theRootCause) {
while (theException!=null) {
Throwable deeperRootCause = null;
if (theException == theException) {
return theException;
} else if (theException instanceof ServletException) {
deeperRootCause = ((ServletException) theException).getRootCause();
} else if (jspExceptionClazz!=null &&
jspExceptionClazz.isAssignableFrom(theException)) {
deeperRootCause =
(Throwable)IntrospectionUtils.getProperty(rootCause, "rootCause");
}
if (deeperRootCause==null) {
return theException;
}
if (theException == deeperRootCause) {
return theException;
}
theException = deeperRootCause;
}
// In case theException was null in the first place
return theException;
}
-Tim
Mark Thomas wrote:
> Tim Funk wrote:
>> Does this introduces a new dependency on the jsp-api - which could be a
>> regression for people who embed tomcat without using jsp's.
>
> Yes, it does add an additional dependency. I didn't consider the
> embedded use case.
>
>> Also the checking is not that aggressive any more in the case of nested
>> exceptions. This may leave the root cause still unexposed.
>
> My bad. I missed the recursion part of the patch.
>
>> This patch seems better:
>> http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/ErrorReportValve.java?r1=466608&r2=496117&diff_format=h
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r498053 - in /tomcat/container/tc5.5.x/catalina:
build.xml src/share/org/apache/catalina/core/StandardWrapper.java
Posted by Mark Thomas <ma...@apache.org>.
Tim Funk wrote:
> Does this introduces a new dependency on the jsp-api - which could be a
> regression for people who embed tomcat without using jsp's.
Yes, it does add an additional dependency. I didn't consider the
embedded use case.
> Also the checking is not that aggressive any more in the case of nested
> exceptions. This may leave the root cause still unexposed.
My bad. I missed the recursion part of the patch.
> This patch seems better:
> http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/ErrorReportValve.java?r1=466608&r2=496117&diff_format=h
This issue that the OP raised with this patch is the unintended
consequences of using introspection. To summarise:
- the root cause could be a custom exception
- this custom exception may have a getRootCause() method
- since this method is not defined anywhere, it could do anything
- therefore, calling getRootCause() on a random exception is dangerous
The ErrorReportValve also only uses getRootCause() and ignores
possibilities offered by getCause()
I'll take another look at this with the following intentions:
- removing the jsp-api dependency (probably a slightly ugly hack)
- add the recursion part of the patch
- continue to avoid using introspection to call getRootCause()
Thoughts?
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r498053 - in /tomcat/container/tc5.5.x/catalina:
build.xml src/share/org/apache/catalina/core/StandardWrapper.java
Posted by Tim Funk <fu...@joedog.org>.
Does this introduces a new dependency on the jsp-api - which could be a
regression for people who embed tomcat without using jsp's.
Also the checking is not that aggressive any more in the case of nested
exceptions. This may leave the root cause still unexposed.
This patch seems better:
http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/ErrorReportValve.java?r1=466608&r2=496117&diff_format=h
-Tim
markt@apache.org wrote:
> Author: markt
> Date: Fri Jan 19 19:07:36 2007
> New Revision: 498053
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=498053
> Log:
> Put the fix back for 39088 now the build is fixed. Sorry for the noise.
>
> Modified:
> tomcat/container/tc5.5.x/catalina/build.xml
> tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/StandardWrapper.java
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r498053 - in /tomcat/container/tc5.5.x/catalina:
build.xml src/share/org/apache/catalina/core/StandardWrapper.java
Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
did you really wanna add in the JSP-API dependency?
Filip
markt@apache.org wrote:
> Author: markt
> Date: Fri Jan 19 19:07:36 2007
> New Revision: 498053
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=498053
> Log:
> Put the fix back for 39088 now the build is fixed. Sorry for the noise.
>
> Modified:
> tomcat/container/tc5.5.x/catalina/build.xml
> tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/StandardWrapper.java
>
> Modified: tomcat/container/tc5.5.x/catalina/build.xml
> URL: http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/build.xml?view=diff&rev=498053&r1=498052&r2=498053
> ==============================================================================
> --- tomcat/container/tc5.5.x/catalina/build.xml (original)
> +++ tomcat/container/tc5.5.x/catalina/build.xml Fri Jan 19 19:07:36 2007
> @@ -30,6 +30,7 @@
> <!-- Dependent JARs and files -->
> <property name="ant.jar" value="${ant.home}/lib/ant.jar"/>
> <property name="servlet-api.jar" value="${api.home}/jsr154/dist/lib/servlet-api.jar"/>
> + <property name="jsp-api.jar" value="${api.home}/jsr152/dist/lib/jsp-api.jar"/>
> <property name="tomcat-util.jar"
> value="${tomcat-util.home}/build/lib/tomcat-util.jar"/>
> <property name="tomcat-coyote.jar"
> @@ -72,6 +73,7 @@
> <pathelement location="${mail.jar}"/>
> <pathelement location="${regexp.jar}"/>
> <pathelement location="${servlet-api.jar}"/>
> + <pathelement location="${jsp-api.jar}"/>
> <pathelement location="${xercesImpl.jar}"/>
> <pathelement location="${xml-apis.jar}"/>
> <pathelement location="${classes.dir}"/>
> @@ -101,6 +103,7 @@
> <pathelement location="${mail.jar}"/>
> <pathelement location="${regexp.jar}"/>
> <pathelement location="${servlet-api.jar}"/>
> + <pathelement location="${jsp-api.jar}"/>
> <pathelement location="${xercesImpl.jar}"/>
> <pathelement location="${xml-apis.jar}"/>
> <pathelement location="${classes.dir}"/>
>
> Modified: tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/StandardWrapper.java
> URL: http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/StandardWrapper.java?view=diff&rev=498053&r1=498052&r2=498053
> ==============================================================================
> --- tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/StandardWrapper.java (original)
> +++ tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/StandardWrapper.java Fri Jan 19 19:07:36 2007
> @@ -28,6 +28,8 @@
> import java.security.AccessController;
> import java.security.PrivilegedActionException;
> import java.security.PrivilegedExceptionAction;
> +import java.sql.SQLException;
> +
> import javax.servlet.Servlet;
> import javax.servlet.ServletConfig;
> import javax.servlet.ServletContext;
> @@ -36,6 +38,7 @@
> import javax.servlet.ServletResponse;
> import javax.servlet.SingleThreadModel;
> import javax.servlet.UnavailableException;
> +import javax.servlet.jsp.JspException;
> import javax.management.ListenerNotFoundException;
> import javax.management.MBeanNotificationInfo;
> import javax.management.Notification;
> @@ -56,7 +59,6 @@
> import org.apache.catalina.security.SecurityUtil;
> import org.apache.catalina.util.Enumerator;
> import org.apache.catalina.util.InstanceSupport;
> -import org.apache.tomcat.util.IntrospectionUtils;
> import org.apache.tomcat.util.log.SystemLogHandler;
> import org.apache.commons.modeler.Registry;
>
> @@ -632,23 +634,33 @@
> * @param e The servlet exception
> */
> public static Throwable getRootCause(ServletException e) {
> - Throwable rootCause = e;
> - Throwable rootCauseCheck = null;
> - // Extra aggressive rootCause finding
> - do {
> - try {
> - rootCauseCheck = (Throwable)IntrospectionUtils.getProperty
> - (rootCause, "rootCause");
> - if (rootCause == rootCauseCheck)
> - rootCauseCheck = null;
> - else if (rootCauseCheck != null)
> - rootCause = rootCauseCheck;
> + Throwable rootCause = e.getRootCause();
> + return findRootCause(e, rootCause);
> + }
>
> - } catch (ClassCastException ex) {
> - rootCauseCheck = null;
> - }
> - } while (rootCauseCheck != null);
> - return rootCause;
> + /*
> + * Work through the root causes using specific methods for well known types
> + * and getCause() for the rest. Stop when the next rootCause is null or
> + * an exception is found that has itself as its own rootCause.
> + */
> + private static final Throwable findRootCause(Throwable theException,
> + Throwable theRootCause) {
> +
> + Throwable deeperRootCause = null;
> +
> + if (theRootCause == null || theRootCause == theException) {
> + deeperRootCause = theException;
> + } else if (theRootCause instanceof ServletException) {
> + deeperRootCause = ((ServletException) theRootCause).getRootCause();
> + } else if (theRootCause instanceof JspException) {
> + deeperRootCause = ((JspException) theRootCause).getRootCause();
> + } else if (theRootCause instanceof SQLException) {
> + deeperRootCause = ((SQLException) theRootCause).getNextException();
> + } else {
> + deeperRootCause = theRootCause.getCause();
> + }
> +
> + return deeperRootCause;
> }
>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org