TC3.2.1 - response commit on included JSPs
Hi folks,
I've spotted what looks like a bug in the jasper
compiler of TC3.2.1 - if this is old news, just let me
know and I'll shut up.
There are two parts to this:
1) the _jspService() method generated by the jasper
compiler uses a try{ ... }catch{...}finally{...}
block. The finally clause looks like so:
}finally{
out.flush();
_jspxFactory.releasePageContext(pageContext);
}
Q: shouldn't the their be a check to see if the
current request is the result of a dynamic include
before we flush the buffer?
2) Examination of
org.apache.jasper.runtime.JspWriterImpl and the
response classes shows that flush() has set the
response.isCommitted() flag of the HttpServletResponse
object before response.flushBuffer() has been called.
Q: JavaDoc for the ServletResponse.flushBuffer()
method implies that it should be the one to set the
'committed' state of the response object:
"Forces any content in the buffer to be written to the
client. A call to this method automatically commits
the response, meaning the status code and headers will
be written."
Without getting stuck on the details of either of the
above issues, the real issue is that as it stands
above, if you do a dynamic include of a JSP page then
the response object is committed upon the return of
the method. I don't believe that is correct behavior.
Is this fixed in a later release? If so, which one
should I upgrade to?
Thanks,
mel
Dr. Mel Martinez
G1440, Inc.
__
Do You Yahoo!?
Yahoo! Auctions - Buy the things you want at great prices! http://auctions.yahoo.com/
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, email: [EMAIL PROTECTED]
Re: TC3.2.1 - response commit on included JSPs
Mel Martinez wrote: > Without getting stuck on the details of either of the > above issues, the real issue is that as it stands > above, if you do a dynamic include of a JSP page then > the response object is committed upon the return of > the method. I don't believe that is correct behavior. > Arguably not correct, but it was a workaround to a worse problem in Tomcat 3.2-b6 that would otherwise have required a pretty substantial rearchitecting of 3.2. The old BugRat system (and the CVS commits) have details on the rationale, but we added a flush to the code in Tomcat 3.2 that handles includes. > > Is this fixed in a later release? If so, which one > should I upgrade to? > Tomcat 4.0 does not exhibit this problem, and does not force a flush. I do not know the status of 3.3 in this respect. > > Thanks, > > mel > Craig McClanahan - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, email: [EMAIL PROTECTED]
Re: TC3.2.1 - response commit on included JSPs
--- Incze Lajos <[EMAIL PROTECTED]> wrote:
> Maybe it's not correct in general but the JSP 1.1
> syntax
> contains the flush="true" attribute - obligatory in
> case
> of jsp:include. So, it's inconvenient but tomcat3 is
> an
> jsp 1.1 implementation. Am I missing something?
incze
The JSP 1.1 spec says that the current response buffer
should be flushed _prior_ to the inclusion. In other
words, the flush="true" requirement is talking about
what should happen at level of the calling page. In
normal java servlet code this is saying :
if(buffered && flush==true){ //JSP1.1 flush always
true
out.flush();
}
requestDispatcher.include(...);
//continue...
All this is saying is that, at least for JSP 1.1, we
are required to write everything we've put in the
buffer now before we add more to the buffer from the
included resource. This requirement is supposed to
happen on the CALLING page, not in the INCLUDED page.
I will note that TC3.2.1 does not actually do this
correctly and if it tried, it would break because of
the same root cause to the problem I am talking about
here.
The problem I am talking about is independent of the
_calling_ page.
The problem is caused by the nature of the JSP servlet
that is generated by TC3.2.1. It cannot properly be
included (dynamically) from either a JSP *OR* a
servlet because when it exits, it always sets the
isCommitted() flag of the response. It should not do
that if the page is processing a request as an
'include' action because only the parent, calling
servlet or jsp should Commit the response.
As explained, the reason it always sets the
isCommitted() state is due to the sequence:
1) Every JSP page generated by TC3.2.1 has a finally{}
block that invokes out.flush() and...
2) For some reason the TC 3.2.1 JspWriterImpl
implementation of out.flush() commits the response
even BEFORE it calls response.flushBuffer()!
3) Also, out.flush() ALWAYS calls
response.flushBuffer() even if processing an included
request.
IMHO out.flush() should not commit the response. Only
response.flushBuffer() should commit the response.
And response.flushBuffer() should not be called from
an inside an 'include' request.
What probably SHOULD happen is the following:
1) Response.flushBuffer() alone should be used to set
the commit state. But it should NEVER be called from
within an include action which can be checked with:
Obj incl = request.getAttribute(
"javax.servlet.include.servlet_path");
if(incl==null){
response.flushBuffer();
}
2) It is probably fine for out.flush() to be called at
the end of every JSP page. Optionally, a check like
in
(1) could be used to call response.flushBuffer().
3) MOST IMPORTANTLY -> JspWriterImpl needs to be
rewritten so as to flush the output stream, but NOT to
commit the response and NOT to call
response.flushBuffer()!
I'd say that I consider this a serious bug because it
makes it impossible for a JSP page to be included in
any other JSP or even a Java Servlet except as the
last committment to the response - which means that it
is essentially only a 'forward' action.
Also, the fact that out.flush() (incorrectly) commits
the response actually means that technically TC3.2.1
can not be made compliant with the very spec you cite
because if it DID enforce the flush="true" attribute
and then tried to include a resource (or do ANY output
or forward or redirect), an error would occur becuase
(as I've stated) the response would already be
committed as soon as out.flush() got called.
"Fortunately", tc3.2.1 is currently broken in this
regard and is NOT actually flushing the output before
the include. If the above problem in out.flush() is
fixed, then this should be fixed as well.
This bug has thrown a HUGE wrench into my current
project and I'm not sure how I'm going to address it
in the short term.
Hasn't someone noticed this before? Is this problem
fixed in either of the later versions (3.3 or 4.0)?
If so, I may need to upgrade and I really hate having
to upgrade to non-release versions of software since
that entails a whole lot of risk/justification I have
to explain to my clients.
Sigh... if it ain't one thing, it's another...
Mel
__
Do You Yahoo!?
Yahoo! Auctions - Buy the things you want at great prices! http://auctions.yahoo.com/
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, email: [EMAIL PROTECTED]
RE: TC3.2.1 - response commit on included JSPs
--- Larry Isaacs <[EMAIL PROTECTED]> wrote: > to the spec. Your discussion below about the > JspWriterImpl > seems to be something different. > > So rather than quess, let me ask exactly what are > you > referring to when you say, "dynamic include of a JSP > page"? > > Is this , PageContext.include(), > RequestDispatcher.include(), or am I off track here? > Yes, by 'dynamic' include (the above) I was making the distinction from a 'static' include, which would be done with the syntax <%@ include file="blah.txt" %>. The problem with the TC3.2.1 behavior is that after performing a dynamic include (using any of the above three methods), of a _JSP_ resource (served by TC3.2.1), the response will be committed. Note that if you do a dynamic include of some other resource (such as a servlet) the response would not necessarily be committed (and generally shouldn't be unless a forward or redirect occurs). Mel __ Do You Yahoo!? Yahoo! Auctions - Buy the things you want at great prices! http://auctions.yahoo.com/ - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, email: [EMAIL PROTECTED]
Re: TC3.2.1 - response commit on included JSPs
Re: out.flush() sets the 'committed' state in the
response in tc3.2.1 JspWriterImpl.
--- [EMAIL PROTECTED] 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
> 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/
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, email: [EMAIL PROTECTED]
Re: TC3.2.1 - response commit on included JSPs
> > The problem is that flush() must still be executed
> > as expected,
> > i.e. if an included JSP or servlet is actually
> > calling flush(),
> > it expects the response to be commited.
> >
>
> But does that necessarily mean to commit it to the
> browser? Or should it instead just commit it to the
> buffer of the calling page (which is the actual
> 'client' here)? I think it is the latter case that is
> correct.
The spec is not very clear ( IMHO ), but the comment on
JspWriter.flush() says:
"flush() invocation will flush all the buffers in a chain of
Writers and OutputStreams."
The flush() on ServletOutputStream and flush() in the Writer
are not redefined - and the java.io.OutputStrem.flush()
says:
"Flushes this output stream and forces any buffered output bytes to be
written out. The general contract of flush is that calling it is an
indication that, if any bytes previously written have
been buffered by the implementation of the output stream,
such bytes should immediately be written to their intended
destination. "
My expectation ( as a java user ) would be that if I call
out.flush() the data will be written to the final destination.
> This may be correct for streams in general, but if
> adhered to in servlet-servlet communication, then we
> have to re-think the way we use flush().
>
> Aside from the question of what an explicit flush()
> should do, one also has to worry about what should
> happen when the buffer of the underlying stream simply
> fills up, forcing an implicit flush.
The implicit flush() shouldn't be different ( unless
the special "throw exception" setting is in place ).
> which effectively makes this equivalent to your
> suggestion of replacing the use of out.flush() in the
> finally{..} with out.flushBuffer(). I'll grant that
> if we accept that flush() must always propagate then
> the above solution is not 'elegant'. However, it does
> work just as well. And it at least tries to adhere to
> the principal that included servlets should not commit
> the response.
I tried flushBuffer() and it seems to work as well.
So the only question is how to interpret flush() on
JspWriter.
( for the flush() on OutputStream and Writer it seems
pretty clear from the java.io. comments - and I haven't
found any mention in the servlet spec to alter the way
streams work )
> Neither solution completely blocks an included servlet
> from flushing the parent stream because as currently
> written flushBuffer() can still end up flushing the
> stream if it writes enough bytes to the underlying
> stream to overfill it's buffer, causing an implicit
> flush().
> This could be prevented by rewriting flushBuffer(),
> but I can't do that in my scheme because for some
> reason it is declared 'final' in the current code.
That's easy to change. In any case - the
ServletResponse.flushBuffer should go all the way - that's
specified in RD.include().
> "If the page output is buffered then the buffer is
> flushed prior to the inclusion."
That's a flushBuffer() to me.
> The current jasper code in tc3.2.1 implements
> jsp:include using PageContextImpl.include() which
> simply does:
>
> String path =
>getAbsPathRelativeToContext(relativeUrlPath);
> out.flush();
> context.getRequestDispatcher(path).include(
> request, response);
That should be a flushBuffer - if that's what we want,
or a flush() if we find a place where flush() is
redefined for servlet streams
( i.e. the Writer that is defined in the
servlet spec is not using the same definition for
flush() as java.io.Writer/OutputStream )
The java.io comment seems clear to me - all bytes should
go to their intended destination.
> In other words, the use of out.flush() (as currently
> defined) just prior to the include means you can not
> do something like so:
Agreed.
> So the core problem remains that flush() should NOT
> commit the response.
Or that flush() shouln't be used where the intentions
is to not commit the response.
> The solution you seem to favor is to make
> JspWriterImpl.flushBuffer() public and then changing
> the generated servlet code to have:
If we don't have a clear specification pointer where
flush() is defined as "flush to the last OutputStream, but
don't flush the last stream" or "ServletOutputStream.flush()
shouldn't commit the answer, but be a noop " - I think
we have to do that.
As I understand the spec, JspWriter.flush() will go down,
to the last writer/stream, and ServletOutputStream.flush()
is not re-defined - so OutputStream definition applys ( and
that sounds clear ).
> This still would allow an explicit flush() or a buffer
> overflow to commit the response.
Based on the assumption that this is the correct behavior
of flush().
> I guess I'd be okay with that solution, but I'm not
> sure I'm convinced that even an explicit flush() from
> an included resource should commit the response. It
> just seems counter-intuitive for a 'child' page that
> I
Re: TC3.2.1 - response commit on included JSPs
> 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? It is clear - just that 3.2 has a bug ( and fixing it was not easy because the buffering had problems ). In 3.3 it'll be fixed, I wasn't aware there is a problem. > 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). We can make it public - and the generated servlets already depends on jasper runtime. The problem is that flush() must still be executed as expected, i.e. if an included JSP or servlet is actually calling flush(), it expects the response to be commited. It is possible to disable commit when a servlet/jsp is included, but then we disable a reasonable function (explicit flush() ). Or we can disable the commit just before the final/implicit out.flush() - but again we need an explicit API call. > 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? Then how do you distinguish explicit flush() calls ? And the spec for flush() is clear IMHO - it should propagate and commit the response. > 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. There is a lot of "workaround" in jasper for tomcat buffering problems. Right now the buffering is more flexible ( including the ability to probagate a buffer without commiting the response, better conversions, more control, etc) > 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. :-( I'll try to commit some changes tonight. I'm going to try with the flushBuffer() - it seems to me this is the right way to do it ( and you can still do explicit flush() inside an included jsp and get the expected behavior ) > 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). Can you send at least 2-3 JSP pages and a test scenario ? We need to include this in the nightly tests to make sure it'll be fixed in the final release. Costin - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, email: [EMAIL PROTECTED]
Re: TC3.2.1 - response commit on included JSPs
On Wed, 21 Feb 2001, Mel Martinez wrote: > IMHO out.flush() should not commit the response. Only > response.flushBuffer() should commit the response. > And response.flushBuffer() should not be called from > an inside an 'include' request. 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(). 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 ). 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 fragment that we can include in our nighlty tests ). Costin - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, email: [EMAIL PROTECTED]
Re: TC3.2.1 - response commit on included JSPs
--- [EMAIL PROTECTED] wrote:
>
> > 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).
>
> We can make it public - and the generated servlets
> already depends
> on jasper runtime.
>
> The problem is that flush() must still be executed
> as expected,
> i.e. if an included JSP or servlet is actually
> calling flush(),
> it expects the response to be commited.
>
But does that necessarily mean to commit it to the
browser? Or should it instead just commit it to the
buffer of the calling page (which is the actual
'client' here)? I think it is the latter case that is
correct.
> It is possible to disable commit when a servlet/jsp
> is included,
> but then we disable a reasonable function (explicit
> flush() ).
> Or we can disable the commit just before the
> final/implicit out.flush()
> - but again we need an explicit API call.
>
>
> > 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?
>
> Then how do you distinguish explicit flush() calls ?
> And the spec for flush() is clear IMHO - it should
> propagate
> and commit the response.
>
This may be correct for streams in general, but if
adhered to in servlet-servlet communication, then we
have to re-think the way we use flush().
Aside from the question of what an explicit flush()
should do, one also has to worry about what should
happen when the buffer of the underlying stream simply
fills up, forcing an implicit flush.
>
> I'll try to commit some changes tonight. I'm going
> to try
> with the flushBuffer() - it seems to me this is the
> right
> way to do it ( and you can still do explicit flush()
> inside an
> included jsp and get the expected behavior )
>
I tried the change I suggested, which is to change
JspWriterImpl.flush() to (essentially):
public void flush(){
flushBuffer();
if(out!=null && !isIncluded){
out.flush();
response.flushBuffer();
}
}
which effectively makes this equivalent to your
suggestion of replacing the use of out.flush() in the
finally{..} with out.flushBuffer(). I'll grant that
if we accept that flush() must always propagate then
the above solution is not 'elegant'. However, it does
work just as well. And it at least tries to adhere to
the principal that included servlets should not commit
the response.
Neither solution completely blocks an included servlet
from flushing the parent stream because as currently
written flushBuffer() can still end up flushing the
stream if it writes enough bytes to the underlying
stream to overfill it's buffer, causing an implicit
flush().
This could be prevented by rewriting flushBuffer(),
but I can't do that in my scheme because for some
reason it is declared 'final' in the current code.
Before I go, I want to cast a light on some other
misbehavior in this area. From the JSP 1.1 spec,
section 2.13.4, paragraph 6, on dynamic includes:
"If the page output is buffered then the buffer is
flushed prior to the inclusion."
The current jasper code in tc3.2.1 implements
jsp:include using PageContextImpl.include() which
simply does:
String path =
getAbsPathRelativeToContext(relativeUrlPath);
out.flush();
context.getRequestDispatcher(path).include(
request, response);
As it stands, this will always commit the response
prior to the include. This is not a problem for the
include, per se, but this is not what performing an
include should do because it alters the commit state
of the current page's response object.
In other words, the use of out.flush() (as currently
defined) just prior to the include means you can not
do something like so:
<%
response.sendRedirect("http://somewhere");
%>
This breaks even if neither the current page or the
included page have actually written anything to the
stream simply because out.flush() is committing the
response prematurely.
So the core problem remains that flush() should NOT
commit the response.
As before, replacing the use of out.flush() in
PageContextImpl.include() with just the functionality
of out.flushBuffer() fixes the problem with the same
exception of the case where the amount of output
written to the underlying servletoutputstream causes
an implicit flush.
The solution you seem to favor is to make
JspWriterImpl.flushBuffer() public and then changing
the generated servlet code to have:
}finally{
((JspWriterImpl)out).flushBuffer();
...
}
and to similarly replace the out.flush() in
PageContextImpl.include().
This still would allow an explicit flush() or a buffer
overflow to commit the response.
I guess I'd be okay with that solution, but I'm not
sure I'm convin
Re: TC3.2.1 - response commit on included JSPs
--- [EMAIL PROTECTED] wrote:
>
>
> My expectation ( as a java user ) would be that if I
> call
> out.flush() the data will be written to the final
> destination.
>
I won't contest that, but I would suggest considering
the idea that the final, or intended destination is
not necessarily the browser. In the case of an
included resource, it is the calling servlet that is
acting as client to the included resource. In good OO
design, it should be in the control of that client
what to do with the data that has been 'committed' by
the included resource.
> That's easy to change. In any case - the
> ServletResponse.flushBuffer should go all the way -
> that's
> specified in RD.include().
>
Here is perhaps some clarification, from the proposed
Servlet 2.3 spec. [sec 8.3 Include]:
"The target servlet has access to all aspects of the
request object, but can only write
information to the ServletOutputStream or Writer of
the response object as well as
the ability to commit a response by either writing
content past the end of the response buffer
or explicitly calling the flush method of the
ServletResponse interface. The included
servlet cannot set headers or call any method that
affects the headers of the response. Any
attempt to do so should be ignored."
This implies that our proposed solution of using
JspWriterImpl.flushBuffer() everywhere (in the JSP
finally{} clause and in PageContextImpl.include())
instead of out.flush() is the intended behavior. I am
not exactly impressed with this design spec, but I'll
accept it.
So that would seem to fix things. However...
Looking at the Tomcat 3.2.1 code, I see that this
situation is still further broken in this regard
because inside inside the
org.apache.tomcat.facade.RequestDispatcherImpl class
the include() method _explicitely_ (and incorrectly)
calls response.flushBuffer(). I realize from the
comments that it is trying to preserve session info
from being blasted by an included servlet - but that
patch is only needed because the design allows the
included servlet to commit the response!
The problem with that call being there is it
automatically forces the commit - even if the included
resource would not have. This simply is not
acceptable.
I realize also, from looking around in the CVS
repositories that the 'facade' package is history so I
am not familiar yet with how newer versions of tomcat
implement the include() method. Have they removed
this offending call?
It becomes clearer and clearer to me that I'm going to
have to swallow upgrading to a newer version of Tomcat
than 3.2.1. Which version should I jump in on?
Sigh...
>
> The java.io comment seems clear to me - all bytes
> should
> go to their intended destination.
>
The intended destination of a stream is determined by
the calling code, not the included stream itself
because the calling code is the determinant of the
target.
In theory, one could build an include() mechanism that
substitutes a separate buffer (i.e. a
ByteArrayOutputStream) for the servlet output stream
when passing a response object over to the included
resource. In that scenario, a flush() by an included
resource would properly flush data to the buffer, but
the calling page would still have control over whether
it went to the browser.
I agree, though that this does not match what the
proposed servlet 2.3. spec (above) allows. I would
argue that the proposed spec is not necessarily the
best design.
Oh well. At the moment, I need to get past the broken
RequestDispatcher.include() in TC 3.2.1.
Cheers,
Mel
__
Do You Yahoo!?
Yahoo! Auctions - Buy the things you want at great prices! http://auctions.yahoo.com/
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, email: [EMAIL PROTECTED]
Re: TC3.2.1 - response commit on included JSPs
> I won't contest that, but I would suggest considering > the idea that the final, or intended destination is > not necessarily the browser. In the case of an > included resource, it is the calling servlet that is > acting as client to the included resource. In good OO > design, it should be in the control of that client > what to do with the data that has been 'committed' by > the included resource. Except that this is explicitely specified - the flush() in JspWriter should be propagated to the next stream. " if that destination is another character or byte stream, flush it." In this case, the destination is the servlet output stream - and it has to be flushed. And it is also explicitely stated that the included servlet can commit the response ( in RequestDispatcher spec ) > Looking at the Tomcat 3.2.1 code, I see that this > situation is still further broken in this regard > because inside inside the We had a lot of problems with buffering - and changing 3.2.1 may be very dangerous ( because of many inter-dependecies like session,etc). The main change in 3.3 is removing many of the dependencies and cleaning the code. M > I realize also, from looking around in the CVS > repositories that the 'facade' package is history so I > am not familiar yet with how newer versions of tomcat > implement the include() method. Have they removed > this offending call? Yes, and the whole buffering has been reimplemented. BTW, both solution should work with the current code - you can make the OutputBuffer ( the main component in the new buffer implementation ) ignore the flush(), etc. ( in 3.3 the Buffer is the central element - instead of hiding it inside the Stream it is now a first-class internal object, that can be controled via API ) > It becomes clearer and clearer to me that I'm going to have to swallow > upgrading to a newer version of Tomcat than 3.2.1. Which version > should I jump in on? Sigh... I would say 3.3 :-) We hope to have a beta in few weeks - the code is already usable and stable. Costin - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, email: [EMAIL PROTECTED]
Re: TC3.2.1 - response commit on included JSPs
--- [EMAIL PROTECTED] wrote: > > I won't contest that, but I would suggest > considering > > the idea that the final, or intended destination > is > > not necessarily the browser. In the case of an > > included resource, it is the calling servlet that > is > > acting as client to the included resource. In > good OO > > design, it should be in the control of that client > > what to do with the data that has been 'committed' > by > > the included resource. > > Except that this is explicitely specified - the > flush() > in JspWriter should be propagated to the next > stream. > > " if that destination is another character or > byte stream, flush it." > > In this case, the destination is the servlet output > stream - and it has to be flushed. That is only because the particular servlet engine implementation implements the ServletOutputStream of the Response object passed to the included servlet by wrapping it around (or simply passing on) the current ServletOutputStream. Technically, that does not have to be so. > > And it is also explicitely stated that the included > servlet can commit the response ( in > RequestDispatcher > spec ) > This is true. My argument then is only that I think the spec is bad in this regard. However, as I stated, I can live with it. > > Looking at the Tomcat 3.2.1 code, I see that this > > situation is still further broken in this regard > > The main change in 3.3 is removing many of the > dependencies and cleaning the code. M > I like what you've alluded to in the 'buffer' being a first class object - that sounds more correct. > > I would say 3.3 :-) > > We hope to have a beta in few weeks - the code is > already > usable and stable. > So, I'm trying to decide whether to use the latest milestone build, nightly build or to setup a cvs project and simply track the latest code with the eventual goal of helping to contribute. Having to build one of my deployment tools from source is not something I really wanted to do from a project management perspective, but oh well I presume I can hook up as a 'read only' user of the tc 3.3 cvs repositories with wincvs? Where can I find the necessary repository url and authentication info? Or can I actually checkout the whole project via cvsweb? Looks like I need to do some poking around... This will be a bit of a learning experience since I've never built with Ant before. :-) Thanks for the discussion, Costin. Mel __ Do You Yahoo!? Get email at your own domain with Yahoo! Mail. http://personal.mail.yahoo.com/ - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, email: [EMAIL PROTECTED]
Re: TC3.2.1 - response commit on included JSPs
> > " if that destination is another character or > > byte stream, flush it." > > > > In this case, the destination is the servlet output > > stream - and it has to be flushed. > > That is only because the particular servlet engine > implementation implements the ServletOutputStream of > the Response object passed to the included servlet by > wrapping it around (or simply passing on) the current > ServletOutputStream. Technically, that does not have > to be so. Ok, the final destination is the user's browser ( where bits will eventually go ). The meaning of flush() seems to be - empty all intermediary buffers. > This is true. My argument then is only that I think > the spec is bad in this regard. However, as I stated, > I can live with it. +1 on that :-) > So, I'm trying to decide whether to use the latest > milestone build, nightly build or to setup a cvs > project and simply track the latest code with the > eventual goal of helping to contribute. Having to > build one of my deployment tools from source is not > something I really wanted to do from a project > management perspective, but oh well The release plan is to have a beta soon, and so far it seems to work fine ( or at least my optimistic look at the bugs doesn't show any stopper ). > I presume I can hook up as a 'read only' user of the > tc 3.3 cvs repositories with wincvs? Where can I find > the necessary repository url and authentication info? > Or can I actually checkout the whole project via > cvsweb? Looks like I need to do some poking around... It's the main branch of jakarta-tomcat, with nightly snapshots and builds available :-). Costin - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, email: [EMAIL PROTECTED]
Autoreply: Re: TC3.2.1 - response commit on included JSPs
This is not a valid address at SNL Securities (http://www.snl.com). Please remove him/her from your mailing list or address book. Your message reads: Received: from snlexch.snl.com (unverified [10.0.1.9]) by mail.snl.com (Rockliffe SMTPRA 4.5.4) with ESMTP id <[EMAIL PROTECTED]> for <[EMAIL PROTECTED]>; Thu, 22 Feb 2001 14:27:50 -0500 Received: by SNLEXCH with Internet Mail Service (5.5.2653.19) id ; Thu, 22 Feb 2001 14:25:31 -0500 Received: from mail.snl.com (SNLDMZBDC [10.0.0.7]) by snlexch.snl.com with SMTP (Microsoft Exchange Internet Mail Service Version 5.5.2653.13) id F3WAVJ8S; Thu, 22 Feb 2001 14:25:29 -0500 Received: from apache.org (unverified [64.208.42.41]) by mail.snl.com (Rockliffe SMTPRA 4.5.4) with SMTP id <[EMAIL PROTECTED]> for <[EMAIL PROTECTED]>; Thu, 22 Feb 2001 14:27:32 -0500 Received: (qmail 73552 invoked by uid 500); 22 Feb 2001 17:52:59 - Received: (qmail 73334 invoked from network); 22 Feb 2001 17:52:56 - Received: from kyoto.javasoft.com (204.160.241.223) by h31.sny.collab.net with SMTP; 22 Feb 2001 17:52:56 - Received: from localhost (localhost [127.0.0.1]) by kyoto.javasoft.com (8.9.3/8.9.3) with ESMTP id JAA19479; Thu, 22 Feb 2001 09:25:33 -0800 Mailing-List: contact [EMAIL PROTECTED]; run by ezmlm Precedence: bulk Reply-To: [EMAIL PROTECTED] list-help: <mailto:[EMAIL PROTECTED]> list-unsubscribe: <mailto:[EMAIL PROTECTED]> list-post: <mailto:[EMAIL PROTECTED]> Delivered-To: mailing list [EMAIL PROTECTED] From: [EMAIL PROTECTED] Date: Thu, 22 Feb 2001 09:52:58 -0800 (PST) X-Sender: [EMAIL PROTECTED] To: [EMAIL PROTECTED] cc: [EMAIL PROTECTED] Subject: Re: TC3.2.1 - response commit on included JSPs In-Reply-To: <[EMAIL PROTECTED]> Message-ID: <[EMAIL PROTECTED]> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Spam-Rating: h31.sny.collab.net 1.6.2 0/1000/N On Wed, 21 Feb 2001, Mel Martinez wrote: > IMHO out.flush() should not commit the response. Only > response.flushBuffer() should commit the response. > And response.flushBuffer() should not be called from > an inside an 'include' request. 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(). 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 ). 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 fragment that we can include in our nighlty tests ). Costin - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, email: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, email: [EMAIL PROTECTED]
Autoreply: Re: TC3.2.1 - response commit on included JSPs
This is not a valid address at SNL Securities (http://www.snl.com). Please remove
him/her from your mailing list or address book.
Your message reads:
Received: from snlexch.snl.com (unverified [10.0.1.9]) by mail.snl.com
(Rockliffe SMTPRA 4.5.4) with ESMTP id <[EMAIL PROTECTED]> for
<[EMAIL PROTECTED]>;
Thu, 22 Feb 2001 14:47:39 -0500
Received: by SNLEXCH with Internet Mail Service (5.5.2653.19)
id ; Thu, 22 Feb 2001 14:45:20 -0500
Received: from mail.snl.com (SNLDMZBDC [10.0.0.7]) by snlexch.snl.com with SMTP
(Microsoft Exchange Internet Mail Service Version 5.5.2653.13)
id F3WAVKH7; Thu, 22 Feb 2001 14:45:17 -0500
Received: from apache.org (unverified [64.208.42.41]) by mail.snl.com
(Rockliffe SMTPRA 4.5.4) with SMTP id <[EMAIL PROTECTED]> for <[EMAIL PROTECTED]>;
Thu, 22 Feb 2001 14:47:36 -0500
Received: (qmail 26304 invoked by uid 500); 22 Feb 2001 19:45:51 -
Received: (qmail 26250 invoked from network); 22 Feb 2001 19:45:49 -
Received: from web1902.mail.yahoo.com (128.11.23.51)
by h31.sny.collab.net with SMTP; 22 Feb 2001 19:45:49 -
Received: (qmail 25194 invoked by uid 60001); 22 Feb 2001 19:45:52 -
Received: from [24.5.88.209] by web1902.mail.yahoo.com; Thu, 22 Feb 2001 11:45:52 PST
Mailing-List: contact [EMAIL PROTECTED]; run by ezmlm
Precedence: bulk
Reply-To: [EMAIL PROTECTED]
list-help: <mailto:[EMAIL PROTECTED]>
list-unsubscribe: <mailto:[EMAIL PROTECTED]>
list-post: <mailto:[EMAIL PROTECTED]>
Delivered-To: mailing list [EMAIL PROTECTED]
Message-ID: <[EMAIL PROTECTED]>
Date: Thu, 22 Feb 2001 11:45:52 -0800 (PST)
From: Mel Martinez <[EMAIL PROTECTED]>
Subject: Re: TC3.2.1 - response commit on included JSPs
To: [EMAIL PROTECTED], [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
In-Reply-To: <[EMAIL PROTECTED]>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
X-Spam-Rating: h31.sny.collab.net 1.6.2 0/1000/N
Re: out.flush() sets the 'committed' state in the
response in tc3.2.1 JspWriterImpl.
--- [EMAIL PROTECTED] 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
Autoreply: Re: TC3.2.1 - response commit on included JSPs
This is not a valid address at SNL Securities (http://www.snl.com). Please remove
him/her from your mailing list or address book.
Your message reads:
Received: from snlexch.snl.com (unverified [10.0.1.9]) by mail.snl.com
(Rockliffe SMTPRA 4.5.4) with ESMTP id <[EMAIL PROTECTED]> for
<[EMAIL PROTECTED]>;
Thu, 22 Feb 2001 14:48:13 -0500
Received: by SNLEXCH with Internet Mail Service (5.5.2653.19)
id ; Thu, 22 Feb 2001 14:45:54 -0500
Received: from mail.snl.com (SNLDMZBDC [10.0.0.7]) by snlexch.snl.com with SMTP
(Microsoft Exchange Internet Mail Service Version 5.5.2653.13)
id F3WAVK23; Thu, 22 Feb 2001 14:45:49 -0500
Received: from apache.org (unverified [64.208.42.41]) by mail.snl.com
(Rockliffe SMTPRA 4.5.4) with SMTP id <[EMAIL PROTECTED]> for <[EMAIL PROTECTED]>;
Thu, 22 Feb 2001 14:48:08 -0500
Received: (qmail 26501 invoked by uid 500); 22 Feb 2001 19:45:58 -
Received: (qmail 26452 invoked from network); 22 Feb 2001 19:45:53 -
Received: from web1902.mail.yahoo.com (128.11.23.51)
by h31.sny.collab.net with SMTP; 22 Feb 2001 19:45:53 -
Received: (qmail 25194 invoked by uid 60001); 22 Feb 2001 19:45:52 -
Received: from [24.5.88.209] by web1902.mail.yahoo.com; Thu, 22 Feb 2001 11:45:52 PST
Mailing-List: contact [EMAIL PROTECTED]; run by ezmlm
Precedence: bulk
Reply-To: [EMAIL PROTECTED]
list-help: <mailto:[EMAIL PROTECTED]>
list-unsubscribe: <mailto:[EMAIL PROTECTED]>
list-post: <mailto:[EMAIL PROTECTED]>
Delivered-To: mailing list [EMAIL PROTECTED]
Message-ID: <[EMAIL PROTECTED]>
Date: Thu, 22 Feb 2001 11:45:52 -0800 (PST)
From: Mel Martinez <[EMAIL PROTECTED]>
Subject: Re: TC3.2.1 - response commit on included JSPs
To: [EMAIL PROTECTED], [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
In-Reply-To: <[EMAIL PROTECTED]>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
X-Spam-Rating: h31.sny.collab.net 1.6.2 0/1000/N
Re: out.flush() sets the 'committed' state in the
response in tc3.2.1 JspWriterImpl.
--- [EMAIL PROTECTED] 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
