httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jerenkra...@apache.org
Subject cvs commit: httpd-2.0/modules/http http_protocol.c
Date Mon, 17 Jun 2002 05:09:45 GMT
jerenkrantz    2002/06/16 22:09:45

  Modified:    .        CHANGES
               modules/http http_protocol.c
  Log:
  Rewrite ap_get_client_block to rely on assumptions that have been
  solidified after this code was originally written.  Namely:
  
  - AP_MODE_READBYTES will only return a brigade representing AT MOST bytes
    of data.  It can NOT return MORE than requested.
  - APR_BLOCK_READ is respected - it is considered a design error of a filter
    if it returns without reading something.
  - apr_brigade_flatten is available to do the heavy lifting of the copying
    into a flat buffer (as hinted at by the removed comment).
  
  Tested with httpd-test.
  
  Revision  Changes    Path
  1.837     +2 -1      httpd-2.0/CHANGES
  
  Index: CHANGES
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/CHANGES,v
  retrieving revision 1.836
  retrieving revision 1.837
  diff -u -r1.836 -r1.837
  --- CHANGES	16 Jun 2002 19:44:44 -0000	1.836
  +++ CHANGES	17 Jun 2002 05:09:45 -0000	1.837
  @@ -1,6 +1,7 @@
  -
   Changes with Apache 2.0.39
   
  +  *) Improve ap_get_client_block implementation by using APR-util helper
  +     functions.  [Justin Erenkrantz]
   
   Changes with Apache 2.0.38
   
  
  
  
  1.437     +27 -62    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.436
  retrieving revision 1.437
  diff -u -r1.436 -r1.437
  --- http_protocol.c	13 Jun 2002 20:30:09 -0000	1.436
  +++ http_protocol.c	17 Jun 2002 05:09:45 -0000	1.437
  @@ -1771,79 +1771,44 @@
    * to read past the data provided by the client, since these reads block.
    * Returns 0 on End-of-body, -1 on error or premature chunk end.
    *
  - * Reading the chunked encoding requires a buffer size large enough to
  - * hold a chunk-size line, including any extensions. For now, we'll leave
  - * that to the caller, at least until we can come up with a better solution.
    */
   AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer,
                                        apr_size_t bufsiz)
   {
  -    apr_size_t total;
       apr_status_t rv;
  -    apr_bucket *b;
  -    const char *tempbuf;
  -    core_request_config *req_cfg =
  -        (core_request_config *)ap_get_module_config(r->request_config,
  -                                                    &core_module);
  -    apr_bucket_brigade *bb = req_cfg->bb;
  +    apr_bucket_brigade *bb;
   
  -    /* read until we get a non-empty brigade */
  -    while (APR_BRIGADE_EMPTY(bb)) {
  -        if (ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES,
  -                           APR_BLOCK_READ, bufsiz) != APR_SUCCESS) {
  -            /* if we actually fail here, we want to just return and
  -             * stop trying to read data from the client.
  -             */
  -            r->connection->keepalive = -1;
  -            apr_brigade_destroy(bb);
  -            return -1;
  -        }
  -    }
  +    bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
   
  -    b = APR_BRIGADE_FIRST(bb);
  -    if (APR_BUCKET_IS_EOS(b)) { /* reached eos on previous invocation */
  -        apr_bucket_delete(b);
  -        return 0;
  +    rv = ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES,
  +                        APR_BLOCK_READ, bufsiz);
  +  
  +    /* We lose the failure code here.  This is why ap_get_client_block should
  +     * not be used.
  +     */
  +    if (rv != APR_SUCCESS) { 
  +        /* if we actually fail here, we want to just return and
  +         * stop trying to read data from the client.
  +         */
  +        r->connection->keepalive = -1;
  +        return -1;
       }
   
  -    /* ### 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)) {
  -        apr_size_t len_read;
  -        apr_bucket *old;
  -
  -        if ((rv = apr_bucket_read(b, &tempbuf, &len_read,
  -                                  APR_BLOCK_READ)) != APR_SUCCESS) {
  -            return -1;
  -        }
  -        if (total + len_read > bufsiz) {
  -            apr_bucket_split(b, bufsiz - total);
  -            len_read = bufsiz - total;
  -        }
  -        memcpy(buffer, tempbuf, len_read);
  -        buffer += len_read;
  -        total += len_read;
  -        /* XXX the next field shouldn't be mucked with here,
  -         * as it is in terms of bytes in the unfiltered body;
  -         * gotta see if anybody else actually uses it
  -         */
  -        r->read_length += len_read; /* XXX yank me? */
  -        old = b;
  -        b = APR_BUCKET_NEXT(b);
  -        apr_bucket_delete(old);
  +    /* If this fails, it means that a filter is written incorrectly and that
  +     * it needs to learn how to properly handle APR_BLOCK_READ requests by
  +     * returning data when requested.
  +     */
  +    AP_DEBUG_ASSERT(!APR_BRIGADE_EMPTY(bb));
  + 
  +    rv = apr_brigade_flatten(bb, buffer, &bufsiz);
  +    if (rv != APR_SUCCESS) {
  +        return -1;
       }
   
  -    return total;
  +    /* XXX yank me? */
  +    r->read_length += bufsiz;
  +
  +    return bufsiz;
   }
   
   /* In HTTP/1.1, any method can have a body.  However, most GET handlers
  
  
  

Mime
View raw message