When setting > request.setAttribute(Globals.EXCEPTION_ATTR, e); > response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); in a server-side component the throwable#getMessage is completely ignored and only the stacktrace is shown. Message is not retrieved. Line 161 has to be simply replaced with > if (throwable != null) > message = RequestUtil.filter(throwable.getMessage()); > else > message = RequestUtil.filter(response.getMessage()); The throwable message is nicely displayed.
This change could also be applied to Tomcat 7.
Fixed in trunk and 7.0.x and will be included in 7.0.28 onwards. Proposed for 6.0.x Notes: 1. The message from the Throwable was displayed in the stack trace. 2. I modified the patch. The message from the Throwable is only used if no message is specified via sendError()
(In reply to comment #2) > Fixed in trunk and 7.0.x and will be included in 7.0.28 onwards. > > Proposed for 6.0.x > > Notes: > 1. The message from the Throwable was displayed in the stack trace. I am aware of that but I don't want to burden my users with the stack traces, the message is just fine. > 2. I modified the patch. The message from the Throwable is only used if no > message > is specified via sendError() This is perfectly fine and does the job. Thanks for the fix. Looking forward to 6.0.36.
Putting this caveat in the bug and not just on the dev list: 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.
Then you have already replaced / modified the error report valve and you won't see this change.
Created attachment 28920 [details] Screen shot from Tomcat 7.0.27 Just a screen shot, for comparison. It is before implementing this feature. It is CookieExample page, when pressing submit button without entering any values.
Created attachment 28921 [details] Screen shot from Tomcat trunk The same with Tomcat trunk after implementing the feature.
(In reply to comment #4) > Putting this caveat in the bug and not just on the dev list: I agree with Mark here. The change in r1348762 is in the place where ErrorReportValve prepares HTML text of the page. It does not affect other components. It does not affect components that display their own error pages. The old implementation already displays the exception, so nothing new is revealed. I personally do not like use of "{0}" in the messages for the "description" field. Especially the ones for 404 and 403. It looks like some unrelated text is inserted into the middle of a sentence. With this change it is printed 4 times on the same page.
(In reply to comment #8) > (In reply to comment #4) > > Putting this caveat in the bug and not just on the dev list: > > I agree with Mark here. > The change in r1348762 is in the place where ErrorReportValve prepares HTML > text of the page. It does not affect other components. It does not affect > components that display their own error pages. The old implementation > already displays the exception, so nothing new is revealed. Exactly, the same information but more prominent. > I personally do not like use of "{0}" in the messages for the "description" > field. Especially the ones for 404 and 403. It looks like some unrelated > text is inserted into the middle of a sentence. With this change it is > printed 4 times on the same page. I personally dislike that one too in this valve. In my opinion, the following should happen: If message is not given: Message should be from https://tools.ietf.org/html/rfc2616#section-6.1.1 as per status code short name and description should be a copy of the first sentence of the RFC description. Maybe just as HTTPd does. If message is given: Display message and show RFC description too. Stack trace should be left as is. It might make sense to limit the message length in the first line to n chars and show the entire message in the message line. But if and only if the message is to0 long. Otherwise the message line should be omitted.
Fixed in 6.0.x and will be included in 6.0.36 onwards.
(In reply to comment #8) > I personally do not like use of "{0}" in the messages for the "description" > field. Especially the ones for 404 and 403. It looks like some unrelated > text is inserted into the middle of a sentence. With this change it is > printed 4 times on the same page. I removed the {0} from the "report" in r1361991 (trunk) and r1362000 (tc7). Motivation: As far as I can see, those properties are only used in ErrorReportValve. There they are only put into the "report", which is only output in an HTML document directly below the "message" that would have been put into {0}. In addition the same "message" is again being output as an h1 heading shortly before. So I see no reason at all to keep the "message" inside the "report".
(In reply to comment #11) > (In reply to comment #8) > > I personally do not like use of "{0}" in the messages for the "description" > > field. Especially the ones for 404 and 403. It looks like some unrelated > > text is inserted into the middle of a sentence. With this change it is > > printed 4 times on the same page. > > I removed the {0} from the "report" in r1361991 (trunk) and r1362000 (tc7). Rainer, can you backport to Tomcat 6 too?
Reopening this. This needs further improvement. When there is a java compilation error in JSP page, the error page becomes especially ugly. Reproducible in 7.0.29. To reproduce: 1. Change ROOT/index.jsp introducing a typo in Java code scriptlet, e.g. s/SimpleDateFormat/SimpleDate Format/ 2. Access the page. Actual result: The whole text of the compilation exception, including a source code fragment, is included in the <h1> header, as well as in the "message" field. That is a lot of text without any formatting, occupying the whole screen. It is ugly. Expected result: Maybe include just some part of the text, and replace the rest with ellipsis " (...)"? Maybe use just the first line, as line breaks are lost when rendering HTML? Also, the changes mentioned in Comment 11 still have to be proposed/fixed in 6.0.
Jasper issue fixed in trunk and 7.0.x. Jasper fix and {0} fix proposed for 6.0.c