httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From gst...@apache.org
Subject cvs commit: httpd-2.0/server/mpm/perchild perchild.c
Date Sat, 05 May 2001 11:18:01 GMT
gstein      01/05/05 04:18:01

  Modified:    include  util_filter.h
               modules/tls mod_tls.c
               modules/experimental mod_charset_lite.c mod_ext_filter.c
               modules/http http_protocol.c http_request.c mod_core.h
               server   core.c protocol.c util_filter.c
               server/mpm/perchild perchild.c
  Log:
  Fix a bug in the input handling. ap_http_filter() was modifying *readbytes
  which corresponded to r->remaining (in ap_get_client_block). However,
  ap_get_client_block was *also* adjusting r->remaining. Net result was that
  PUT (and probably POST) was broken. (at least on large inputs)
  
  To fix it, I simply removed the indirection on "readbytes" for input
  filters. There is no reason for them to return data (the brigade length is
  the return length). This also simplifies a number of calls where people
  needed to do &zero just to pass zero.
  
  I also added a number of comments about operations and where things could be
  improved, or are (semi) broken.
  
  Revision  Changes    Path
  1.52      +2 -2      httpd-2.0/include/util_filter.h
  
  Index: util_filter.h
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/include/util_filter.h,v
  retrieving revision 1.51
  retrieving revision 1.52
  diff -u -u -r1.51 -r1.52
  --- util_filter.h	2001/04/23 17:28:58	1.51
  +++ util_filter.h	2001/05/05 11:17:53	1.52
  @@ -155,7 +155,7 @@
    */
   typedef apr_status_t (*ap_out_filter_func)(ap_filter_t *f, apr_bucket_brigade *b);
   typedef apr_status_t (*ap_in_filter_func)(ap_filter_t *f, apr_bucket_brigade *b, 
  -                                          ap_input_mode_t mode, apr_size_t *readbytes);
  +                                          ap_input_mode_t mode, apr_size_t readbytes);
   
   typedef union ap_filter_func {
       ap_out_filter_func out_func;
  @@ -276,7 +276,7 @@
    *                  a single line should be read.
    */
   AP_DECLARE(apr_status_t) ap_get_brigade(ap_filter_t *filter, apr_bucket_brigade *bucket,

  -                                        ap_input_mode_t mode, apr_size_t *readbytes);
  +                                        ap_input_mode_t mode, apr_size_t readbytes);
   
   /**
    * Pass the current bucket brigade down to the next filter on the filter
  
  
  
  1.12      +4 -5      httpd-2.0/modules/tls/mod_tls.c
  
  Index: mod_tls.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/tls/mod_tls.c,v
  retrieving revision 1.11
  retrieving revision 1.12
  diff -u -u -r1.11 -r1.12
  --- mod_tls.c	2001/04/30 11:23:51	1.11
  +++ mod_tls.c	2001/05/05 11:17:54	1.12
  @@ -186,7 +186,7 @@
       return APR_SUCCESS;
   }
   
  -static apr_status_t churn(TLSFilterCtx *pCtx,apr_read_type_e eReadType,apr_size_t *readbytes)
  +static apr_status_t churn(TLSFilterCtx *pCtx,apr_read_type_e eReadType,apr_size_t readbytes)
   {
       ap_input_mode_t eMode=eReadType == APR_BLOCK_READ ? AP_MODE_BLOCKING
         : AP_MODE_NONBLOCKING;
  @@ -283,7 +283,6 @@
   {
       TLSFilterCtx *pCtx=f->ctx;
       apr_bucket *pbktIn;
  -    apr_size_t zero = 0;
   
       APR_BRIGADE_FOREACH(pbktIn,pbbIn) {
   	const char *data;
  @@ -299,7 +298,7 @@
   		ret=churn_output(pCtx);
   		if(ret != APR_SUCCESS)
   		    return ret;
  -		ret=churn(pCtx,APR_NONBLOCK_READ,&zero);
  +		ret=churn(pCtx,APR_NONBLOCK_READ,0);
   		if(ret != APR_SUCCESS) {
   		    if(ret == APR_EOF)
   			return APR_SUCCESS;
  @@ -312,7 +311,7 @@
   
   	if(APR_BUCKET_IS_FLUSH(pbktIn)) {
   	    /* assume that churn will flush (or already has) if there's output */
  -	    ret=churn(pCtx,APR_NONBLOCK_READ,&zero);
  +	    ret=churn(pCtx,APR_NONBLOCK_READ,0);
   	    if(ret != APR_SUCCESS)
   		return ret;
   	    continue;
  @@ -334,7 +333,7 @@
   }
   
   static apr_status_t tls_in_filter(ap_filter_t *f,apr_bucket_brigade *pbbOut,
  -				  ap_input_mode_t eMode, apr_size_t *readbytes)
  +				  ap_input_mode_t eMode, apr_size_t readbytes)
   {
       TLSFilterCtx *pCtx=f->ctx;
       apr_read_type_e eReadType=eMode == AP_MODE_BLOCKING ? APR_BLOCK_READ :
  
  
  
  1.46      +1 -1      httpd-2.0/modules/experimental/mod_charset_lite.c
  
  Index: mod_charset_lite.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_charset_lite.c,v
  retrieving revision 1.45
  retrieving revision 1.46
  diff -u -u -r1.45 -r1.46
  --- mod_charset_lite.c	2001/04/22 22:19:28	1.45
  +++ mod_charset_lite.c	2001/05/05 11:17:55	1.46
  @@ -1005,7 +1005,7 @@
   }
   
   static int xlate_in_filter(ap_filter_t *f, apr_bucket_brigade *bb, 
  -                           ap_input_mode_t mode, apr_size_t *readbytes)
  +                           ap_input_mode_t mode, apr_size_t readbytes)
   {
       apr_status_t rv;
       charset_req_t *reqinfo = ap_get_module_config(f->r->request_config,
  
  
  
  1.15      +1 -1      httpd-2.0/modules/experimental/mod_ext_filter.c
  
  Index: mod_ext_filter.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_ext_filter.c,v
  retrieving revision 1.14
  retrieving revision 1.15
  diff -u -u -r1.14 -r1.15
  --- mod_ext_filter.c	2001/04/22 22:19:29	1.14
  +++ mod_ext_filter.c	2001/05/05 11:17:56	1.15
  @@ -749,7 +749,7 @@
   
   #if 0
   static int ef_input_filter(ap_filter_t *f, apr_bucket_brigade *bb, 
  -                           ap_input_mode_t mode, apr_size_t *readbytes)
  +                           ap_input_mode_t mode, apr_size_t readbytes)
   {
       apr_status_t rv;
       apr_bucket *b;
  
  
  
  1.318     +67 -18    httpd-2.0/modules/http/http_protocol.c
  
  Index: http_protocol.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
  retrieving revision 1.317
  retrieving revision 1.318
  diff -u -u -r1.317 -r1.318
  --- http_protocol.c	2001/04/22 22:19:29	1.317
  +++ http_protocol.c	2001/05/05 11:17:57	1.318
  @@ -414,13 +414,17 @@
   struct dechunk_ctx {
       apr_size_t chunk_size;
       apr_size_t bytes_delivered;
  -    enum {WANT_HDR /* must have value zero */, WANT_BODY, WANT_TRL} state;
  +    enum {
  +        WANT_HDR /* must have value zero */,
  +        WANT_BODY,
  +        WANT_TRL
  +    } state;
   };
   
   static long get_chunk_size(char *);
   
   apr_status_t ap_dechunk_filter(ap_filter_t *f, apr_bucket_brigade *bb,
  -                               ap_input_mode_t mode, apr_size_t *readbytes)
  +                               ap_input_mode_t mode, apr_size_t readbytes)
   {
       apr_status_t rv;
       struct dechunk_ctx *ctx = f->ctx;
  @@ -440,7 +444,8 @@
                */
               char line[30];
               
  -            if ((rv = ap_getline(line, sizeof(line), f->r, 0)) < 0) {
  +            if ((rv = ap_getline(line, sizeof(line), f->r,
  +                                 0 /* readline */)) < 0) {
                   return rv;
               }
               switch(ctx->state) {
  @@ -460,6 +465,7 @@
                       /* bad trailer */
                   }
                   if (ctx->chunk_size == 0) { /* we just finished the last chunk? */
  +                    /* ### woah... ap_http_filter() is doing this, too */
                       /* append eos bucket and get out */
                       b = apr_bucket_eos_create();
                       APR_BRIGADE_INSERT_TAIL(bb, b);
  @@ -475,12 +481,21 @@
   
       if (ctx->state == WANT_BODY) {
           /* Tell ap_http_filter() how many bytes to deliver. */
  -        apr_size_t readbytes = ctx->chunk_size - ctx->bytes_delivered;
  -        if ((rv = ap_get_brigade(f->next, bb, mode, &readbytes)) != APR_SUCCESS)
{
  +        apr_size_t chunk_bytes = ctx->chunk_size - ctx->bytes_delivered;
  +
  +        if ((rv = ap_get_brigade(f->next, bb, mode,
  +                                 chunk_bytes)) != APR_SUCCESS) {
               return rv;
           }
  -        /* Walk through the body, accounting for bytes, and removing an eos bucket if
  -         * ap_http_filter() delivered the entire chunk.
  +
  +        /* Walk through the body, accounting for bytes, and removing an eos
  +         * bucket if ap_http_filter() delivered the entire chunk.
  +         *
  +         * ### this shouldn't be necessary. 1) ap_http_filter shouldn't be
  +         * ### adding EOS buckets. 2) it shouldn't return more bytes than
  +         * ### we requested, therefore the total len can be found with a
  +         * ### simple call to apr_brigade_length(). no further munging
  +         * ### would be needed.
            */
           b = APR_BRIGADE_FIRST(bb);
           while (b != APR_BRIGADE_SENTINEL(bb) && !APR_BUCKET_IS_EOS(b)) {
  @@ -503,7 +518,7 @@
       apr_bucket_brigade *b;
   } http_ctx_t;
   
  -apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode,
apr_size_t *readbytes)
  +apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode,
apr_size_t readbytes)
   {
       apr_bucket *e;
       char *buff;
  @@ -560,8 +575,17 @@
               return rv;
           }
       }
  +
  +    /* readbytes == 0 is "read a single line". otherwise, read a block. */
  +    if (readbytes) {
  +
  +        /* ### the code below, which moves bytes from one brigade to the
  +           ### other is probably bogus. presuming the next filter down was
  +           ### working properly, it should not have returned more than
  +           ### READBYTES bytes, and we wouldn't have to do any work. further,
  +           ### we could probably just use brigade_partition() in here.
  +        */
   
  -    if (*readbytes) {
           while (!APR_BRIGADE_EMPTY(ctx->b)) {
               const char *ignore;
   
  @@ -580,12 +604,12 @@
                    * a time - don't assume that one call to apr_bucket_read()
                    * will return the full string.
                    */
  -                if (*readbytes < len) {
  -                    apr_bucket_split(e, *readbytes);
  -                    *readbytes = 0;
  +                if (readbytes < len) {
  +                    apr_bucket_split(e, readbytes);
  +                    readbytes = 0;
                   }
                   else {
  -                    *readbytes -= len;
  +                    readbytes -= len;
                   }
                   APR_BUCKET_REMOVE(e);
                   APR_BRIGADE_INSERT_TAIL(b, e);
  @@ -593,7 +617,18 @@
               }
               apr_bucket_delete(e);
           }
  -        if (*readbytes == 0) {
  +
  +        /* ### this is a hack. it is saying, "if we have read everything
  +           ### that was requested, then we are at the end of the request."
  +           ### it presumes that the next filter up will *only* call us
  +           ### with readbytes set to the Content-Length of the request.
  +           ### that may not always be true, and this code is *definitely*
  +           ### too presumptive of the caller's intent. the point is: this
  +           ### filter is not the guy that is parsing the headers or the
  +           ### chunks to determine where the end of the request is, so we
  +           ### shouldn't be monkeying with EOS buckets.
  +        */
  +        if (readbytes == 0) {
               apr_bucket *eos = apr_bucket_eos_create();
                   
               APR_BRIGADE_INSERT_TAIL(b, eos);
  @@ -601,6 +636,7 @@
           return APR_SUCCESS;
       }
   
  +    /* we are reading a single line, e.g. the HTTP headers */
       while (!APR_BRIGADE_EMPTY(ctx->b)) {
           e = APR_BRIGADE_FIRST(ctx->b);
           if ((rv = apr_bucket_read(e, (const char **)&buff, &len, mode)) != APR_SUCCESS)
{
  @@ -1366,7 +1402,8 @@
   
       do {
           if (APR_BRIGADE_EMPTY(bb)) {
  -            if (ap_get_brigade(r->input_filters, bb, AP_MODE_BLOCKING, &r->remaining)
!= APR_SUCCESS) {
  +            if (ap_get_brigade(r->input_filters, bb, AP_MODE_BLOCKING,
  +                               r->remaining) != APR_SUCCESS) {
                   /* if we actually fail here, we want to just return and
                    * stop trying to read data from the client.
                    */
  @@ -1375,16 +1412,28 @@
                   return -1;
               }
           }
  -        b = APR_BRIGADE_FIRST(bb);
       } while (APR_BRIGADE_EMPTY(bb));
   
  -    if (APR_BUCKET_IS_EOS(b)) {         /* reached eos on previous invocation */
  +    b = APR_BRIGADE_FIRST(bb);
  +    if (APR_BUCKET_IS_EOS(b)) { /* reached eos on previous invocation */
           apr_bucket_delete(b);
           return 0;
       }
   
  +    /* ### it would be nice to replace the code below with "consume N bytes
  +       ### from this brigade, placing them into that buffer." there are
  +       ### other places where we do the same...
  +       ###
  +       ### alternatively, we could partition the brigade, then call a
  +       ### function which serializes a given brigade into a buffer. that
  +       ### semantic is used elsewhere, too...
  +    */
  +
       total = 0;
  -    while (total < bufsiz &&  b != APR_BRIGADE_SENTINEL(bb) && !APR_BUCKET_IS_EOS(b))
{
  +    while (total < bufsiz
  +           && b != APR_BRIGADE_SENTINEL(bb)
  +           && !APR_BUCKET_IS_EOS(b)) {
  +
           if ((rv = apr_bucket_read(b, &tempbuf, &len_read, APR_BLOCK_READ)) != APR_SUCCESS)
{
               return -1;
           }
  
  
  
  1.96      +2 -2      httpd-2.0/modules/http/http_request.c
  
  Index: http_request.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/http/http_request.c,v
  retrieving revision 1.95
  retrieving revision 1.96
  diff -u -u -r1.95 -r1.96
  --- http_request.c	2001/04/22 22:19:30	1.95
  +++ http_request.c	2001/05/05 11:17:57	1.96
  @@ -367,7 +367,6 @@
          ### allow us to defer creation of the brigade to when we actually
          ### need to send a FLUSH. */
       apr_bucket_brigade *bb = apr_brigade_create(r->pool);
  -    apr_size_t zero = 0;
   
       /* Flush the filter contents if:
        *
  @@ -375,8 +374,9 @@
        *   2) there isn't a request ready to be read
        */
       /* ### shouldn't this read from the connection input filters? */
  +    /* ### is zero correct? that means "read one line" */
       if (!r->connection->keepalive || 
  -        ap_get_brigade(r->input_filters, bb, AP_MODE_PEEK, &zero) != APR_SUCCESS)
{
  +        ap_get_brigade(r->input_filters, bb, AP_MODE_PEEK, 0) != APR_SUCCESS) {
           apr_bucket *e = apr_bucket_flush_create();
   
           /* We just send directly to the connection based filters.  At
  
  
  
  1.11      +4 -2      httpd-2.0/modules/http/mod_core.h
  
  Index: mod_core.h
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/http/mod_core.h,v
  retrieving revision 1.10
  retrieving revision 1.11
  diff -u -u -r1.10 -r1.11
  --- mod_core.h	2001/04/22 22:19:30	1.10
  +++ mod_core.h	2001/05/05 11:17:57	1.11
  @@ -73,8 +73,10 @@
   /*
    * These (input) filters are internal to the mod_core operation.
    */
  -apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode,
apr_size_t *readbytes);
  -apr_status_t ap_dechunk_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode,
apr_size_t *readbytes);
  +apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
  +                            ap_input_mode_t mode, apr_size_t readbytes);
  +apr_status_t ap_dechunk_filter(ap_filter_t *f, apr_bucket_brigade *b,
  +                               ap_input_mode_t mode, apr_size_t readbytes);
   
   char *ap_response_code_string(request_rec *r, int error_index);
   
  
  
  
  1.15      +10 -2     httpd-2.0/server/core.c
  
  Index: core.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/server/core.c,v
  retrieving revision 1.14
  retrieving revision 1.15
  diff -u -u -r1.14 -r1.15
  --- core.c	2001/05/02 20:15:56	1.14
  +++ core.c	2001/05/05 11:17:59	1.15
  @@ -2995,10 +2995,18 @@
       return ap_pass_brigade(r->output_filters, bb);
   }
   
  -static int core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode,
apr_size_t *readbytes)
  +static int core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode,
apr_size_t readbytes)
   {
       apr_bucket *e;
  -    
  +
  +    /* ### we should obey readbytes. the policy is to not insert more than
  +       ### READBYTES into the brigade. the caller knows the amount that is
  +       ### proper for the protocol. reading more than that could cause
  +       ### problems.
  +       ### (of course, we can read them from the socket, we just should not
  +       ###  return them until asked)
  +    */
  +
       if (!f->ctx) {    /* If we haven't passed up the socket yet... */
           f->ctx = (void *)1;
           e = apr_bucket_socket_create(f->c->client_socket);
  
  
  
  1.18      +3 -2      httpd-2.0/server/protocol.c
  
  Index: protocol.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/server/protocol.c,v
  retrieving revision 1.17
  retrieving revision 1.18
  diff -u -u -r1.17 -r1.18
  --- protocol.c	2001/04/26 00:33:14	1.17
  +++ protocol.c	2001/05/05 11:17:59	1.18
  @@ -203,7 +203,6 @@
       int retval;
       int total = 0;
       int looking_ahead = 0;
  -    apr_size_t zero = 0;
       apr_size_t length;
       conn_rec *c = r->connection;
       core_request_config *req_cfg;
  @@ -218,7 +217,9 @@
   
       while (1) {
           if (APR_BRIGADE_EMPTY(b)) {
  -            if ((retval = ap_get_brigade(c->input_filters, b, AP_MODE_BLOCKING, &zero))
!= APR_SUCCESS ||
  +            if ((retval = ap_get_brigade(c->input_filters, b,
  +                                         AP_MODE_BLOCKING,
  +                                         0 /* readline */)) != APR_SUCCESS ||
                   APR_BRIGADE_EMPTY(b)) {
                   apr_brigade_destroy(b);
                   return -1;
  
  
  
  1.57      +4 -2      httpd-2.0/server/util_filter.c
  
  Index: util_filter.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/server/util_filter.c,v
  retrieving revision 1.56
  retrieving revision 1.57
  diff -u -u -r1.56 -r1.57
  --- util_filter.c	2001/04/22 22:19:32	1.56
  +++ util_filter.c	2001/05/05 11:17:59	1.57
  @@ -208,8 +208,10 @@
    * save data off to the side should probably create their own temporary
    * brigade especially for that use.
    */
  -AP_DECLARE(apr_status_t) ap_get_brigade(ap_filter_t *next, apr_bucket_brigade *bb, 
  -                                        ap_input_mode_t mode, apr_size_t *readbytes)
  +AP_DECLARE(apr_status_t) ap_get_brigade(ap_filter_t *next,
  +                                        apr_bucket_brigade *bb, 
  +                                        ap_input_mode_t mode,
  +                                        apr_size_t readbytes)
   {
       if (next) {
           return next->frec->filter_func.in_func(next, bb, mode, readbytes);
  
  
  
  1.64      +7 -4      httpd-2.0/server/mpm/perchild/perchild.c
  
  Index: perchild.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/server/mpm/perchild/perchild.c,v
  retrieving revision 1.63
  retrieving revision 1.64
  diff -u -u -r1.63 -r1.64
  --- perchild.c	2001/04/24 02:17:21	1.63
  +++ perchild.c	2001/05/05 11:18:01	1.64
  @@ -1362,7 +1362,6 @@
                                                    &mpm_perchild_module);
       char *foo;
       apr_size_t len;
  -    apr_size_t readbytes = 0;
   
       apr_pool_userdata_get((void **)&foo, "PERCHILD_BUFFER", r->connection->pool);
       len = strlen(foo);
  @@ -1397,8 +1396,12 @@
       }
   
       write(sconf->sd2, foo, len);
  -   
  -    while (ap_get_brigade(r->input_filters, bb, AP_MODE_NONBLOCKING, &readbytes)
== APR_SUCCESS) {
  +
  +    /* ### this "read one line" doesn't seem right... shouldn't we be
  +       ### reading large chunks of data or something?
  +    */
  +    while (ap_get_brigade(r->input_filters, bb, AP_MODE_NONBLOCKING,
  +                          0 /* read one line */) == APR_SUCCESS) {
           apr_bucket *e;
           APR_BRIGADE_FOREACH(e, bb) {
               const char *str;
  @@ -1492,7 +1495,7 @@
       return OK;
   }
   
  -static apr_status_t perchild_buffer(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t
mode, apr_size_t *readbytes)
  +static apr_status_t perchild_buffer(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t
mode, apr_size_t readbytes)
   {
       apr_bucket *e;
       apr_status_t rv;
  
  
  

Mime
View raw message