You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bi...@apache.org on 2005/12/29 08:58:55 UTC
svn commit: r359753 -
/tomcat/connectors/trunk/http11/src/java/org/apache/coyote/http11/Http11Processor.java
Author: billbarker
Date: Wed Dec 28 23:58:51 2005
New Revision: 359753
URL: http://svn.apache.org/viewcvs?rev=359753&view=rev
Log:
Unconditionally return EOS for an attempt to read the body of any request that doesn't send CL or TE.
I haven't seen any real objections to the patch (and it can't break a working HTTP/1.0 client :). Also, it brings us in like with what Httpd does in this case.
Fix for Bug #38030
Submitted By: Remy
Modified:
tomcat/connectors/trunk/http11/src/java/org/apache/coyote/http11/Http11Processor.java
Modified: tomcat/connectors/trunk/http11/src/java/org/apache/coyote/http11/Http11Processor.java
URL: http://svn.apache.org/viewcvs/tomcat/connectors/trunk/http11/src/java/org/apache/coyote/http11/Http11Processor.java?rev=359753&r1=359752&r2=359753&view=diff
==============================================================================
--- tomcat/connectors/trunk/http11/src/java/org/apache/coyote/http11/Http11Processor.java (original)
+++ tomcat/connectors/trunk/http11/src/java/org/apache/coyote/http11/Http11Processor.java Wed Dec 28 23:58:51 2005
@@ -1327,18 +1327,13 @@
parseHost(valueMB);
if (!contentDelimitation) {
- // If there's no content length and we're using keep-alive
- // (HTTP/1.0 with keep-alive or HTTP/1.1), assume
+ // If there's no content length
+ // (broken HTTP/1.0 or HTTP/1.1), assume
// the client is not broken and didn't send a body
- if (keepAlive) {
- inputBuffer.addActiveFilter
+ inputBuffer.addActiveFilter
(inputFilters[Constants.VOID_FILTER]);
- contentDelimitation = true;
- }
+ contentDelimitation = true;
}
-
- if (!contentDelimitation)
- keepAlive = false;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r359753 - /tomcat/connectors/trunk/http11/src/java/org/apache/coyote/http11/Http11Processor.java
Posted by Bill Barker <wb...@wilshire.com>.
"Remy Maucherat" <re...@apache.org> wrote in message
news:43B6BB52.9020802@apache.org...
> Bill Barker wrote:
>> Without actually checking, mod_jk/mod_proxy_ajp should only send an
>> unrequested initial bodyChunk if the client sends a CL. So if there is
>> no CL, Tomcat will send back GET_BODY_CHUNK message, and act on the
>> response from Httpd/IIS/SunOne.
>>
>> The main purpose of the "read required" field is to clean up if the
>> Servlet fails to actually read the (small) Request body, even though
>> Httpd sent it to Tomcat. Now (like the APR/AJP Connector), Tomcat
>> doesn't attempt to actually read (by default) the unrequested initial
>> bodyChunk until the Servlet asks for it. However, if Httpd sent it, and
>> the Servlet didn't ask for it, Tomcat needs to get it out of the the
>> Socket stream, or bad things are going to happen ;-).
>
> I have no idea if I need to make a similar change in APR AJP. This stuff
> is a bit black magic to me.
>
It looks like the APR/AJP connector will have similar problems (but I
haven't tried it :). The 'try to read body when none sent' case (BZ 38047)
is easy to test. You should just see the Request hang forever. The
'servlet doesn't read the body' case should look like it's just really slow,
since mod_jk/mod_proxy_ajp will recover from it, but you should see lots of
stuff in the logs.
What this is about is that (for the sake of shaving a couple of ms :) tha
AJP/1.3 protocol is inconsistant when the CL > 0. In this case,
mod_jk/mod_proxy_ajp will send two packets initially: The normal
Request-Header packet, followed by the first Body-Chunk packet. This is
where the special handling is required. It used to be the case that the
JK/Java Connector would look for the CL > 0, and read the Body-Chunk packet
right away. This worked fine in keeping the special-case handling
restricted to one small place. However, when I was benchmarking it, it
seemed that Tomcat would get to the read a lot of times before Httpd got to
the write (so Tomcat would block needlessly). That's when I changed it to
be more like the APR/AJP Connector.
Because the APR/AJP Connector isn't so over-designed, you could probably do
a cleaner solution than my (e.g. checking CL directly, instead of setting a
flag).
> R�my
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r359753 - /tomcat/connectors/trunk/http11/src/java/org/apache/coyote/http11/Http11Processor.java
Posted by Remy Maucherat <re...@apache.org>.
Bill Barker wrote:
> Without actually checking, mod_jk/mod_proxy_ajp should only send an
> unrequested initial bodyChunk if the client sends a CL. So if there is no
> CL, Tomcat will send back GET_BODY_CHUNK message, and act on the response
> from Httpd/IIS/SunOne.
>
> The main purpose of the "read required" field is to clean up if the Servlet
> fails to actually read the (small) Request body, even though Httpd sent it
> to Tomcat. Now (like the APR/AJP Connector), Tomcat doesn't attempt to
> actually read (by default) the unrequested initial bodyChunk until the
> Servlet asks for it. However, if Httpd sent it, and the Servlet didn't ask
> for it, Tomcat needs to get it out of the the Socket stream, or bad things
> are going to happen ;-).
I have no idea if I need to make a similar change in APR AJP. This stuff
is a bit black magic to me.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r359753 - /tomcat/connectors/trunk/http11/src/java/org/apache/coyote/http11/Http11Processor.java
Posted by Mladen Turk <mt...@apache.org>.
Bill Barker wrote:
>
> Without actually checking, mod_jk/mod_proxy_ajp should only send an
> unrequested initial bodyChunk if the client sends a CL. So if there is no
> CL, Tomcat will send back GET_BODY_CHUNK message, and act on the response
> from Httpd/IIS/SunOne.
>
For reading initial body mod_jk( and mod_ajp) relies on web
server providing initial body. For apache we use ap_get_client_block
that will return body length if CL is present or in case of chunked
encoding.
There is however one more thing that needs to be resolved and is
pretty important.
JK code in Tomcat has no way to figure out if the
AJP13_SEND_BODY_CHUNK has failed or not due to client error
(for example client closed browser before entire data has been sent).
Now I think that the JK connector recycles the current send
loop in case there is a 'new' request instead SEND_BODY_CHUNK
while the connector was in the send loop, but it does not
propagate that to the upper chain, so Tomcat on the next
request returns 304 (Not Modified) for the same resource
although there was error in previous send.
So, in case mod_jk sends AJP13_FORWARD_REQUEST while the
Tomcat part was in the SEND_BODY_CHUNK chunk loop the Tomcat
part of connector must behave like the connection was closed
and then re-established (without closing the socket of course).
Regards,
Mladen.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r359753 - /tomcat/connectors/trunk/http11/src/java/org/apache/coyote/http11/Http11Processor.java
Posted by Bill Barker <wb...@wilshire.com>.
"Remy Maucherat" <re...@apache.org> wrote in message
news:43B3A335.9040001@apache.org...
> billbarker@apache.org wrote:
>> Author: billbarker
>> Date: Wed Dec 28 23:58:51 2005
>> New Revision: 359753
>>
>> URL: http://svn.apache.org/viewcvs?rev=359753&view=rev
>> Log:
>> Unconditionally return EOS for an attempt to read the body of any request
>> that doesn't send CL or TE.
>>
>> I haven't seen any real objections to the patch (and it can't break a
>> working HTTP/1.0 client :). Also, it brings us in like with what Httpd
>> does in this case.
>>
>> Fix for Bug #38030
>> Submitted By: Remy
>
> I have to do a separate, identical, commit for the HTTP connector people
> should be using, then.
>
> In JK, I don't understand the usage of the "read required" field. It seems
> to be set according to:
> // Check to see if there should be a body packet coming along
> // immediately after
> int cl=req.getContentLength();
> if(cl > 0) {
> JkInputStream jkIS = ep.getInputStream();
> jkIS.setIsReadRequired(true);
> if(!delayInitialRead) {
> jkIS.receive();
> }
> }
>
> Does this work with chunking ?
>
Without actually checking, mod_jk/mod_proxy_ajp should only send an
unrequested initial bodyChunk if the client sends a CL. So if there is no
CL, Tomcat will send back GET_BODY_CHUNK message, and act on the response
from Httpd/IIS/SunOne.
The main purpose of the "read required" field is to clean up if the Servlet
fails to actually read the (small) Request body, even though Httpd sent it
to Tomcat. Now (like the APR/AJP Connector), Tomcat doesn't attempt to
actually read (by default) the unrequested initial bodyChunk until the
Servlet asks for it. However, if Httpd sent it, and the Servlet didn't ask
for it, Tomcat needs to get it out of the the Socket stream, or bad things
are going to happen ;-).
> R�my
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r359753 - /tomcat/connectors/trunk/http11/src/java/org/apache/coyote/http11/Http11Processor.java
Posted by Remy Maucherat <re...@apache.org>.
billbarker@apache.org wrote:
> Author: billbarker
> Date: Wed Dec 28 23:58:51 2005
> New Revision: 359753
>
> URL: http://svn.apache.org/viewcvs?rev=359753&view=rev
> Log:
> Unconditionally return EOS for an attempt to read the body of any request that doesn't send CL or TE.
>
> I haven't seen any real objections to the patch (and it can't break a working HTTP/1.0 client :). Also, it brings us in like with what Httpd does in this case.
>
> Fix for Bug #38030
> Submitted By: Remy
I have to do a separate, identical, commit for the HTTP connector people
should be using, then.
In JK, I don't understand the usage of the "read required" field. It
seems to be set according to:
// Check to see if there should be a body packet coming along
// immediately after
int cl=req.getContentLength();
if(cl > 0) {
JkInputStream jkIS = ep.getInputStream();
jkIS.setIsReadRequired(true);
if(!delayInitialRead) {
jkIS.receive();
}
}
Does this work with chunking ?
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org