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/06/11 11:24:53 UTC
svn commit: r1348762 - in /tomcat/trunk:
java/org/apache/catalina/valves/ErrorReportValve.java
test/org/apache/catalina/valves/TestErrorReportValve.java
Author: markt
Date: Mon Jun 11 09:24:53 2012
New Revision: 1348762
URL: http://svn.apache.org/viewvc?rev=1348762&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53071
Stepping through the code, light dawns as to what the bug report was getting at.
Use the message from the Throwable for the error report if none was specified via sendError()
Modified:
tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
tomcat/trunk/test/org/apache/catalina/valves/TestErrorReportValve.java
Modified: tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java?rev=1348762&r1=1348761&r2=1348762&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java (original)
+++ tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java Mon Jun 11 09:24:53 2012
@@ -141,7 +141,11 @@ public class ErrorReportValve extends Va
String message = RequestUtil.filter(response.getMessage());
if (message == null) {
- message = "";
+ if (throwable != null) {
+ message = RequestUtil.filter(throwable.getMessage());
+ } else {
+ message = "";
+ }
}
// Do nothing if there is no report for the specified status code
Modified: tomcat/trunk/test/org/apache/catalina/valves/TestErrorReportValve.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/valves/TestErrorReportValve.java?rev=1348762&r1=1348761&r2=1348762&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/valves/TestErrorReportValve.java (original)
+++ tomcat/trunk/test/org/apache/catalina/valves/TestErrorReportValve.java Mon Jun 11 09:24:53 2012
@@ -49,7 +49,8 @@ public class TestErrorReportValve extend
ByteChunk res = getUrl("http://localhost:" + getPort());
- Assert.assertTrue(res.toString().contains(ErrorServlet.ERROR_TEXT));
+ Assert.assertTrue(res.toString().contains("<p><b>message</b> <u>" +
+ ErrorServlet.ERROR_TEXT + "</u></p>"));
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1348762 - in /tomcat/trunk: java/org/apache/catalina/valves/ErrorReportValve.java
test/org/apache/catalina/valves/TestErrorReportValve.java
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
On 6/11/12 5:24 AM, markt@apache.org wrote:
> Author: markt
> Date: Mon Jun 11 09:24:53 2012
> New Revision: 1348762
>
> URL: http://svn.apache.org/viewvc?rev=1348762&view=rev
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53071
> Stepping through the code, light dawns as to what the bug report was getting at.
> Use the message from the Throwable for the error report if none was specified via sendError()
This might end up being a security problem, depending on what
information is in the exception message. Can we make this a non-default
option? Many sites (ours included) attempt to avoid any part of a stack
trace (even the message) leaking-out to users.
-chris
Re: svn commit: r1348762 - in /tomcat/trunk: java/org/apache/catalina/valves/ErrorReportValve.java
test/org/apache/catalina/valves/TestErrorReportValve.java
Posted by Mark Thomas <ma...@apache.org>.
On 11/06/2012 10:33, Konstantin Kolinko wrote:
> 2012/6/11 <ma...@apache.org>:
>> Author: markt
>> Date: Mon Jun 11 09:24:53 2012
>> New Revision: 1348762
>>
>> URL: http://svn.apache.org/viewvc?rev=1348762&view=rev
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53071
>> Stepping through the code, light dawns as to what the bug report was getting at.
>> Use the message from the Throwable for the error report if none was specified via sendError()
>>
>> Modified:
>> tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
>> tomcat/trunk/test/org/apache/catalina/valves/TestErrorReportValve.java
>>
>> Modified: tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java?rev=1348762&r1=1348761&r2=1348762&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java (original)
>> +++ tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java Mon Jun 11 09:24:53 2012
>> @@ -141,7 +141,11 @@ public class ErrorReportValve extends Va
>>
>> String message = RequestUtil.filter(response.getMessage());
>> if (message == null) {
>> - message = "";
>> + if (throwable != null) {
>> + message = RequestUtil.filter(throwable.getMessage());
>> + } else {
>> + message = "";
>> + }
>
> throwable.getMessage() can return null, e.g. it usually happens with
> a NullPointerException
ACK. I'll fix that.
Mark
>
> So it'd be better to do
> if (throwable != null) {
> message = RequestUtil.filter(throwable.getMessage());
> }
> if (message == null) {
> message = "";
> }
>
> Alternatively, maybe String.valueOf(throwable.getMessage()) to convert
> null to "null". I cannot say what is better without a bit of
> experimenting, but using "" as the value is what we had before.
>
>> }
>>
>> // Do nothing if there is no report for the specified status code
>>
>
> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> 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
Re: svn commit: r1348762 - in /tomcat/trunk: java/org/apache/catalina/valves/ErrorReportValve.java
test/org/apache/catalina/valves/TestErrorReportValve.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/6/11 <ma...@apache.org>:
> Author: markt
> Date: Mon Jun 11 09:24:53 2012
> New Revision: 1348762
>
> URL: http://svn.apache.org/viewvc?rev=1348762&view=rev
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53071
> Stepping through the code, light dawns as to what the bug report was getting at.
> Use the message from the Throwable for the error report if none was specified via sendError()
>
> Modified:
> tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
> tomcat/trunk/test/org/apache/catalina/valves/TestErrorReportValve.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java?rev=1348762&r1=1348761&r2=1348762&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java Mon Jun 11 09:24:53 2012
> @@ -141,7 +141,11 @@ public class ErrorReportValve extends Va
>
> String message = RequestUtil.filter(response.getMessage());
> if (message == null) {
> - message = "";
> + if (throwable != null) {
> + message = RequestUtil.filter(throwable.getMessage());
> + } else {
> + message = "";
> + }
throwable.getMessage() can return null, e.g. it usually happens with
a NullPointerException
So it'd be better to do
if (throwable != null) {
message = RequestUtil.filter(throwable.getMessage());
}
if (message == null) {
message = "";
}
Alternatively, maybe String.valueOf(throwable.getMessage()) to convert
null to "null". I cannot say what is better without a bit of
experimenting, but using "" as the value is what we had before.
> }
>
> // Do nothing if there is no report for the specified status code
>
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org