httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Stoddard" <b...@wstoddard.com>
Subject Re: cvs commit: httpd-2.0/server core.c
Date Thu, 03 Jan 2002 17:50:50 GMT
Here is the problem with this patch... (or with proxy's use of HTTP_IN)...

ap_http_filter is called to read -responses- from the proxied server. This patch makes an
implicit assumption that HTTP_IN is only being used to read requests. So, we either need
to create a whole new filter stack for reading proxy responses or we need to keep all code
that is specific to either requests or responses out of HTTP_IN. I am leaning in the
direction of creating a new filter, PROXIED_RESPONSE_IN or something or the other.

Bill

> This patch breaks the proxy.  Specifically, anyone who uses ap_proxy_make_fake_req().
Get
> a seg fault in ap_get_limit_req_body because r->per_dir_config is NULL.  I'll spend
some
> time on this tomorrow unless someone wants to jump on it tonight.
>
> Bill
>
> ----- Original Message -----
> From: <jerenkrantz@apache.org>
> To: <httpd-2.0-cvs@apache.org>
> Sent: Wednesday, January 02, 2002 2:56 AM
> Subject: cvs commit: httpd-2.0/server core.c
>
>
> > jerenkrantz    02/01/01 23:56:25
> >
> >   Modified:    .        CHANGES
> >                include  http_core.h
> >                modules/http http_protocol.c
> >                server   core.c
> >   Log:
> >   Fix LimitRequestBody directive by moving the relevant code from
> >   ap_*_client_block to ap_http_filter (aka HTTP_IN).  This is the
> >   only appropriate place for limit checking to occur (otherwise,
> >   chunked input is not correctly limited).
> >
> >   Also changed the type of limit_req_body to apr_off_t to match the
> >   other types inside of HTTP_IN.  Also made the strtol call for
> >   limit_req_body a bit more robust.
> >
> >   Revision  Changes    Path
> >   1.499     +4 -0      httpd-2.0/CHANGES
> >
> >   Index: CHANGES
> >   ===================================================================
> >   RCS file: /home/cvs/httpd-2.0/CHANGES,v
> >   retrieving revision 1.498
> >   retrieving revision 1.499
> >   diff -u -r1.498 -r1.499
> >   --- CHANGES 31 Dec 2001 21:03:12 -0000 1.498
> >   +++ CHANGES 2 Jan 2002 07:56:24 -0000 1.499
> >   @@ -1,4 +1,8 @@
> >    Changes with Apache 2.0.30-dev
> >   +
> >   +  *) Fix LimitRequestBody directive by placing it in the HTTP
> >   +     filter.  [Justin Erenkrantz]
> >   +
> >      *) Fix mod_proxy seg fault when the proxied server returns
> >         an HTTP/0.9 response or a bogus status line.
> >         [Adam Sussman]
> >
> >
> >
> >   1.58      +3 -3      httpd-2.0/include/http_core.h
> >
> >   Index: http_core.h
> >   ===================================================================
> >   RCS file: /home/cvs/httpd-2.0/include/http_core.h,v
> >   retrieving revision 1.57
> >   retrieving revision 1.58
> >   diff -u -r1.57 -r1.58
> >   --- http_core.h 1 Jan 2002 20:36:18 -0000 1.57
> >   +++ http_core.h 2 Jan 2002 07:56:24 -0000 1.58
> >   @@ -234,9 +234,9 @@
> >     * Return the limit on bytes in request msg body
> >     * @param r The current request
> >     * @return the maximum number of bytes in the request msg body
> >   - * @deffunc unsigned long ap_get_limit_req_body(const request_rec *r)
> >   + * @deffunc apr_off_t ap_get_limit_req_body(const request_rec *r)
> >     */
> >   -AP_DECLARE(unsigned long) ap_get_limit_req_body(const request_rec *r);
> >   +AP_DECLARE(apr_off_t) ap_get_limit_req_body(const request_rec *r);
> >
> >    /**
> >     * Return the limit on bytes in XML request msg body
> >   @@ -471,7 +471,7 @@
> >    #ifdef RLIMIT_NPROC
> >        struct rlimit *limit_nproc;
> >    #endif
> >   -    unsigned long limit_req_body;  /* limit on bytes in request msg body */
> >   +    apr_off_t limit_req_body;      /* limit on bytes in request msg body */
> >        long limit_xml_body;           /* limit on bytes in XML request msg body
*/
> >
> >        /* logging options */
> >
> >
> >
> >   1.383     +33 -11    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.382
> >   retrieving revision 1.383
> >   diff -u -r1.382 -r1.383
> >   --- http_protocol.c 6 Dec 2001 02:57:19 -0000 1.382
> >   +++ http_protocol.c 2 Jan 2002 07:56:24 -0000 1.383
> >   @@ -510,6 +510,8 @@
> >
> >    typedef struct http_filter_ctx {
> >        apr_off_t remaining;
> >   +    apr_off_t limit;
> >   +    apr_off_t limit_used;
> >        enum {
> >            BODY_NONE,
> >            BODY_LENGTH,
> >   @@ -536,6 +538,9 @@
> >            const char *tenc, *lenp;
> >            f->ctx = ctx = apr_palloc(f->r->pool, sizeof(*ctx));
> >            ctx->state = BODY_NONE;
> >   +        ctx->remaining = 0;
> >   +        ctx->limit_used = 0;
> >   +        ctx->limit = ap_get_limit_req_body(f->r);
> >
> >            tenc = apr_table_get(f->r->headers_in, "Transfer-Encoding");
> >            lenp = apr_table_get(f->r->headers_in, "Content-Length");
> >   @@ -562,6 +567,18 @@
> >                    ctx->state = BODY_LENGTH;
> >                    ctx->remaining = atol(lenp);
> >                }
> >   +
> >   +            /* If we have a limit in effect and we know the C-L ahead of
> >   +             * time, stop it here if it is invalid.
> >   +             */
> >   +            if (ctx->limit && ctx->limit < ctx->remaining)
{
> >   +                ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|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, ctx->limit);
> >   +                ap_die(HTTP_REQUEST_ENTITY_TOO_LARGE, f->r);
> >   +                return APR_EGENERAL;
> >   +            }
> >            }
> >        }
> >
> >   @@ -620,6 +637,22 @@
> >            ctx->remaining -= *readbytes;
> >        }
> >
> >   +    /* We have a limit in effect. */
> >   +    if (ctx->limit) {
> >   +        /* FIXME: Note that we might get slightly confused on chunked inputs
> >   +         * as we'd need to compensate for the chunk lengths which may not
> >   +         * really count.  This seems to be up for interpretation.  */
> >   +        ctx->limit_used += *readbytes;
> >   +        if (ctx->limit < ctx->limit_used) {
> >   +            ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, f->r,
> >   +                          "Read content-length of %" APR_OFF_T_FMT
> >   +                          " is larger than the configured limit"
> >   +                          " of %" APR_OFF_T_FMT, ctx->limit_used, ctx->limit);
> >   +            ap_die(HTTP_REQUEST_ENTITY_TOO_LARGE, f->r);
> >   +            return APR_EGENERAL;
> >   +        }
> >   +    }
> >   +
> >        return APR_SUCCESS;
> >    }
> >
> >   @@ -1283,7 +1316,6 @@
> >    {
> >        const char *tenc = apr_table_get(r->headers_in, "Transfer-Encoding");
> >        const char *lenp = apr_table_get(r->headers_in, "Content-Length");
> >   -    apr_off_t max_body;
> >
> >        r->read_body = read_policy;
> >        r->read_chunked = 0;
> >   @@ -1322,16 +1354,6 @@
> >            && (r->read_chunked || (r->remaining > 0))) {
> >            ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r,
> >                          "%s with body is not allowed for %s", r->method, r->uri);
> >   -        return HTTP_REQUEST_ENTITY_TOO_LARGE;
> >   -    }
> >   -
> >   -    max_body = ap_get_limit_req_body(r);
> >   -    if (max_body && (r->remaining > max_body)) {
> >   -        /* XXX shouldn't we enforce this for chunked encoding too? */
> >   -        ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r,
> >   -                      "Request content-length of %s is larger than "
> >   -                      "the configured limit of %" APR_OFF_T_FMT, lenp,
> >   -                      max_body);
> >            return HTTP_REQUEST_ENTITY_TOO_LARGE;
> >        }
> >
> >
> >
> >
> >   1.126     +6 -2      httpd-2.0/server/core.c
> >
> >   Index: core.c
> >   ===================================================================
> >   RCS file: /home/cvs/httpd-2.0/server/core.c,v
> >   retrieving revision 1.125
> >   retrieving revision 1.126
> >   diff -u -r1.125 -r1.126
> >   --- core.c 2 Jan 2002 05:29:08 -0000 1.125
> >   +++ core.c 2 Jan 2002 07:56:25 -0000 1.126
> >   @@ -778,7 +778,7 @@
> >        return apr_psprintf(p, "%s://%s:%u%s", ap_http_method(r), host, port, uri);
> >    }
> >
> >   -AP_DECLARE(unsigned long) ap_get_limit_req_body(const request_rec *r)
> >   +AP_DECLARE(apr_off_t) ap_get_limit_req_body(const request_rec *r)
> >    {
> >        core_dir_config *d =
> >          (core_dir_config *)ap_get_module_config(r->per_dir_config, &core_module);
> >   @@ -2093,6 +2093,7 @@
> >    {
> >        core_dir_config *conf=conf_;
> >        const char *err = ap_check_cmd_context(cmd, NOT_IN_LIMIT);
> >   +    char *errp;
> >        if (err != NULL) {
> >            return err;
> >        }
> >   @@ -2101,7 +2102,10 @@
> >         *      Instead we have an idiotic define in httpd.h that prevents
> >         *      it from being used even when it is available. Sheesh.
> >         */
> >   -    conf->limit_req_body = (unsigned long)strtol(arg, (char **)NULL, 10);
> >   +    conf->limit_req_body = (apr_off_t)strtol(arg, &errp, 10);
> >   +    if (*errp != '\0') {
> >   +        return "LimitRequestBody requires a non-negative integer.";
> >   +    }
> >        return NULL;
> >    }
> >
> >
> >
> >
> >
>


Mime
View raw message