You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Mike Kaufman <mi...@mkaufman.fsnet.co.uk> on 2007/01/24 21:06:32 UTC

Possible javax.servlet.http.HttpServlet bugs, and where to report them?

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 "servletapi-feedback@eng.sun.com", 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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Possible javax.servlet.http.HttpServlet bugs, and where to report them?

Posted by Jan Luehe <Ja...@Sun.COM>.
Hi Mike,

 > 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!

I found another problem with HttpServlet's NoBodyOutputStream: If you use
getWriter(), the returned writer is constructed as follows:

    OutputStreamWriter w = new OutputStreamWriter(
        noBody, getCharacterEncoding());
    writer = new PrintWriter(w);

Notice that when you wrap an OutputStreamWriter around noBody, the
output will be buffered by the OutputStreamWriter before it is written
to "noBody", meaning that when NoBodyOutputStream.setContentLength()
is called, noBody.getContentLength() will return 0 if the output is still
buffered. Therefore, NoBodyOutputStream.setContentLength() needs to
flush the writer (if one is being used) before calling 
noBody.getContentLength(),
as follows:

    void setContentLength() {
        if (!didSetContentLength) {
            // BEGIN PATCH
            if (writer != null) {
                writer.flush();
            }
            // END PATCH
            setContentLength(noBody.getContentLength());
        }
    }

Let me know what you think.

Thanks,


Jan




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Possible javax.servlet.http.HttpServlet bugs, and where to report them?

Posted by Mark Thomas <ma...@apache.org>.
Jan Luehe wrote:
> please go to https://servlet-spec-eg.dev.java.net/
> and request observer role.

I tried by I can't access any of the project. My id is medthomas.
Could you forward the request for me?

Many thanks,

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Possible javax.servlet.http.HttpServlet bugs, and where to report them?

Posted by Jan Luehe <Ja...@Sun.COM>.
Mark,

Mark Thomas wrote On 01/24/07 07:59 PM,:

>Jan Luehe wrote:
>  
>
>>Hi Mike,
>>
>>thanks for your comments, which I have recorded at:
>>
>> https://servlet-spec-eg.dev.java.net/issues/show_bug.cgi?id=40
>>    
>>
>
>Jan - is it possible to get read access to this?
>  
>

please go to https://servlet-spec-eg.dev.java.net/
and request observer role.

Jan

>Cheers,
>
>Mark
>
>---------------------------------------------------------------------
>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: Possible javax.servlet.http.HttpServlet bugs, and where to report them?

Posted by Mark Thomas <ma...@apache.org>.
Jan Luehe wrote:
> Hi Mike,
> 
> thanks for your comments, which I have recorded at:
> 
>  https://servlet-spec-eg.dev.java.net/issues/show_bug.cgi?id=40

Jan - is it possible to get read access to this?

Cheers,

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Possible javax.servlet.http.HttpServlet bugs, and where to report them?

Posted by Jan Luehe <Ja...@Sun.COM>.
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 "servletapi-feedback@eng.sun.com", 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: 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