tomcat-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mel Martinez <melaqu...@yahoo.com>
Subject Re: TC3.2.1 - response commit on included JSPs
Date Thu, 22 Feb 2001 19:45:52 GMT
Re: out.flush() sets the 'committed' state in the
response in tc3.2.1 JspWriterImpl.

--- cmanolache@yahoo.com wrote:
> Hi Mel,
> 
> First, JspWriter needs to be flushed at the end of
> the page - it has a
> buffer, and if the buffer is not commited the data
> will be lost.
> There is a method ( flushBuffer ) in JspWriterImpl,
> and that method should
> be called instead of flush().
> 

Hi Costin,

Two points come to mind here.  First off, I totally
agree that at the end of the page the buffer should be
'committed' to the underlying ServletOutputStream -
but I also believe that the ServletResponse should not
be committed from within a dynamically included
servlet or JSP page - is that distinction not clear?

Second, while there is a flushBuffer() method in
JspWriterImpl, this method is NOT a part of the
javax.servlet.jsp.JspWriter interface nor is it public
(it is protected).

An examination of JspWriterImpl.flush() shows that it
does it's own flushBuffer() before calling it's own
'out.flush()' (to cascade the flush to the underlying
ServletOutputStream) and then response.flushBuffer():

    /**
     * Flush the stream.
     *
     */
    public void flush()  throws IOException {
        synchronized (lock) {
            flushBuffer();
            if (out != null) {
                out.flush();
                // Also flush the response buffer.
                response.flushBuffer();
            }
        }
    }

(the above code is identical in both the tc3.2.1
version (JspWriterImpl rev1.4) and the latest in the
cvs web mainline branch (rev1.5).

Looking at JspWriterImpl.flushBuffer() it does indeed
look like it does all that you'd want it to do from
within an included request, as it simply writes the
current buffer to the underlying ServletOutputStream.

Where trouble is probably being caused is that when
flush is being called it also calls
response.flushBuffer() both directly as well as
indirectly (through cascaded out.flush() calls).  If
we can prevent that from happening in the 'included'
case, perhaps that would be enough to fix the problem?

So, it seems from this, that a simple fix might be to
do the following :

    /**
     * Flush the stream.
     *
     */
    public void flush()  throws IOException {
        synchronized (lock) {
            flushBuffer();
            if (out != null && !isIncluded) {
                out.flush();
                // Also flush the response buffer.
                response.flushBuffer();
            }
        }
    }

where we would redefine or create a new constructor:

    public JspWriterImpl(
                     ServletResponse response, 
                     int sz, 
                     boolean autoFlush,
                     boolean isIncluded) {
        this(response,sz,autoFlush);
        this.isIncluded = isIncluded;
    }

where isIncluded would default to false, of course.
And finally, the PageContextImpl would have to be
fixed
to use the new constructor.  It would set the
isIncluded flag based on the presence of the request
attribute "javax.servlet.include.servlet_path".


> In 3.2 we had a lot of problems with the buffers -
> changing that may be a
> bit dangerous. For 3.3, the whole buffering has been
> re-designed and
> refactored, and most problems we knew about in the
> servlet container
> are fixed ( but so far this issue hasn't been fixed
> - to be honest I
> didn't knew about it, I've been focused more on the
> servlet side ).
> 

I believe the problem here is solely on the jasper
side.  This problem shows up whether running jasper
inside tomcat or inside weblogic server's servlet
engine.

> It shouldn't be difficult to fix it, and since it is
> a spec issue I think
> this is a must_fix bug.
> 
> The best way to make sure it'll be indeed fixed is
> to send a patch or at
> least a test case ( a small war with some
> servlets/jsps and a <gtest>
> fragment that we can include in our nighlty tests ).
> 

Are the above suggestions useful?  I can test them out
on my own because I have a framework for overriding
and extending most all jasper behavior through
subclassing, but I am not a tomcat cvs committer nor
am I well setup yet to put the changes directly into
jasper code.  I want to get more directly involved,
but I don't have the luxury of time just yet.  Just
researching this problem is costing a lot of money so
far.

As for test cases, the problem should show up if you
try to include a .jsp file and then subsequently in
the calling page or servlet do something that requires
an uncommitted response (such as a redirect).

I'm  going to try to tackle this and verify if my
ideas above will help or not.  I'll report back later
on it.

Mel

__________________________________________________
Do You Yahoo!?
Yahoo! Auctions - Buy the things you want at great prices! http://auctions.yahoo.com/

Mime
View raw message