[possible bug] -- Re: costin: fix reverted

[peter lin <pe...@labs.gte.com>]

On a related note, i just discovered some behavior that may be a bug. 
In my case, I have a request filter that buffers the output, so that I
can gzip it and time the internal time.

when a page hits an exception, it correctly forwards to my error page. 
but what ends up happening is the buffer isn't flushed, therefore the
output has unwanted data.  According to the spec, reset is supposed to
be called on ServletOutputStream, but when I print out debug messages, I
never see it call reset() or resetBuffer() in my output wrapper. It's
unclear to me if this part of the spec applies to requests that are
forwarded.

the relevant section from the spec is below.

5.1
----------
The reset method clears data in the buffer when the response is not
committed. Headers and status codes set by the servlet prior to the
reset call must
be cleared as well. The resetBuffer method clears content in the buffer
if the
response is not committed without clearing the headers and status code.


does someone know what the correct behavior should be?


peter lin


Costin Manolache wrote:
> 
> Remy Maucherat wrote:
> 
> >
> >> Can you point me to the code doing this extra processing ?
> >>
> >> I'm a bit confused:
> >> - for jsps ( with flush-in-release ) that just can't work, since the
> >> flush is already done by the jsp page.
> >> - the comment ( or the message ) in forward is probably wrong.
> >> - my bigger question is when is the response not a facade ? My
> >> understanding
> >> is that facades are used all the time, so the second part of the if will
> >> never happen
> >
> > That code was already there before the facades. I don't think it is used
> > anymore, but would leave it anyway in 4.1.x (it could be removed in 5).
> 
> I already reverted the patch in 4.1.x - it's too dangerous for the
> stable tree.
> 
> So you are saying that forward() used to do a flush, but it was
> changed when facades where added to just set the flag - to improve
> the error handling. Sounds consistent with the behavior we have now.
> 
> Could you add a little comment ( or at least remove the debug message
> that says 'flushing' ) ? The new behavior ( no real flush ) seems
> correct.
> 
> Costin
> 
> >> I do see your point - if forward() flushes then the extra error
> >> processing is broken. That's another argument to not do the flush() in
> >> release() :-) And it does explain who is generating the stack trace.
> >
> > The error page and status report processing is in
> > ErrorReport/DispatcherValve. That's where the stack trace is output
> > unless the response has been committed (by flushing usually).
> 
> >
> >>
> >> I have a feeling we just need to clear the error flags after the jsp
> >> error page - I'm pretty sure the jsp error page was executed and the data
> >> is in the out buffer. Somehow the servlet error processing kicks in and
> >> overrides the jsp error page.
> >
> > Yes, that's likely the right explanation.
> >
> > Remy
> 
> --
> To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>