You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by gr...@apache.org on 2007/04/30 20:16:07 UTC
svn commit: r533820 - /httpd/httpd/trunk/modules/http/http_request.c
Author: gregames
Date: Mon Apr 30 11:16:06 2007
New Revision: 533820
URL: http://svn.apache.org/viewvc?view=rev&rev=533820
Log:
check_pipeline: use AP_MODE_SPECULATIVE to check for data in the input filters
to accomodate mod_ssl's input filter. AP_MODE_EATCRLF is essentially a no-op
in that filter.
this fixes missing imbedded graphics etc when using the Event MPM with mod_ssl
with HTTP pipelining enabled in the browser. the pipelined requests after the
first were never read from mod_ssl's input filter. it might reduce unneeded
flushes with other MPMs.
Modified:
httpd/httpd/trunk/modules/http/http_request.c
Modified: httpd/httpd/trunk/modules/http/http_request.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_request.c?view=diff&rev=533820&r1=533819&r2=533820
==============================================================================
--- httpd/httpd/trunk/modules/http/http_request.c (original)
+++ httpd/httpd/trunk/modules/http/http_request.c Mon Apr 30 11:16:06 2007
@@ -193,11 +193,10 @@
static void check_pipeline(conn_rec *c)
{
- /* ### is zero correct? that means "read one line" */
if (c->keepalive != AP_CONN_CLOSE) {
apr_bucket_brigade *bb = apr_brigade_create(c->pool, c->bucket_alloc);
- if (ap_get_brigade(c->input_filters, bb, AP_MODE_EATCRLF,
- APR_NONBLOCK_READ, 0) != APR_SUCCESS) {
+ if (ap_get_brigade(c->input_filters, bb, AP_MODE_SPECULATIVE,
+ APR_NONBLOCK_READ, 1) != APR_SUCCESS) {
c->data_in_input_filters = 0; /* we got APR_EOF or an error */
}
else {
Re: svn commit: r533820 -
/httpd/httpd/trunk/modules/http/http_request.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
gregames@apache.org wrote:
> Author: gregames
> Date: Mon Apr 30 11:16:06 2007
> New Revision: 533820
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=533820
> Log:
> check_pipeline: use AP_MODE_SPECULATIVE to check for data in the input filters
> to accomodate mod_ssl's input filter. AP_MODE_EATCRLF is essentially a no-op
> in that filter.
Whatever happened to AP_MODE_INIT which was added for just this purpose?
Re: svn commit: r533820 - /httpd/httpd/trunk/modules/http/http_request.c
Posted by Greg Ames <am...@yahoo.com>.
--- Ruediger Pluem <rp...@apache.org> wrote:
> >>check_pipeline: use AP_MODE_SPECULATIVE to check for data in the input
> filters
> >>to accomodate mod_ssl's input filter. AP_MODE_EATCRLF is essentially a
> no-op
> >>in that filter.
> >
> > EATCRLF was used here for a specific reason though - the fact that "many
> > browsers" (says ap_core_input_filter) send newlines after request data -
> > is that no longer the case?
>
> Does this really matter? AFAICT we are ignoring empty lines
> in front of a request up to a certain limit (around line 573 in protocol.c)
> anyway, so I guess we don't need to eat them at check_pipeline.
that is true.
I confirmed it by using a test case constructed to match Dean's description of
what the old browsers had to do to cope with the old CERN server, i.e, a POST
request with an extra blank line following the body. the request itself was
handled normally. then there is a 2nd call to ap_read_request and
read_request_line which executes the read loop that you pointed out, discards
the extra blank line, and then it breaks out of the loop due to rv 70014
(APR_EOF??) from ap_rgetline.
so we're good as far as I can tell, and our input filter path is cleaner. I
would like to wait for a while to insure nobody turns up unexpected bugs due to
my change, then remove AP_MODE_EATCRLF support which appears to be dead.
Greg
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
Re: svn commit: r533820 - /httpd/httpd/trunk/modules/http/http_request.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 01.05.2007 12:38, Joe Orton wrote:
> On Mon, Apr 30, 2007 at 06:16:07PM -0000, Greg Ames wrote:
>
>>Author: gregames
>>Date: Mon Apr 30 11:16:06 2007
>>New Revision: 533820
>>
>>URL: http://svn.apache.org/viewvc?view=rev&rev=533820
>>Log:
>>check_pipeline: use AP_MODE_SPECULATIVE to check for data in the input filters
>>to accomodate mod_ssl's input filter. AP_MODE_EATCRLF is essentially a no-op
>>in that filter.
>
>
> EATCRLF was used here for a specific reason though - the fact that "many
> browsers" (says ap_core_input_filter) send newlines after request data -
> is that no longer the case?
Does this really matter? AFAICT we are ignoring empty lines
in front of a request up to a certain limit (around line 573 in protocol.c)
anyway, so I guess we don't need to eat them at check_pipeline.
Regards
Rüdiger
Re: svn commit: r533820 - /httpd/httpd/trunk/modules/http/http_request.c
Posted by Greg Ames <am...@yahoo.com>.
> > check_pipeline: use AP_MODE_SPECULATIVE to check for data in the input
> filters
> > to accomodate mod_ssl's input filter. AP_MODE_EATCRLF is essentially a
> no-op
> > in that filter.
>
> EATCRLF was used here for a specific reason though - the fact that "many
> browsers" (says ap_core_input_filter) send newlines after request data -
> is that no longer the case?
EATCRLF started out life as AP_MODE_PEEK. its mission was to see if any more
input data was stashed in the pipeline in order to know if it is appropriate to
flush the output filters, without doing a read() syscall.
then someone added the side effect of eating the single CRLFs from old
browsers. an inappropriate design in my opinion. the filters should be lean
and mean and relatively dumb.
then someone else decided to rewrite the input filters and rename MODE_PEEK.
the result was that it got named after its side effect and redundant code was
added as MODE_SPECULATIVE which was used in mod_ssl.
my thoughts: don't try to optimize for old browsers within the core filters.
if they really need any special attention (I doubt it), do it at the protocol
level, above the filters.
Greg
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
Re: svn commit: r533820 - /httpd/httpd/trunk/modules/http/http_request.c
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Apr 30, 2007 at 06:16:07PM -0000, Greg Ames wrote:
> Author: gregames
> Date: Mon Apr 30 11:16:06 2007
> New Revision: 533820
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=533820
> Log:
> check_pipeline: use AP_MODE_SPECULATIVE to check for data in the input filters
> to accomodate mod_ssl's input filter. AP_MODE_EATCRLF is essentially a no-op
> in that filter.
EATCRLF was used here for a specific reason though - the fact that "many
browsers" (says ap_core_input_filter) send newlines after request data -
is that no longer the case?
joe
Re: svn commit: r533820 -
/httpd/httpd/trunk/modules/http/http_request.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
gregames@apache.org wrote:
> Author: gregames
> Date: Mon Apr 30 11:16:06 2007
> New Revision: 533820
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=533820
> Log:
> check_pipeline: use AP_MODE_SPECULATIVE to check for data in the input filters
> to accomodate mod_ssl's input filter. AP_MODE_EATCRLF is essentially a no-op
> in that filter.
Whatever happened to AP_MODE_INIT which was added for just this purpose?