Bug 54536 - ErrorReportValve doesn't respect HttpServletResponse.sendError(int sc, String msg)
Summary: ErrorReportValve doesn't respect HttpServletResponse.sendError(int sc, String...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 7.0.30
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-07 12:33 UTC by Yannis Thanasoulas
Modified: 2013-02-12 20:33 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yannis Thanasoulas 2013-02-07 12:33:41 UTC
The changes of the ErrorSupportValve introduced in catalina-v7.0.30 does not respect HttpServletResponse.sendError(int sc, String msg)  because when a msg is provided with a status which is not present in the LocalStrings.properties of the 'org.apache.catalina.valves' package, the error page won't be forwarded to the response body.

This issue was triggered due to the following change which was performed at revision 1348777:

The change of the             
   report = sm.getString("http." + statusCode, message);
to
   report = sm.getString("http." + statusCode);

results in 'report==null' for custom status codes returning a response without the error page body.

        if (report == null) {
            return;
        }
Comment 1 Yannis Thanasoulas 2013-02-07 12:38:55 UTC
The correct revision is 1362000
Comment 2 Mark Thomas 2013-02-07 13:19:22 UTC
What status code are you sending and why?
Comment 3 Yannis Thanasoulas 2013-02-07 15:09:52 UTC
I am using custom code '901' in order to detect and handle session timeouts in AJAX requests.
Comment 4 Michael Osipov 2013-02-10 20:32:02 UTC
Why not 408?
Comment 5 Mark Thomas 2013-02-12 11:31:33 UTC
My reading of RFC2616 is that custom status codes are not permitted.

If you set a custom status code that will still be sent to the client - just with a default error page.

I'm currently on the fence whether to fix this or close it as invalid.

Why do you need the custom error page to be sent to the client? Why isn't the 901 status code and the default error page enough?
Comment 6 Konstantin Kolinko 2013-02-12 13:42:29 UTC
(In reply to comment #0)
>
> The correct revision is r1362000
>
> The change of the             
>    report = sm.getString("http." + statusCode, message);
> to
>    report = sm.getString("http." + statusCode);
> 
> results in 'report==null' for custom status codes

Regarding the change in r1362000

The 2nd argument to sm.getString(..) is not a default value, but an argument used for "{0}" in a MessageFormat.

BUT, there is small difference in the values returned by
(a) sm.getString(key) and
(b) sm.getString(key, args)
when the resource is not found.

(a) returns null, (b) returns MessageFormat.format(key, args) which is "key"

This caused a change in behaviour that this code block is now called:

> 
>         if (report == null) {
>             return;
>         }

Thus instead of rendering the "default" error page, a blank page is rendered.


BTW, I suspect that you can fix your issue by defining a custom error page in your web.xml, instead of relying on the default one.
Comment 7 Yannis Thanasoulas 2013-02-12 16:37:17 UTC
Status code 408 - Request Timeout, as it is described in RFC 2616 occurs when "The client did not produce a request within the time that the server was prepared to wait. The client MAY repeat the request without modifications at any later time.". My understanding of status code 408 is that  when a client(browser) sends a request and while the request is being processed by the server the connection timeouts, which is not the same case. 

The actual problem is that i expect the response body (default error page) to contain a message field with the url in which the user should be redirected (ex login page), because otherwise without this functionality the response body of the request, the session of which has been timed out, returns the login page with status 200, this cannot be handled effectively in the context of an AJAX request (at least with my current implementation). So, i return an error which can be caught in JS level and using the message (<b>message</b> <u>redirectUrl:/test/login?login_error=3</u>) redirects them to the desired page.

As for RFC2616, in section 6.1.1,  i found the following part which mentions that:

" HTTP status codes are extensible. HTTP applications are not required
   to understand the meaning of all registered status codes, though such
   understanding is obviously desirable."
   
but it isn't my intention to argue over this, i just found this inconsistency between the releases and i mention it.
Comment 8 Mark Thomas 2013-02-12 19:52:11 UTC
Thanks for the explanation.

I read a different part of RFC2616 that indicated to me that custom status codes were outside the spec. I see the part you referenced and agree with your interpretation.

Given the above I think Tomcat should not return a blank page if you use a custom status code. I'll look at getting this patched for the next release.
Comment 9 Mark Thomas 2013-02-12 20:08:52 UTC
Fixed in trunk and 7.0.x and will be included in 7.0.37 onwards.
Comment 10 Michael Osipov 2013-02-12 20:33:36 UTC
Please port such an improvement back to Tomcat 6.