On Thursday 03 January 2002 09:50 am, Bill Stoddard wrote:
> 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.
Most of the logic from the filter's perspective for reading requests and responses is
the
same though. In both cases, we must read a bunch of headers, doing folding if
appropriate, and then we read a bunch of body. In both cases, we must be able to
handle
chunking.
I would prefer to just create a new filter to do the limit request body stuff.
Ryan
>
> 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: <[EMAIL PROTECTED]>
> > To: <[EMAIL PROTECTED]>
> > 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;
> > > }
> > >
> > >
> > >
> > >
> > >
> >
>
>
>
--
______________________________________________________________
Ryan Bloom [EMAIL PROTECTED]
Covalent Technologies [EMAIL PROTECTED]
--------------------------------------------------------------
