You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2007/10/05 16:51:39 UTC
svn commit: r582291 - /tomcat/tc6.0.x/trunk/STATUS
Author: remm
Date: Fri Oct 5 07:51:35 2007
New Revision: 582291
URL: http://svn.apache.org/viewvc?rev=582291&view=rev
Log:
- Propose r580815 for review.
Modified:
tomcat/tc6.0.x/trunk/STATUS
Modified: tomcat/tc6.0.x/trunk/STATUS
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS?rev=582291&r1=582290&r2=582291&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS (original)
+++ tomcat/tc6.0.x/trunk/STATUS Fri Oct 5 07:51:35 2007
@@ -25,3 +25,9 @@
PATCHES PROPOSED TO BACKPORT:
[ New proposals should be added at the end of the list ]
+* Fix explicit flush before response commit in the org.apache.jk AJP connector.
+ http://svn.apache.org/viewvc?view=rev&revision=580815
+
+ +1: remm
+ -1:
+
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r582291 - /tomcat/tc6.0.x/trunk/STATUS
Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
Rainer Jung wrote:
> Remy Maucherat wrote:
>> Rainer Jung wrote:
>>> So my question is: is there a reliable way to detect, if it would be
>>> safer to just drop a flush request?
>>
>> I think flush should still send a flush packet. One scenario would be
>> (in the servlet): write 10 bytes and flush (in this case, the
>> response has not been committed yet). The server should write the
>> response header, then make sure the 10 bytes are sent because it has
>> to send them right away. AFAIK, the front end server will not do that
>> without the explicit flush.
>
> If it's 10 body bytes (thinking about http), then in principle OK, but
> see below.
>
> What if the flush comes before the body and the headers are not yet
> setup for the future body? Then we send out kind of default headers
> not fitting to the response generated afterwards. Of course the flush
> command coming from the webapp is the root of the evil, but in that
> case, doing the flush would not be necessary and harmful.
as you mention, the webapp calls flush, the servlet container must
submit headers, buffering and not sending headers, even though the
client wants to submit 10 bytes, would be erronous, and not sending any
headers, but just the body, is protocol violation :)
>
> I looked around in the existing code and found some strange things:
>
> a) the patch we are talking about is for JK and we do something
> similar for http already. This might be a plus point for the patch.
>
> b) the jk code path looks like the sending of the headers resets the
> output buffer, so uncommitted response bytes will be lost
> (JkInputStream.appendHead() first does a outputMsg.reset())
>
> c) For http I've got the impression, that
> Http11Processor.prepareResponse() triggers direct writes via
> InternalOutputBuffer (sendStatus(), write() and sendHeaders()). This
> looks like uncommitted body parts will get out before the headers.
>
> d) in the http case all relevant implementations of doWrite seem to
> implicitely commit the response, if it is not already committed (e.g.
> in InternalOutputBuffer). In InternalOutputBuffer, a flush() also
> implies a commit. Assuming that I'm looking at the right classes, this
> might fix c), but would also let a) for http (i.e. the commit before
> flush) be unnecessary.
>
> e) In the JK case, doWrite() does an implicit commit in
> JkInputStream but not in RequestHandler. flush does not do an implicit
> commit in ChannelSocket. Again assuming the classes are correct, the
> JK case indeed needs additional commits to get the headers back early
> enough, but those will destroy the uncommitted content when sending
> out the headers.
not too familiar with this code either, but a flush, should always do an
implicit commit, as soon as the headers have gone out the response
is(should be) committed.
Filip
>
> As you all know, the class structure is not easy and I might be wrong.
>
> Rainer
>
> ---------------------------------------------------------------------
> 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: svn commit: r582291 - /tomcat/tc6.0.x/trunk/STATUS
Posted by Rainer Jung <ra...@kippdata.de>.
Remy Maucherat wrote:
> Rainer Jung wrote:
>> So my question is: is there a reliable way to detect, if it would be
>> safer to just drop a flush request?
>
> I think flush should still send a flush packet. One scenario would be
> (in the servlet): write 10 bytes and flush (in this case, the response
> has not been committed yet). The server should write the response
> header, then make sure the 10 bytes are sent because it has to send them
> right away. AFAIK, the front end server will not do that without the
> explicit flush.
If it's 10 body bytes (thinking about http), then in principle OK, but
see below.
What if the flush comes before the body and the headers are not yet
setup for the future body? Then we send out kind of default headers not
fitting to the response generated afterwards. Of course the flush
command coming from the webapp is the root of the evil, but in that
case, doing the flush would not be necessary and harmful.
I looked around in the existing code and found some strange things:
a) the patch we are talking about is for JK and we do something similar
for http already. This might be a plus point for the patch.
b) the jk code path looks like the sending of the headers resets the
output buffer, so uncommitted response bytes will be lost
(JkInputStream.appendHead() first does a outputMsg.reset())
c) For http I've got the impression, that
Http11Processor.prepareResponse() triggers direct writes via
InternalOutputBuffer (sendStatus(), write() and sendHeaders()). This
looks like uncommitted body parts will get out before the headers.
d) in the http case all relevant implementations of doWrite seem to
implicitely commit the response, if it is not already committed (e.g. in
InternalOutputBuffer). In InternalOutputBuffer, a flush() also implies a
commit. Assuming that I'm looking at the right classes, this might fix
c), but would also let a) for http (i.e. the commit before flush) be
unnecessary.
e) In the JK case, doWrite() does an implicit commit in
JkInputStream but not in RequestHandler. flush does not do an implicit
commit in ChannelSocket. Again assuming the classes are correct, the JK
case indeed needs additional commits to get the headers back early
enough, but those will destroy the uncommitted content when sending out
the headers.
As you all know, the class structure is not easy and I might be wrong.
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r582291 - /tomcat/tc6.0.x/trunk/STATUS
Posted by Remy Maucherat <re...@apache.org>.
Rainer Jung wrote:
> remm@apache.org wrote:
>> +* Fix explicit flush before response commit in the org.apache.jk AJP
>> connector.
>> + http://svn.apache.org/viewvc?view=rev&revision=580815
>> + + +1: remm
>
> I admit, that I don't have a better solution, but nevertheless a
> question: In case the flush comes to early, i.e. before the headers are
> set up correctly, it wcould be safer to drop the flush.
>
> The reason for a flush usually is, to make sure, that prepared
> information reaches the client soon, and is not kept back by any buffering.
>
> Detecting an unconditional flush is not easy, so I'm OK to commit and
> flush, whenever it makes sense. But if we are to early in the request
> handling, we will send out headers, that might not fit to the response
> generated later.
>
> So my question is: is there a reliable way to detect, if it would be
> safer to just drop a flush request?
I think flush should still send a flush packet. One scenario would be
(in the servlet): write 10 bytes and flush (in this case, the response
has not been committed yet). The server should write the response
header, then make sure the 10 bytes are sent because it has to send them
right away. AFAIK, the front end server will not do that without the
explicit flush.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r582291 - /tomcat/tc6.0.x/trunk/STATUS
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Oct 5, 2007, at 11:42 AM, Rainer Jung wrote:
> remm@apache.org wrote:
>> +* Fix explicit flush before response commit in the org.apache.jk
>> AJP connector.
>> + http://svn.apache.org/viewvc?view=rev&revision=580815
>> + + +1: remm
>
> I admit, that I don't have a better solution, but nevertheless a
> question: In case the flush comes to early, i.e. before the headers
> are set up correctly, it wcould be safer to drop the flush.
>
> The reason for a flush usually is, to make sure, that prepared
> information reaches the client soon, and is not kept back by any
> buffering.
>
> Detecting an unconditional flush is not easy, so I'm OK to commit
> and flush, whenever it makes sense. But if we are to early in the
> request handling, we will send out headers, that might not fit to
> the response generated later.
>
> So my question is: is there a reliable way to detect, if it would
> be safer to just drop a flush request?
>
Possibly... but we can also possible document this (mis)behavior
with the caveat that "clients" need to be able to handle this
correctly...
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r582291 - /tomcat/tc6.0.x/trunk/STATUS
Posted by Rainer Jung <ra...@kippdata.de>.
remm@apache.org wrote:
> +* Fix explicit flush before response commit in the org.apache.jk AJP connector.
> + http://svn.apache.org/viewvc?view=rev&revision=580815
> +
> + +1: remm
I admit, that I don't have a better solution, but nevertheless a
question: In case the flush comes to early, i.e. before the headers are
set up correctly, it wcould be safer to drop the flush.
The reason for a flush usually is, to make sure, that prepared
information reaches the client soon, and is not kept back by any buffering.
Detecting an unconditional flush is not easy, so I'm OK to commit and
flush, whenever it makes sense. But if we are to early in the request
handling, we will send out headers, that might not fit to the response
generated later.
So my question is: is there a reliable way to detect, if it would be
safer to just drop a flush request?
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org