Bug 42409 - Extra response headers not sent when using custom error page
Summary: Extra response headers not sent when using custom error page
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Catalina (show other bugs)
Version: unspecified
Hardware: All All
: P3 major (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-05-14 09:03 UTC by Hayden James
Modified: 2008-11-30 14:58 UTC (History)
2 users (show)



Attachments
WAR with custom error page; extra response headers not seen (2.88 KB, application/octet-stream)
2007-05-31 05:24 UTC, Shiva Kumar H R
Details
WAR with default error page; extra response headers seen (2.88 KB, application/octet-stream)
2007-05-31 05:25 UTC, Shiva Kumar H R
Details
call "response.resetBuffer()" instead of "response.reset(statusCode, message)" (1.70 KB, patch)
2007-05-31 07:53 UTC, Shiva Kumar H R
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hayden James 2007-05-14 09:03:17 UTC
If I create a servlet that does the following:

response.setHeader("X-BUG", "Value");
response.sendError(HttpServletResponse.SC_BAD_REQUEST);

If I use the tomcat default error page for 400 responses, the response will
contain the header "X-BUG" with value "Value".

However, if I create a custom error page, the response headers do NOT contain
the "X-BUG" header in the response. 

Here is my web.xml, for example:

<error-page>
    <error-code>400</error-code>
    <location>/400.jsp</location>
</error-page>

400.jsp contains plain html:

<html>
<body>
<h3>Custom 400 error page</h3>
</body>
</html>
Comment 1 Hayden James 2007-05-14 09:06:24 UTC
Not sure if it matters, but this occurred for me using the doGet() method in my
servlet.  Also this problem appears to exist in 5.5.23, 6.0.10 and the
unreleased 6.0.13 build.
Comment 2 Shiva Kumar H R 2007-05-17 08:03:15 UTC
Was wondering how I could view the HTTP Headers. Found this useful:
http://www.jmarshall.com/easy/http/http_footnotes.html#manually

I could reproduce the reported behavior with trunk build tc6.0.x. 

Now looking at Specs to see if this is really a Bug.
Comment 3 Hayden James 2007-05-17 08:51:00 UTC
I tried the same situation with jetty and it appears to work properly.  It seems
incorrect for the response to not send all the data (including headers) I
specify just because the error page was set.  Also, an easy way to test this
behavior is to use the web developer plugin if you browse with firefox. 
Information -> View Response Headers.
Comment 4 Hayden James 2007-05-30 17:49:37 UTC
Same problem exists in 6.0.13.  How can I motivate someone to fix this bug or at
least point me in the right direction so I can fix it myself?
Comment 5 Shiva Kumar H R 2007-05-31 05:15:16 UTC
I too tested this scenario on Jetty, as well as on WebSphere Application Server
(WAS) binaries and this problem doesn't occur with them. In both Jetty and WAS,
I am able to see the extra response header even when using custom error page.

My search of specification for anything against it has been in vain.

Following is the piece of code that I have looked into:
org.apache.catalina.core.StandardHostValve
protected void status(Request request, Response response);
protected boolean custom(Request request, Response response, ErrorPage errorPage);

Here are my observations:
1) When a custom error page is used, client request is "forwarded" to the custom
error page. 

Now, as per Servlet 2.4 specification (page 65, SRV.8.4):
"The forward method of the RequestDispatcher interface may be called by the
calling servlet only when no output has been committed to the client. If output
data exists in the response buffer that has not been committed, the content must
be cleared before the target servlet’s service method is called. If the response
has been committed, an IllegalStateException must be thrown."

I see that in current Tomcat code, in "protected boolean custom(Request request,
Response response,ErrorPage errorPage)" method we are calling
"response.reset(statusCode, message)" to clear the response content. This also
clears the response headers that users might have set in their code.

Question for Committers:
I am seeing that the specification asks us to reset just the response buffer
(before 'forwarding' requests) while we are resetting the entire response object
(which also resets response headers). 

Can we replace the call to "response.reset(statusCode, message)" with a call to
"response.resetBuffer()"? 

If yes, some of the "request.getAttribute(..)" calls in "protected boolean
custom(Request request, Response response, ErrorPage errorPage)" method and
hence some of the "request.setAttribute(..)" calls in "protected void
status(Request request, Response response)" method might become redundant and
could be removed as well.

2) When default error page is used, there is no request "forwarding" and hence
no response resets (and hence the response headers that users set in their code
are preserved). I think an error message is simply added to existing response's
body and response is pushed to client.

- Shiva
Comment 6 Shiva Kumar H R 2007-05-31 05:24:31 UTC
Created attachment 20297 [details]
WAR with custom error page; extra response headers not seen

page to run: http://localhost:8080/Bug_42409_error/test
Comment 7 Shiva Kumar H R 2007-05-31 05:25:59 UTC
Created attachment 20298 [details]
WAR with default error page; extra response headers seen

page to run: http://localhost:8080/Bug_42409_correct/test
Comment 8 Shiva Kumar H R 2007-05-31 07:53:26 UTC
Created attachment 20299 [details]
call "response.resetBuffer()" instead of "response.reset(statusCode, message)"

does anyone foresee any regressions out of this?
Comment 9 Remy Maucherat 2007-05-31 09:11:11 UTC
No, I don't think we're going to change this. We'll keep the current code which
sets the status code + message, but your customization for your own is fine too.
Comment 10 Hayden James 2007-05-31 09:59:26 UTC
If the code is kept as is, how would one use a custom error page and still be
able to set response headers?  I'm a little confused, is it not a bug?
Comment 11 Shiva Kumar H R 2007-05-31 22:10:29 UTC
James,
If you have any strong use-case for your requirement, this might be reconsidered.
Comment 12 Shiva Kumar H R 2007-05-31 22:26:28 UTC
Forgot to mention the answer to your question. Extra response headers set in the
Custom error page will be preserved and passed back to the client. 

Is there any strong reason why you are not doing this and why you want to set
the extra response headers in the Servlet/JSP from where error is thrown?
Comment 13 Hayden James 2007-06-02 07:58:35 UTC
Ok.  I will tell you what my use case is for this feature.  I'm writing a set of
servlets that handle requests via javascript and the XMLHttpRequest js object. 
In order to differentiate between good requests and bad requests, I either have
the response code set to 200 for success and 400 for failure.  I have separate
handlers for each status number on the javascript end.  As part of the response,
I send data in the header via the "X-JSON" header with a json string as the
value in the header.  The client reads the json parses the json string in both
success responses and error responses (SC_BAD_REQUEST).  The reason im using a
special error page for 400 responses is that the default tomcat page is a lot
more verbose, I want to respond with a very small json string, not a full html page.

The prototype javascript library, (http://www.prototypejs.org) is what
automatically evaluates json for me via the X-JSON response header (in case that
information helps).  Anyway, I think this is a pretty reasonable use case, but
if others disagree, I would at least like to know some other avenues I can take
to get this type of usage with tomcat.  Thanks.
Comment 14 Shiva Kumar H R 2007-06-04 07:36:11 UTC
Interesting use-case. One immediate solution that is coming to mind is setting
"X-JSON" header in your custom error page for 400 response. Doesn't this work
for you?
Comment 15 Hayden James 2007-06-04 08:29:05 UTC
How would that work?  If a javascript client makes a request to the
controller/servlet, the controller decides whether the request is "good" or not,
if not then the controller generates the response json and sets it in the header
and returns a page containing that header.  It seems it would be easier to set
the header on the page if the header value was always the same but it isn't.
Comment 16 rm 2007-06-05 02:58:46 UTC
When you need to pass information from the controller to a page, you normally use
request attributes, not response attributes.
You can can do the same in your case.

I hope that helps.
R
Comment 17 Shiva Kumar H R 2007-06-05 07:20:20 UTC
As hinted in Comment #16, I tried this:

1) In "TestServlet.java" (see
http://issues.apache.org/bugzilla/attachment.cgi?id=20297) modified the doGet()
method as below:
		//response.setHeader("X-BUG", "Value1");
		request.setAttribute("X-BUG", "Value1");
		response.sendError(HttpServletResponse.SC_BAD_REQUEST);

2) Updated "400.jsp" as below:
<html>
<body>
<h3>Custom 400 error page</h3>
<% String value = (String) request.getAttribute("X-BUG");
if(value != null && value.length() > 0) {
	response.setHeader("X-BUG", value);
} %>
</body>
</html>

Does this solve your problem?
Comment 18 Hayden James 2007-06-07 08:27:40 UTC
I guess this workaround should work, unfortunately I'll have to rip through all
my source to replace setHeader with setAttribute and then add a scriptlet to my
error jsp page.  However, I still don't understand why this won't be fixed,
seems like a legitimate bug.  Anyhow, thanks for the help.
Comment 19 Hayden James 2007-06-16 19:00:42 UTC
One problem I find with the above code is that if I decide not to use a custom
400 page then my extra header will not be available in the response.  Is there
another solution where it would work regardless of page I use (besides fixing
the bug I guess)?
Comment 20 Konstantin Kolinko 2007-07-03 15:21:56 UTC
Hi, I would like to add my use case on to the scales.

We are using Acegi Security Library for Spring (http://acegisecurity.org/) to
perform authentication and authorization tasks in our web application. In
essence, it works as a filter, declared in web.xml, and preprocesses the web
request. We are using Digest authentication as per RFC 2617, but you might
consider using Basic authentication as well.

When there is a need to request user credentials, the library ([1]) generates
WWW-Authenticate header containing realm name, random nonce value, and other
information, and calls
httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED), and the rest of the
response is generated by the tomcat error page.

Now, if I configure my own dynamic or static page for error code 401, the
authentication stops working, because the WWW-Authenticate header is lost from
the response.


Versions:
 - Tomcat: 5.5.23
 - Acegi Security System for Spring: 1.0.4

The relevant Acegi Security source code is method "commence()" of class
org.acegisecurity.ui.digestauth.DigestProcessingFilterEntryPoint, lines 104-105
and above ([1])


 [1]
http://svn.sourceforge.net/viewvc/acegisecurity/tags/release_1_0_4/core/src/main/java/org/acegisecurity/ui/digestauth/DigestProcessingFilterEntryPoint.java?revision=1881&view=markup
Comment 21 Brian Curnow 2008-10-22 06:22:47 UTC
I'll add that other contains (Jetty, BEA/Oracle WebLogic Server, etc) appear to be handling this case correctly.

We are having a similar issue to Comment #20 with Spring Security (new name for ACEGI).

This really needs to be fixed. The resolution suggested in Comment #5 sounds like it would fix this issue and keep Tomcat spec compliant. At this point Tomcat is doing *more* than the spec requires.
Comment 22 Filip Hanik 2008-10-22 22:59:18 UTC
we are investigating this further, thank you for the detailed report

Filip
Comment 23 Mark Thomas 2008-11-03 06:28:34 UTC
After some further review, this has been fixed in trunk and proposed for 6.0.x
Comment 24 Hayden James 2008-11-03 18:47:27 UTC
Great, I'm glad to see this fix, I had given up hope last year and started using jetty since that didn't have this issue.
Comment 25 Mark Thomas 2008-11-30 14:58:37 UTC
This has been fixed in 6.0.x and will be included in 6.0.19 onwards.