httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r647263 - in /httpd/httpd/trunk: ./ docs/manual/mod/ include/ modules/aaa/ modules/filters/ modules/http/ server/
Date Thu, 01 May 2008 20:42:41 GMT


On 04/11/2008 08:41 PM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Fri Apr 11 11:41:53 2008
> New Revision: 647263
> 
> URL: http://svn.apache.org/viewvc?rev=647263&view=rev
> Log:
> Move the KeptBodySize directive, kept_body filters and the
> ap_parse_request_body function out of the http module and into a
> new module called mod_request, reducing the size of the core.
> 
> Added:
>     httpd/httpd/trunk/docs/manual/mod/mod_request.xml   (with props)
>     httpd/httpd/trunk/include/mod_request.h   (with props)
>     httpd/httpd/trunk/modules/filters/mod_request.c   (with props)
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/docs/manual/mod/core.xml
>     httpd/httpd/trunk/docs/manual/mod/mod_include.xml
>     httpd/httpd/trunk/include/http_protocol.h
>     httpd/httpd/trunk/modules/aaa/mod_auth_form.c
>     httpd/httpd/trunk/modules/filters/config.m4
>     httpd/httpd/trunk/modules/http/http_core.c
>     httpd/httpd/trunk/modules/http/http_filters.c
>     httpd/httpd/trunk/modules/http/mod_core.h
>     httpd/httpd/trunk/server/request.c
> 

> Added: httpd/httpd/trunk/modules/filters/mod_request.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_request.c?rev=647263&view=auto
> ==============================================================================
> --- httpd/httpd/trunk/modules/filters/mod_request.c (added)
> +++ httpd/httpd/trunk/modules/filters/mod_request.c Fri Apr 11 11:41:53 2008

> +typedef struct keep_body_filter_ctx {
> +    apr_off_t remaining;
> +    apr_off_t keep_body;
> +} keep_body_ctx_t;
> +
> +/**
> + * This is the KEEP_BODY_INPUT filter for HTTP requests, for times when the
> + * body should be set aside for future use by other modules.
> + */
> +AP_DECLARE(apr_status_t) ap_keep_body_filter(ap_filter_t *f, apr_bucket_brigade *b,
> +                                             ap_input_mode_t mode, apr_read_type_e block,
> +                                             apr_off_t readbytes)
> +{
> +    apr_bucket *e;
> +    keep_body_ctx_t *ctx = f->ctx;
> +    apr_status_t rv;
> +    apr_bucket *bucket;
> +    apr_off_t len = 0;
> +
> +
> +    if (!ctx) {
> +        const char *lenp;
> +        char *endstr = NULL;
> +        request_dir_conf *dconf = ap_get_module_config(f->r->per_dir_config,
> +                                                       &request_module);
> +
> +        /* must we step out of the way? */
> +        if (!dconf->keep_body || f->r->kept_body) {
> +            ap_remove_input_filter(f);
> +            return ap_get_brigade(f->next, b, mode, block, readbytes);
> +        }
> +
> +        f->ctx = ctx = apr_pcalloc(f->r->pool, sizeof(*ctx));
> +        
> +        /* fail fast if the content length exceeds keep body */
> +        lenp = apr_table_get(f->r->headers_in, "Content-Length");
> +        if (lenp) {
> +
> +            /* Protects against over/underflow, non-digit chars in the
> +             * string (excluding leading space) (the endstr checks)
> +             * and a negative number. */
> +            if (apr_strtoff(&ctx->remaining, lenp, &endstr, 10)
> +                || endstr == lenp || *endstr || ctx->remaining < 0) {
> +
> +                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, f->r,
> +                              "Invalid Content-Length");
> +
> +                ap_remove_input_filter(f);
> +                return bail_out_on_error(b, f, HTTP_REQUEST_ENTITY_TOO_LARGE);
> +            }
> +
> +            /* If we have a limit in effect and we know the C-L ahead of
> +             * time, stop it here if it is invalid.
> +             */
> +            if (dconf->keep_body < ctx->remaining) {
> +                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, f->r,
> +                          "Requested content-length of %" APR_OFF_T_FMT
> +                          " is larger than the configured limit"
> +                          " of %" APR_OFF_T_FMT, ctx->remaining, dconf->keep_body);
> +                ap_remove_input_filter(f);
> +                return bail_out_on_error(b, f, HTTP_REQUEST_ENTITY_TOO_LARGE);
> +            }
> +
> +        }
> +
> +        f->r->kept_body = apr_brigade_create(f->r->pool, f->r->connection->bucket_alloc);
> +        ctx->remaining = dconf->keep_body;
> +
> +    }
> +
> +    /* get the brigade from upstream, and read it in to get its length */
> +    rv = ap_get_brigade(f->next, b, mode, block, readbytes);
> +    if (rv == APR_SUCCESS) {
> +        rv = apr_brigade_length(b, 1, &len);
> +    }
> +        
> +    /* does the length take us over the limit? */
> +    if (APR_SUCCESS == rv && len > ctx->remaining) {
> +        if (f->r->kept_body) {
> +            apr_brigade_cleanup(f->r->kept_body);
> +            f->r->kept_body = NULL;
> +        }
> +        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, f->r,
> +                      "Requested content-length of %" APR_OFF_T_FMT
> +                      " is larger than the configured limit"
> +                      " of %" APR_OFF_T_FMT, len, ctx->keep_body);
> +        return bail_out_on_error(b, f, HTTP_REQUEST_ENTITY_TOO_LARGE);
> +    }
> +    ctx->remaining -= len;
> +
> +    /* pass any errors downstream */
> +    if (rv != APR_SUCCESS) {
> +        if (f->r->kept_body) {
> +            apr_brigade_cleanup(f->r->kept_body);
> +            f->r->kept_body = NULL;
> +        }
> +        return rv;
> +    }
> +
> +    /* all is well, set aside the buckets */
> +    for (bucket = APR_BRIGADE_FIRST(b);
> +         bucket != APR_BRIGADE_SENTINEL(b);
> +         bucket = APR_BUCKET_NEXT(bucket))
> +    {
> +        apr_bucket_copy(bucket, &e);

What about transient buckets? Don't we need to set them aside?

> +        APR_BRIGADE_INSERT_TAIL(f->r->kept_body, e);
> +    }
> +
> +    return APR_SUCCESS;
> +}
> +
> +
> +typedef struct kept_body_filter_ctx {
> +    apr_off_t offset;
> +    apr_off_t remaining;
> +} kept_body_ctx_t;
> +
> +/**
> + * Initialisation of filter to handle a kept body on subrequests.
> + * 
> + * If a body is to be reinserted into a subrequest, any chunking will have
> + * been removed from the body during storage. We need to change the request
> + * from Transfer-Encoding: chunked to an explicit Content-Length.
> + */
> +static int kept_body_filter_init(ap_filter_t *f) {
> +    apr_off_t length = 0;
> +    request_rec *r = f->r;
> +    apr_bucket_brigade *kept_body = r->kept_body;
> +
> +    if (kept_body) {
> +        apr_table_unset(r->headers_in, "Transfer-Encoding");
> +        apr_brigade_length(kept_body, 1, &length);
> +        apr_table_set(r->headers_in, "Content-Length", apr_off_t_toa(r->pool,
length));
> +    }
> +
> +    return OK;
> +}
> +
> +/**
> + * Filter to handle a kept body on subrequests.
> + * 
> + * If a body has been previously kept by the request, and if a subrequest wants
> + * to re-insert the body into the request, this input filter makes it happen.
> + */
> +AP_DECLARE(apr_status_t) ap_kept_body_filter(ap_filter_t *f, apr_bucket_brigade *b,
> +                                             ap_input_mode_t mode, apr_read_type_e block,
> +                                             apr_off_t readbytes) {
> +    request_rec *r = f->r;
> +    apr_bucket_brigade *kept_body = r->kept_body;
> +    kept_body_ctx_t *ctx = f->ctx;
> +    apr_bucket *ec, *e2;
> +    apr_status_t rv;
> +
> +    /* just get out of the way of things we don't want. */
> +    if (!kept_body || (mode != AP_MODE_READBYTES && mode != AP_MODE_GETLINE))
{
> +        return ap_get_brigade(f->next, b, mode, block, readbytes);
> +    }
> +
> +    /* set up the context if it does not already exist */
> +    if (!ctx) {
> +        f->ctx = ctx = apr_palloc(f->r->pool, sizeof(*ctx));
> +        ctx->offset = 0;
> +        apr_brigade_length(kept_body, 1, &ctx->remaining);
> +    }
> +
> +    /* kept_body is finished, send next filter */
> +    if (ctx->remaining <= 0) {
> +        return ap_get_brigade(f->next, b, mode, block, readbytes);
> +    }
> +
> +    /* send all of the kept_body, but no more */
> +    if (readbytes > ctx->remaining) {
> +        readbytes = ctx->remaining;
> +    }
> +
> +    /* send part of the kept_body */
> +    if ((rv = apr_brigade_partition(kept_body, ctx->offset, &ec)) != APR_SUCCESS)
{
> +        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
> +                      "apr_brigade_partition() failed on kept_body at %" APR_OFF_T_FMT,
ctx->offset);
> +        return rv;
> +    }
> +    if ((rv = apr_brigade_partition(kept_body, ctx->offset + readbytes, &e2))
!= APR_SUCCESS) {
> +        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
> +                      "apr_brigade_partition() failed on kept_body at %" APR_OFF_T_FMT,
ctx->offset + readbytes);
> +        return rv;
> +    }
> + 
> +    do {
> +        apr_bucket *foo;
> +        const char *str;
> +        apr_size_t len;
> +
> +        if (apr_bucket_copy(ec, &foo) != APR_SUCCESS) {
> +            /* As above; this should not fail since the bucket has
> +             * a known length, but just to be sure, this takes
> +             * care of uncopyable buckets that do somehow manage
> +             * to slip through.  */
> +            /* XXX: check for failure? */
> +            apr_bucket_read(ec, &str, &len, APR_BLOCK_READ);
> +            apr_bucket_copy(ec, &foo);
> +        }
> +        APR_BRIGADE_INSERT_TAIL(b, foo);
> +        ec = APR_BUCKET_NEXT(ec);
> +    } while (ec != e2);
> +
> +    ctx->remaining -= readbytes;
> +    ctx->offset += readbytes;
> +    return APR_SUCCESS;

Why using ctx->offset at all and not just taking all data from the kept_body brigade until
readbytes,
copy it over to b and remove it afterwards from the kept_body brigade. This would save one
call
to apr_brigade_partition.


> +
> +}
> +
> +/* form parsing stuff */
> +typedef enum {
> +    FORM_NORMAL,
> +    FORM_AMP,
> +    FORM_NAME,
> +    FORM_VALUE,
> +    FORM_PERCENTA,
> +    FORM_PERCENTB,
> +    FORM_ABORT
> +} ap_form_type_t;
> +
> +/**
> + * Read the body and parse any form found, which must be of the
> + * type application/x-www-form-urlencoded.
> + *
> + * Name/value pairs are returned in an array, with the names as
> + * strings with a maximum length of HUGE_STRING_LEN, and the
> + * values as bucket brigades. This allows values to be arbitrarily
> + * large.
> + *
> + * All url-encoding is removed from both the names and the values
> + * on the fly. The names are interpreted as strings, while the
> + * values are interpreted as blocks of binary data, that may
> + * contain the 0 character.
> + *
> + * In order to ensure that resource limits are not exceeded, a
> + * maximum size must be provided. If the sum of the lengths of
> + * the names and the values exceed this size, this function
> + * will return HTTP_REQUEST_ENTITY_TOO_LARGE.
> + *
> + * An optional number of parameters can be provided, if the number
> + * of parameters provided exceeds this amount, this function will
> + * return HTTP_REQUEST_ENTITY_TOO_LARGE. If this value is negative,
> + * no limit is imposed, and the number of parameters is in turn
> + * constrained by the size parameter above.
> + * 
> + * This function honours any kept_body configuration, and the
> + * original raw request body will be saved to the kept_body brigade
> + * if so configured, just as ap_discard_request_body does.
> + * 
> + * NOTE: File upload is not yet supported, but can be without change
> + * to the function call.
> + */
> +AP_DECLARE(int) ap_parse_request_form(request_rec * r, apr_array_header_t ** ptr,
> +                                      apr_size_t num, apr_size_t size)
> +{
> +    request_dir_conf *dconf;
> +    apr_off_t left = 0;
> +    apr_bucket_brigade *bb = NULL, *kept_body = NULL;
> +    apr_bucket *e;
> +    int seen_eos = 0;
> +    char buffer[HUGE_STRING_LEN + 1];
> +    const char *ct;
> +    apr_size_t offset = 0;
> +    ap_form_type_t state = FORM_NAME, percent = FORM_NORMAL;
> +    ap_form_pair_t *pair = NULL;
> +    apr_array_header_t *pairs = apr_array_make(r->pool, 4, sizeof(ap_form_pair_t));
> +
> +    char hi = 0;
> +    char low = 0;
> +
> +    *ptr = pairs;
> +
> +    /* sanity check - we only support forms for now */
> +    ct = apr_table_get(r->headers_in, "Content-Type");
> +    if (!ct || strcmp("application/x-www-form-urlencoded", ct)) {
> +        return ap_discard_request_body(r);
> +    }
> +
> +    dconf = ap_get_module_config(r->per_dir_config,
> +                                     &request_module);
> +    if (dconf->keep_body > 0) {
> +        left = dconf->keep_body;
> +        kept_body = apr_brigade_create(r->pool, r->connection->bucket_alloc);
> +    }
> +
> +    bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
> +    do {
> +        apr_bucket *bucket = NULL, *last = NULL;
> +
> +        int rv = ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES,
> +                                APR_BLOCK_READ, HUGE_STRING_LEN);
> +        if (rv != APR_SUCCESS) {
> +            apr_brigade_destroy(bb);
> +            return (rv == AP_FILTER_ERROR) ? rv : HTTP_BAD_REQUEST;
> +        }
> +
> +        for (bucket = APR_BRIGADE_FIRST(bb);
> +             bucket != APR_BRIGADE_SENTINEL(bb);
> +             last = bucket, bucket = APR_BUCKET_NEXT(bucket)) {
> +            const char *data;
> +            apr_size_t len, slide;
> +
> +            if (last) {
> +                apr_bucket_delete(last);
> +            }
> +            if (APR_BUCKET_IS_EOS(bucket)) {
> +                seen_eos = 1;
> +                break;
> +            }
> +            if (bucket->length == 0) {
> +                continue;
> +            }
> +
> +            rv = apr_bucket_read(bucket, &data, &len, APR_BLOCK_READ);
> +            if (rv != APR_SUCCESS) {
> +                apr_brigade_destroy(bb);
> +                return HTTP_BAD_REQUEST;
> +            }
> +
> +            slide = len;
> +            while (state != FORM_ABORT && slide-- > 0 && size >=
0 && num != 0) {
> +                char c = *data++;
> +                if ('+' == c) {
> +                    c = ' ';
> +                }
> +                else if ('&' == c) {
> +                    state = FORM_AMP;
> +                }
> +                if ('%' == c) {
> +                    percent = FORM_PERCENTA;
> +                    continue;
> +                }
> +                if (FORM_PERCENTA == percent) {
> +                    if (c >= 'a') {
> +                        hi = c - 'a' + 10;
> +                    }
> +                    else if (c >= 'A') {
> +                        hi = c - 'A' + 10;
> +                    }
> +                    else if (c >= '0') {
> +                        hi = c - '0';
> +                    }
> +                    hi = hi << 4;
> +                    percent = FORM_PERCENTB;
> +                    continue;
> +                }
> +                if (FORM_PERCENTB == percent) {
> +                    if (c >= 'a') {
> +                        low = c - 'a' + 10;
> +                    }
> +                    else if (c >= 'A') {
> +                        low = c - 'A' + 10;
> +                    }
> +                    else if (c >= '0') {
> +                        low = c - '0';
> +                    }
> +                    c = low ^ hi;

Shouldn't this be c = low + hi ?

> +                    percent = FORM_NORMAL;
> +                }
> +                switch (state) {
> +                case FORM_AMP:
> +                    if (pair) {
> +                        const char *tmp = apr_pmemdup(r->pool, buffer, offset);
> +                        apr_bucket *b = apr_bucket_pool_create(tmp, offset, r->pool,
r->connection->bucket_alloc);
> +                        APR_BRIGADE_INSERT_TAIL(pair->value, b);
> +                    }
> +                    state = FORM_NAME;
> +                    pair = NULL;
> +                    offset = 0;
> +                    num--;
> +                    break;
> +                case FORM_NAME:
> +                    if (offset < HUGE_STRING_LEN) {
> +                        if ('=' == c) {
> +                            buffer[offset] = 0;
> +                            offset = 0;
> +                            pair = (ap_form_pair_t *) apr_array_push(pairs);
> +                            pair->name = apr_pstrdup(r->pool, buffer);
> +                            pair->value = apr_brigade_create(r->pool, r->connection->bucket_alloc);
> +                            state = FORM_VALUE;
> +                        }
> +                        else {
> +                            buffer[offset++] = c;
> +                            size--;
> +                        }
> +                    }
> +                    else {
> +                        state = FORM_ABORT;
> +                    }
> +                    break;
> +                case FORM_VALUE:
> +                    if (offset >= HUGE_STRING_LEN) {
> +                        const char *tmp = apr_pmemdup(r->pool, buffer, offset);
> +                        apr_bucket *b = apr_bucket_pool_create(tmp, offset, r->pool,
r->connection->bucket_alloc);
> +                        APR_BRIGADE_INSERT_TAIL(pair->value, b);
> +                        offset = 0;
> +                    }
> +                    buffer[offset++] = c;
> +                    size--;
> +                    break;
> +                default:
> +                    break;
> +                }
> +            }
> +
> +            /* If we have been asked to, keep the data up until the
> +             * configured limit. If the limit is exceeded, we return an
> +             * HTTP_REQUEST_ENTITY_TOO_LARGE response so the caller is
> +             * clear the server couldn't handle their request.
> +             */
> +            if (kept_body) {
> +                if (len <= left) {
> +                    apr_bucket_copy(bucket, &e);
> +                    APR_BRIGADE_INSERT_TAIL(kept_body, e);
> +                    left -= len;
> +                }
> +                else {
> +                    apr_brigade_destroy(bb);
> +                    apr_brigade_destroy(kept_body);
> +                    return HTTP_REQUEST_ENTITY_TOO_LARGE;
> +                }

Why is this needed? Should this job be performed by the ap_keep_body_filter that should
be in our input filter chain if we want to keep the body?
Of course this depends when we call ap_parse_request_form. If we call it during the
authn/z phase the filter chain hasn't been setup. So maybe we should ensure that
this is the case.

> +            }
> +
> +        }
> +
> +        apr_brigade_cleanup(bb);
> +    } while (!seen_eos);
> +
> +    if (FORM_ABORT == state || size < 0 || num == 0) {
> +        return HTTP_REQUEST_ENTITY_TOO_LARGE;
> +    }
> +    else if (FORM_VALUE == state && pair && offset > 0) {
> +        const char *tmp = apr_pmemdup(r->pool, buffer, offset);
> +        apr_bucket *b = apr_bucket_pool_create(tmp, offset, r->pool, r->connection->bucket_alloc);
> +        APR_BRIGADE_INSERT_TAIL(pair->value, b);
> +    }
> +
> +    if (kept_body) {
> +        r->kept_body = kept_body;
> +    }
> +
> +    return OK;
> +
> +}
> +
> +/**
> + * Fixups hook.
> + * 
> + * Add the KEEP_BODY filter to the request, if the admin wants to keep
> + * the body using the KeptBodySize directive.
> + * 
> + * @param r The request
> + */
> +static int request_fixups(request_rec * r)
> +{
> +    request_dir_conf *conf = ap_get_module_config(r->per_dir_config,
> +                                                  &request_module);
> +
> +    if (conf->keep_body) {
> +        ap_add_input_filter_handle(ap_keep_body_input_filter_handle,
> +                                   NULL, r, r->connection);
> +    }
> +
> +    return OK;

Why not using the insert_filter hook?

> +
> +}
> +


> Modified: httpd/httpd/trunk/server/request.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/request.c?rev=647263&r1=647262&r2=647263&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/request.c (original)
> +++ httpd/httpd/trunk/server/request.c Fri Apr 11 11:41:53 2008
> @@ -45,6 +45,7 @@
>  #include "util_charset.h"
>  #include "util_script.h"
>  #include "ap_expr.h"
> +#include "mod_request.h"
>  
>  #include "mod_core.h"
>  
> @@ -1648,8 +1649,8 @@
>       * Add the KEPT_BODY filter, which will insert any body marked to be
>       * kept for the use of a subrequest, into the subrequest.
>       */
> -    ap_add_input_filter_handle(ap_kept_body_input_filter_handle,
> -                               NULL, rnew, rnew->connection);
> +    ap_add_input_filter(KEPT_BODY_FILTER,
> +                        NULL, rnew, rnew->connection);
>  

This creates an error message on each subrequest if mod_request is not loaded, because
in this case the KEPT_BODY_FILTER is not registered.

Regards

RĂ¼diger


Mime
View raw message