Hi Mike,
thanks for your comments, which I have recorded at:
https://servlet-spec-eg.dev.java.net/issues/show_bug.cgi?id=40
and
https://glassfish.dev.java.net/issues/show_bug.cgi?id=2212
I agree with all the problems you've pointed out, and I agree they need
to be fixed.
I think we've inherited the "NoBodyResponse" inner class from one of
the very first servlet releases. I agree this inner class is poorly
designed, in addition to having the bugs you've pointed out.
I very much support the idea of it implementing
javax.servlet.http.HttpServletResponseWrapper. This would be a
backwards-compatible change (since "NoBodyResponse" would continue to
implement javax.servlet.http.HttpServletResponse), would reduce the
code, and
would also take care of the missing IllegalStateException in connection
with calling
getWriter() after getOutputStream() (and vice versa).
I'll produce code diffs for javax.servlet.http.HttpServlet and will run them
by the EG and you.
Thanks!
Jan
Mike Kaufman wrote On 01/24/07 12:06 PM,:
I think there's a bug in javax.servlet.http.HttpServlet, but I'm not
sure where to report it. I'm posting this here for the time being (and
possibly on the Glassfish "issue tracker" if and when I can jump
through the hoops required to do so), but please let me know if it
ought to be reported some other way: and apologies if this doesn't
belong here!
The actual bug is that the "NoBodyResponse" inner class within
HttpServlet doesn't track whether getWriter or getOutput stream have
been called, and doesn't alter the response's behaviour accordingly.
This differs from the ServletResponse Javadoc which states that:
- Calls to getWriter are supposed to throw IllegalStateException if
getOutputStream has already been called and vice-versa.
- The behaviour of various other methods depend on whether or not
getWriter has already been called (e.g. setCharacterEncoding is
supposed to have no effect if getWriter has already been called).
As a result, HTTP "HEAD" requests may complete successfully where an
identical "GET" would fail with an IllegalStateException, or "HEAD"
and "GET" may result in different headers (and different results from
methods such as getCharacterEncoding whilst processing the request).
A more general issue is that the NoBodyResponse class could, and
perhaps should (to conform to SRV 8.2 of the Servlet specification),
be a ServletResponseWrapper subclass. At the very least this would
shorten and simplify its code. The Apache Tomcat bug database includes
a couple of entries for this (Tomcat 4 bug 10555 and Tomcat 5 bug
22290), and although these have long since been "closed" on the basis
that this code is the responsibility of the Servlet API itself rather
than Tomcat, and therefore these bugs were handed over to the Servlet
API feedback e-mail address "[EMAIL PROTECTED]", I can't
find any sign of this being followed up anywhere or ever being finally
resolved.
This has left me rather unclear as to where to report HttpServlet
bugs. The code itself doesn't seem to be part of the Servlet
specification, but is supplied as part of the standard "servlet.jar",
"javaee.jar" or equivalent; the source code is present and identical
in multiple versions of Tomcat, and Glassfish, and probably elsewhere;
Tomcat bug 22290 indicates that the Servlet API people are responsible
for maintaining the code but they only seem to have an e-mail address,
which itself may be obsolete (I've had no reply to an e-mail sent to
it, though with e-mails it's always hard to be sure); Tomcat was the
reference implementation for earlier API versions but now it's
Glassfish (or is it Sun Java System Application Server?); the code has
copyright notices for both Sun and Apache; and the Sun bug database
has some bug reports for HttpServlet but not since Dec 2002, and its
bug report form doesn't really seem to cater for this.
Sorry, but I'm confused!!! Can anyone give me a definitive answer as
to where bugs in javax.servlet.http.HttpServlet should be reported?
Whilst we're here, I think there are also some other minor/cosmetic
issues in the HttpServlet code:
- The "write" method of the NoBodyOutputStream inner class has a
comment saying that a negative length should "maybe" be an
IllegalArgumentException, but the code actually throws an IOException.
If this really should be an IllegalArgumentException, it ought to be
fixed. If an IOException is OK, or if fixing this now isn't acceptable
in case it breaks existing code, then maybe the comment should be
removed (or updated to explain this).
- The "write" method of the NoBodyOutputStream inner class reports
negative lengths by looking-up an "err.io.negativelength" message, but
it then ignores the result and instead uses a hard-coded string as the
actual error message.
- The doTrace method includes a "close" of the output stream when it
has finished. However, prior to this it sets the content length and
then writes that amount of content, which should always result in the
output stream being automatically flushed and closed. Hence the
response has already been closed when the call to close occurs. Whilst
it should usually be fairly safe to assume that repeating a close
won't do any harm, the output stream is implementation-specific, and I
can't see anything that says it must allow duplicate close attempts
(and why attempt the close at all if it isn't needed?).
- The doTrace method ends with a "return;" statement, which seems
rather pointless as the last statement of a method.
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
