From 94d01b65aa7ce7a9c06f7e65708be628f049cf30 Mon Sep 17 00:00:00 2001 From: Justin Erenkrantz Date: Fri, 25 Jan 2002 01:11:47 +0000 Subject: [PATCH] Change ap_get_brigade prototype to remove *readbytes in favor of readbytes. If you need the length, you should be using apr_brigade_length. This is much more consistent. Of all the places that call ap_get_brigade, only one (ap_http_filter) needs the length. This makes it now possible to pass constants down without assigning them to a temporary variable first. Also: - Change proxy_ftp to use EXHAUSTIVE mode (didn't catch its -1 before) - Fix buglet in mod_ssl that would cause it to return too much data in some circumstances git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@93014 13f79535-47bb-0310-9956-ffa450edef68 --- include/util_filter.h | 6 ++--- modules/echo/mod_echo.c | 3 +-- modules/experimental/mod_case_filter_in.c | 2 +- modules/experimental/mod_charset_lite.c | 2 +- modules/experimental/mod_ext_filter.c | 2 +- modules/http/http_protocol.c | 23 ++++++++++------ modules/http/http_request.c | 3 +-- modules/http/mod_core.h | 2 +- modules/proxy/proxy_ftp.c | 5 ++-- modules/proxy/proxy_http.c | 9 ++++--- modules/proxy/proxy_util.c | 3 +-- modules/ssl/ssl_engine_io.c | 17 ++++++------ server/core.c | 29 +++++++-------------- server/mpm/experimental/perchild/perchild.c | 5 ++-- server/mpm/perchild/perchild.c | 5 ++-- server/protocol.c | 4 +-- server/util_filter.c | 2 +- 17 files changed, 57 insertions(+), 65 deletions(-) diff --git a/include/util_filter.h b/include/util_filter.h index 2be1e60a73..f15c6deebc 100644 --- a/include/util_filter.h +++ b/include/util_filter.h @@ -86,7 +86,7 @@ extern "C" { * input filtering modes */ typedef enum { - /** The filter should return at most *readbytes data. */ + /** The filter should return at most readbytes data. */ AP_MODE_READBYTES, /** The filter should return at most one line of CRLF data. * (If a potential line is too long or no CRLF is found, the @@ -165,7 +165,7 @@ typedef struct ap_filter_t ap_filter_t; */ 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_read_type_e block, apr_off_t *readbytes); + ap_input_mode_t mode, apr_read_type_e block, apr_off_t readbytes); typedef union ap_filter_func { ap_out_filter_func out_func; @@ -290,7 +290,7 @@ AP_DECLARE(apr_status_t) ap_get_brigade(ap_filter_t *filter, apr_bucket_brigade *bucket, ap_input_mode_t mode, apr_read_type_e block, - apr_off_t *readbytes); + apr_off_t readbytes); /** * Pass the current bucket brigade down to the next filter on the filter diff --git a/modules/echo/mod_echo.c b/modules/echo/mod_echo.c index 7d28603389..53d8a29650 100644 --- a/modules/echo/mod_echo.c +++ b/modules/echo/mod_echo.c @@ -94,7 +94,6 @@ static int process_echo_connection(conn_rec *c) apr_bucket_brigade *bb; apr_bucket *b; apr_status_t rv; - apr_off_t zero = 0; EchoConfig *pConfig = ap_get_module_config(c->base_server->module_config, &echo_module); @@ -107,7 +106,7 @@ static int process_echo_connection(conn_rec *c) for ( ; ; ) { /* Get a single line of input from the client */ if ((rv = ap_get_brigade(c->input_filters, bb, AP_MODE_GETLINE, - APR_BLOCK_READ, &zero) != APR_SUCCESS || + APR_BLOCK_READ, 0) != APR_SUCCESS || APR_BRIGADE_EMPTY(bb))) { apr_brigade_destroy(bb); break; diff --git a/modules/experimental/mod_case_filter_in.c b/modules/experimental/mod_case_filter_in.c index 0ff01289fa..18b44dd721 100644 --- a/modules/experimental/mod_case_filter_in.c +++ b/modules/experimental/mod_case_filter_in.c @@ -102,7 +102,7 @@ static apr_status_t CaseFilterInFilter(ap_filter_t *f, apr_bucket_brigade *pbbOut, ap_input_mode_t eMode, apr_read_type_e eBlock, - apr_off_t *nBytes) + apr_off_t nBytes) { request_rec *r = f->r; CaseFilterInContext *pCtx; diff --git a/modules/experimental/mod_charset_lite.c b/modules/experimental/mod_charset_lite.c index 9b490d6d04..aad8bcb018 100644 --- a/modules/experimental/mod_charset_lite.c +++ b/modules/experimental/mod_charset_lite.c @@ -991,7 +991,7 @@ static void transfer_brigade(apr_bucket_brigade *in, apr_bucket_brigade *out) static int xlate_in_filter(ap_filter_t *f, apr_bucket_brigade *bb, ap_input_mode_t mode, apr_read_type_e block, - apr_off_t *readbytes) + apr_off_t readbytes) { apr_status_t rv; charset_req_t *reqinfo = ap_get_module_config(f->r->request_config, diff --git a/modules/experimental/mod_ext_filter.c b/modules/experimental/mod_ext_filter.c index f8565bf43e..ba5444bb31 100644 --- a/modules/experimental/mod_ext_filter.c +++ b/modules/experimental/mod_ext_filter.c @@ -750,7 +750,7 @@ static apr_status_t ef_output_filter(ap_filter_t *f, apr_bucket_brigade *bb) #if 0 static int ef_input_filter(ap_filter_t *f, apr_bucket_brigade *bb, ap_input_mode_t mode, apr_read_type_e block, - apr_off_t *readbytes) + apr_off_t readbytes) { apr_status_t rv; apr_bucket *b; diff --git a/modules/http/http_protocol.c b/modules/http/http_protocol.c index ce208c0170..e9a2b82d04 100644 --- a/modules/http/http_protocol.c +++ b/modules/http/http_protocol.c @@ -534,11 +534,12 @@ typedef struct http_filter_ctx { */ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode, apr_read_type_e block, - apr_off_t *readbytes) + apr_off_t readbytes) { apr_bucket *e; http_ctx_t *ctx = f->ctx; apr_status_t rv; + apr_off_t totalread; /* just get out of the way of things we don't want. */ if (mode != AP_MODE_READBYTES && mode != AP_MODE_GETLINE) { @@ -643,10 +644,10 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, /* Ensure that the caller can not go over our boundary point. */ if (ctx->state == BODY_LENGTH || ctx->state == BODY_CHUNK) { - if (ctx->remaining < *readbytes) { - *readbytes = ctx->remaining; + if (ctx->remaining < readbytes) { + readbytes = ctx->remaining; } - AP_DEBUG_ASSERT(*readbytes > 0); /* shouldn't be in getline mode */ + AP_DEBUG_ASSERT(readbytes > 0); } rv = ap_get_brigade(f->next, b, mode, block, readbytes); @@ -655,8 +656,15 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, return rv; } + /* How many bytes did we just read? */ + apr_brigade_length(b, 0, &totalread); + + /* If this happens, we have a bucket of unknown length. Die because + * it means our assumptions have changed. */ + AP_DEBUG_ASSERT(totalread > 0); + if (ctx->state != BODY_NONE) { - ctx->remaining -= *readbytes; + ctx->remaining -= totalread; } /* We have a limit in effect. */ @@ -664,7 +672,7 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, /* 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; + ctx->limit_used += totalread; 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 @@ -1479,9 +1487,8 @@ AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer, /* read until we get a non-empty brigade */ while (APR_BRIGADE_EMPTY(bb)) { - apr_off_t len_read = bufsiz; if (ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES, - APR_BLOCK_READ, &len_read) != APR_SUCCESS) { + 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. */ diff --git a/modules/http/http_request.c b/modules/http/http_request.c index cb4e44007a..244c346cf1 100644 --- a/modules/http/http_request.c +++ b/modules/http/http_request.c @@ -239,7 +239,6 @@ static void check_pipeline_flush(request_rec *r) ### 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_off_t zero = 0; /* Flush the filter contents if: * @@ -250,7 +249,7 @@ static void check_pipeline_flush(request_rec *r) /* ### is zero correct? that means "read one line" */ if (!r->connection->keepalive || ap_get_brigade(r->input_filters, bb, AP_MODE_EATCRLF, - APR_NONBLOCK_READ, &zero) != APR_SUCCESS) { + APR_NONBLOCK_READ, 0) != APR_SUCCESS) { apr_bucket *e = apr_bucket_flush_create(); /* We just send directly to the connection based filters. At diff --git a/modules/http/mod_core.h b/modules/http/mod_core.h index e594f3ffc9..9576b40180 100644 --- a/modules/http/mod_core.h +++ b/modules/http/mod_core.h @@ -75,7 +75,7 @@ extern "C" { */ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode, apr_read_type_e block, - apr_off_t *readbytes); + apr_off_t readbytes); char *ap_response_code_string(request_rec *r, int error_index); diff --git a/modules/proxy/proxy_ftp.c b/modules/proxy/proxy_ftp.c index 273f88b2ea..6d2c2b8a54 100644 --- a/modules/proxy/proxy_ftp.c +++ b/modules/proxy/proxy_ftp.c @@ -555,7 +555,6 @@ int ap_proxy_ftp_handler(request_rec *r, proxy_server_conf *conf, int i = 0, j, len, rc; int one = 1; char *size = NULL; - apr_off_t readbytes = -1; apr_socket_t *origin_sock; /* stuff for PASV mode */ @@ -1586,8 +1585,8 @@ int ap_proxy_ftp_handler(request_rec *r, proxy_server_conf *conf, "proxy: FTP: start body send"); /* read the body, pass it to the output filters */ - while (ap_get_brigade(remote->input_filters, bb, AP_MODE_READBYTES, - APR_BLOCK_READ, &readbytes) == APR_SUCCESS) { + while (ap_get_brigade(remote->input_filters, bb, AP_MODE_EXHAUSTIVE, + APR_BLOCK_READ, 0) == APR_SUCCESS) { if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) { ap_pass_brigade(r->output_filters, bb); break; diff --git a/modules/proxy/proxy_http.c b/modules/proxy/proxy_http.c index cde97c6693..4895e05332 100644 --- a/modules/proxy/proxy_http.c +++ b/modules/proxy/proxy_http.c @@ -840,18 +840,20 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, */ if ( (conf->error_override ==0) || r->status < 400 ) { /* read the body, pass it to the output filters */ - apr_off_t readbytes; apr_bucket *e; - readbytes = AP_IOBUFSIZE; while (ap_get_brigade(rp->input_filters, bb, AP_MODE_READBYTES, APR_BLOCK_READ, - &readbytes) == APR_SUCCESS) { + AP_IOBUFSIZE) == APR_SUCCESS) { #if DEBUGGING + { + apr_off_t readbytes; + apr_brigade_length(bb, 0, &readbytes); ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server, "proxy (PID %d): readbytes: %#x", getpid(), readbytes); + } #endif if (APR_BRIGADE_EMPTY(bb)) { break; @@ -868,7 +870,6 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, break; } apr_brigade_cleanup(bb); - readbytes = AP_IOBUFSIZE; } } ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server, diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index ce54085cbe..0c3a85e3e4 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -1014,12 +1014,11 @@ PROXY_DECLARE(apr_status_t) ap_proxy_string_read(conn_rec *c, apr_bucket_brigade /* loop through each brigade */ while (!found) { - apr_off_t zero = 0; /* get brigade from network one line at a time */ if (APR_SUCCESS != (rv = ap_get_brigade(c->input_filters, bb, AP_MODE_GETLINE, APR_BLOCK_READ, - &zero /* readline */))) { + 0))) { return rv; } /* loop through each bucket */ diff --git a/modules/ssl/ssl_engine_io.c b/modules/ssl/ssl_engine_io.c index 699ca30fc5..b2d1d26f02 100644 --- a/modules/ssl/ssl_engine_io.c +++ b/modules/ssl/ssl_engine_io.c @@ -354,7 +354,6 @@ static int bio_bucket_in_read(BIO *bio, char *in, int inl) { BIO_bucket_in_t *inbio = BIO_bucket_in_ptr(bio); int len = 0; - apr_off_t readbytes = inl; /* XXX: flush here only required for SSLv2; * OpenSSL calls BIO_flush() at the appropriate times for @@ -371,6 +370,7 @@ static int bio_bucket_in_read(BIO *bio, char *in, int inl) if ((len <= inl) || inbio->mode == AP_MODE_GETLINE) { return len; } + inl -= len; } while (1) { @@ -390,7 +390,8 @@ static int bio_bucket_in_read(BIO *bio, char *in, int inl) * GETLINE. */ inbio->rc = ap_get_brigade(inbio->f->next, inbio->bb, - AP_MODE_READBYTES, inbio->block, &readbytes); + AP_MODE_READBYTES, inbio->block, + inl); if ((inbio->rc != APR_SUCCESS) || APR_BRIGADE_EMPTY(inbio->bb)) { @@ -736,13 +737,12 @@ static apr_status_t ssl_io_filter_Input(ap_filter_t *f, apr_bucket_brigade *bb, ap_input_mode_t mode, apr_read_type_e block, - apr_off_t *readbytes) + apr_off_t readbytes) { apr_status_t status; ssl_io_input_ctx_t *ctx = f->ctx; apr_size_t len = sizeof(ctx->buffer); - apr_off_t bytes = *readbytes; int is_init = (mode == AP_MODE_INIT); /* XXX: we don't currently support anything other than these modes. */ @@ -774,9 +774,10 @@ static apr_status_t ssl_io_filter_Input(ap_filter_t *f, if (ctx->inbio.mode == AP_MODE_READBYTES || ctx->inbio.mode == AP_MODE_SPECULATIVE) { - /* Protected from truncation, bytes < MAX_SIZE_T */ - if (bytes < len) { - len = (apr_size_t)bytes; + /* Protected from truncation, readbytes < MAX_SIZE_T + * FIXME: No, it's *not* protected. -- jre */ + if (readbytes < len) { + len = (apr_size_t)readbytes; } status = ssl_io_input_read(ctx, ctx->buffer, &len); } @@ -798,8 +799,6 @@ static apr_status_t ssl_io_filter_Input(ap_filter_t *f, APR_BRIGADE_INSERT_TAIL(bb, bucket); } - *readbytes = len; - return APR_SUCCESS; } diff --git a/server/core.c b/server/core.c index d76dc77d66..013b5344e8 100644 --- a/server/core.c +++ b/server/core.c @@ -2975,7 +2975,7 @@ static int default_handler(request_rec *r) static int net_time_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode, apr_read_type_e block, - apr_off_t *readbytes) + apr_off_t readbytes) { int keptalive = f->c->keepalive == 1; apr_socket_t *csd = ap_get_module_config(f->c->conn_config, &core_module); @@ -3006,7 +3006,7 @@ static int net_time_filter(ap_filter_t *f, apr_bucket_brigade *b, static int core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode, apr_read_type_e block, - apr_off_t *readbytes) + apr_off_t readbytes) { apr_bucket *e; apr_status_t rv; @@ -3107,7 +3107,6 @@ static int core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, /* Force a recompute of the length and force a read-all */ apr_brigade_length(ctx->b, 1, &total); APR_BRIGADE_CONCAT(b, ctx->b); - *readbytes = total; /* We have read until the brigade was empty, so we know that we * must be EOS. */ e = apr_bucket_eos_create(); @@ -3120,25 +3119,24 @@ static int core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, apr_bucket *e; apr_bucket_brigade *newbb; - AP_DEBUG_ASSERT(*readbytes > 0); + AP_DEBUG_ASSERT(readbytes > 0); e = APR_BRIGADE_FIRST(ctx->b); rv = apr_bucket_read(e, &str, &len, block); if (APR_STATUS_IS_EAGAIN(rv)) { - *readbytes = 0; return APR_SUCCESS; } else if (rv != APR_SUCCESS) { return rv; } - /* We can only return at most what the user asked for. */ - if (len < *readbytes) { - *readbytes = len; + /* We can only return at most what we read. */ + if (len < readbytes) { + readbytes = len; } - apr_brigade_partition(ctx->b, *readbytes, &e); + apr_brigade_partition(ctx->b, readbytes, &e); /* Must do split before CONCAT */ newbb = apr_brigade_split(ctx->b, e); @@ -3161,7 +3159,6 @@ static int core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, APR_BRIGADE_CONCAT(ctx->b, newbb); apr_brigade_length(b, 1, &total); - *readbytes = total; return APR_SUCCESS; } @@ -3172,17 +3169,11 @@ static int core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, * empty). We do this by returning whatever we have read. This may * or may not be bogus, but is consistent (for now) with EOF logic. */ - if (APR_STATUS_IS_EAGAIN(rv) || rv == APR_SUCCESS) { - apr_off_t total; - - apr_brigade_length(b, 1, &total); - *readbytes = total; - } - else { - return rv; + if (APR_STATUS_IS_EAGAIN(rv)) { + rv = APR_SUCCESS; } - return APR_SUCCESS; + return rv; } /* Default filter. This filter should almost always be used. Its only job diff --git a/server/mpm/experimental/perchild/perchild.c b/server/mpm/experimental/perchild/perchild.c index ccce28aa5a..9edffedfa1 100644 --- a/server/mpm/experimental/perchild/perchild.c +++ b/server/mpm/experimental/perchild/perchild.c @@ -1510,7 +1510,6 @@ static int pass_request(request_rec *r) &mpm_perchild_module); char *foo; apr_size_t len; - apr_off_t zero = 0; apr_pool_userdata_get((void **)&foo, "PERCHILD_BUFFER", r->connection->pool); @@ -1551,7 +1550,7 @@ static int pass_request(request_rec *r) ### reading large chunks of data or something? */ while (ap_get_brigade(r->input_filters, bb, AP_MODE_GETLINE, - APR_NONBLOCK_READ, &zero) == APR_SUCCESS) { + APR_NONBLOCK_READ, 0) == APR_SUCCESS) { apr_bucket *e; APR_BRIGADE_FOREACH(e, bb) { const char *str; @@ -1658,7 +1657,7 @@ static int perchild_post_read(request_rec *r) static apr_status_t perchild_buffer(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode, apr_read_type_e block, - apr_off_t *readbytes) + apr_off_t readbytes) { apr_bucket *e; apr_status_t rv; diff --git a/server/mpm/perchild/perchild.c b/server/mpm/perchild/perchild.c index ccce28aa5a..9edffedfa1 100644 --- a/server/mpm/perchild/perchild.c +++ b/server/mpm/perchild/perchild.c @@ -1510,7 +1510,6 @@ static int pass_request(request_rec *r) &mpm_perchild_module); char *foo; apr_size_t len; - apr_off_t zero = 0; apr_pool_userdata_get((void **)&foo, "PERCHILD_BUFFER", r->connection->pool); @@ -1551,7 +1550,7 @@ static int pass_request(request_rec *r) ### reading large chunks of data or something? */ while (ap_get_brigade(r->input_filters, bb, AP_MODE_GETLINE, - APR_NONBLOCK_READ, &zero) == APR_SUCCESS) { + APR_NONBLOCK_READ, 0) == APR_SUCCESS) { apr_bucket *e; APR_BRIGADE_FOREACH(e, bb) { const char *str; @@ -1658,7 +1657,7 @@ static int perchild_post_read(request_rec *r) static apr_status_t perchild_buffer(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode, apr_read_type_e block, - apr_off_t *readbytes) + apr_off_t readbytes) { apr_bucket *e; apr_status_t rv; diff --git a/server/protocol.c b/server/protocol.c index c2099f34e3..a3facd2c82 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -218,7 +218,7 @@ AP_DECLARE(apr_status_t) ap_rgetline(char **s, apr_size_t n, b = apr_brigade_create(r->pool); rv = ap_get_brigade(r->input_filters, b, AP_MODE_GETLINE, - APR_BLOCK_READ, &bytes_read); + APR_BLOCK_READ, bytes_read); if (rv != APR_SUCCESS) { return rv; @@ -381,7 +381,7 @@ AP_DECLARE(apr_status_t) ap_rgetline(char **s, apr_size_t n, /* We only care about the first byte. */ bytes_read = 1; rv = ap_get_brigade(r->input_filters, b, AP_MODE_SPECULATIVE, - APR_BLOCK_READ, &bytes_read); + APR_BLOCK_READ, bytes_read); if (rv != APR_SUCCESS) { return rv; diff --git a/server/util_filter.c b/server/util_filter.c index 4af3e90092..0282cca8bc 100644 --- a/server/util_filter.c +++ b/server/util_filter.c @@ -357,7 +357,7 @@ AP_DECLARE(apr_status_t) ap_get_brigade(ap_filter_t *next, apr_bucket_brigade *bb, ap_input_mode_t mode, apr_read_type_e block, - apr_off_t *readbytes) + apr_off_t readbytes) { if (next) { return next->frec->filter_func.in_func(next, bb, mode, block, -- 2.50.1