At the hack-athon we decided to change the way that input filters

determine how much data is returned to the previous filter.  Prior to this
change, we used a field in the conn_rec to determine how much to return.
After this change, we use an argument to ap_get_brigade.  This makes it
much more obvious how things work at all levels, so that module authors
can easily determine how much data is supposed to be returned to them.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@88912 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 6f780d4b3539f2115e3e9466587b2b878f764481..e338adb78a48ec0213c204aa1330d01c39e3ffa9 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,4 +1,10 @@
 Changes with Apache 2.0.18-dev
+
+  *) Change how input filters decide how much data is returned to the
+     higher filter.  We used to use a field in the conn_rec, with this
+     change, we use an argument to ap_get_brigade to determine how much
+     data is retrieved. [Ryan Bloom]
+
   *) Fix seg fault at start-up introduced by Ryan's change to enable
      modules to specify their own logging tags. mod_log_config
      registers an optional function, ap_register_log_handler().

diff --git a/include/httpd.h b/include/httpd.h
index d1f00af5d8d8f0df52018b6c579bab007e7bbfa2..27423fc17acb25797b99adc8cb2ab93582d004be 100644 (file)
--- a/include/httpd.h
+++ b/include/httpd.h
@@ -697,7 +697,7 @@ struct request_rec {
     apr_off_t clength;
 
     /** bytes left to read */
-    long remaining;
+    apr_size_t remaining;
     /** bytes that have been read */
     long read_length;
     /** how the request body should be read */
@@ -887,9 +887,6 @@ struct conn_rec {
     /** A list of output filters to be used for this connection
      *  @defvar ap_filter_t *filters */
     struct ap_filter_t *output_filters;
-    /** The length of the current request body
-     *  @defvar long remain */
-    long remain;
 };
 
 /* Per-vhost config... */

diff --git a/include/util_filter.h b/include/util_filter.h
index 4784f8af978d0ee963de524957b5f12d5b101196..1b9e2490efd9707bf3e68efcef0286c405d5f830 100644 (file)
--- a/include/util_filter.h
+++ b/include/util_filter.h
@@ -155,7 +155,8 @@ 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);
+                                          ap_input_mode_t mode, apr_size_t *readbytes);
+
 typedef union ap_filter_func {
     ap_out_filter_func out_func;
     ap_in_filter_func in_func;
@@ -273,7 +274,7 @@ struct ap_filter_t {
  * @param mode   ::AP_MODE_BLOCKING, ::AP_MODE_NONBLOCKING, or ::AP_MODE_PEEK
  */
 AP_DECLARE(apr_status_t) ap_get_brigade(ap_filter_t *filter, apr_bucket_brigade *bucket, 
-                                        ap_input_mode_t mode);
+                                        ap_input_mode_t mode, apr_size_t *readbytes);
 
 /**
  * Pass the current bucket brigade down to the next filter on the filter

diff --git a/modules/experimental/mod_charset_lite.c b/modules/experimental/mod_charset_lite.c
index 8cadc729cf02a04b65fe6815f804bf8c6e67f240..79830eed08da375cae5011a9cfea1c3eac8b4536 100644 (file)
--- a/modules/experimental/mod_charset_lite.c
+++ b/modules/experimental/mod_charset_lite.c
@@ -1005,7 +1005,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)
+                           ap_input_mode_t mode, apr_size_t *readbytes)
 {
     apr_status_t rv;
     charset_req_t *reqinfo = ap_get_module_config(f->r->request_config,
@@ -1049,11 +1049,11 @@ static int xlate_in_filter(ap_filter_t *f, apr_bucket_brigade *bb,
     }
 
     if (ctx->noop) {
-        return ap_get_brigade(f->next, bb, mode);
+        return ap_get_brigade(f->next, bb, mode, readbytes);
     }
 
     if (APR_BRIGADE_EMPTY(ctx->bb)) {
-        if ((rv = ap_get_brigade(f->next, bb, mode)) != APR_SUCCESS) {
+        if ((rv = ap_get_brigade(f->next, bb, mode, readbytes)) != APR_SUCCESS) {
             return rv;
         }
     }

diff --git a/modules/experimental/mod_ext_filter.c b/modules/experimental/mod_ext_filter.c
index defc85492c76cb0fc4352fbb94bd300e27d39f47..46b4b1e5273df102727481e201163f58fff9e7b2 100644 (file)
--- a/modules/experimental/mod_ext_filter.c
+++ b/modules/experimental/mod_ext_filter.c
@@ -749,7 +749,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)
+                           ap_input_mode_t mode, apr_size_t *readbytes)
 {
     apr_status_t rv;
     apr_bucket *b;
@@ -757,7 +757,7 @@ static int ef_input_filter(ap_filter_t *f, apr_bucket_brigade *bb,
     apr_ssize_t len;
     char *zero;
 
-    rv = ap_get_brigade(f->next, bb, mode);
+    rv = ap_get_brigade(f->next, bb, mode, readbytes);
     if (rv != APR_SUCCESS) {
         return rv;
     }

diff --git a/modules/http/http_protocol.c b/modules/http/http_protocol.c
index 6e1298a9da2a514df3adff111a2d8cff5e77a7c4..a4426466be8fcbefadef9aedd34f909b18559dca 100644 (file)
--- a/modules/http/http_protocol.c
+++ b/modules/http/http_protocol.c
@@ -420,7 +420,7 @@ struct dechunk_ctx {
 static long get_chunk_size(char *);
 
 apr_status_t ap_dechunk_filter(ap_filter_t *f, apr_bucket_brigade *bb,
-                               ap_input_mode_t mode)
+                               ap_input_mode_t mode, apr_size_t *readbytes)
 {
     apr_status_t rv;
     struct dechunk_ctx *ctx = f->ctx;
@@ -475,8 +475,8 @@ apr_status_t ap_dechunk_filter(ap_filter_t *f, apr_bucket_brigade *bb,
 
     if (ctx->state == WANT_BODY) {
         /* Tell ap_http_filter() how many bytes to deliver. */
-        f->c->remain = ctx->chunk_size - ctx->bytes_delivered;
-        if ((rv = ap_get_brigade(f->next, bb, mode)) != APR_SUCCESS) {
+        apr_size_t readbytes = ctx->chunk_size - ctx->bytes_delivered;
+        if ((rv = ap_get_brigade(f->next, bb, mode, &readbytes)) != APR_SUCCESS) {
             return rv;
         }
         /* Walk through the body, accounting for bytes, and removing an eos bucket if
@@ -503,7 +503,7 @@ typedef struct http_filter_ctx {
     apr_bucket_brigade *b;
 } http_ctx_t;
 
-apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode)
+apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode, apr_size_t *readbytes)
 {
     apr_bucket *e;
     char *buff;
@@ -556,12 +556,12 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode
     }
 
     if (APR_BRIGADE_EMPTY(ctx->b)) {
-        if ((rv = ap_get_brigade(f->next, ctx->b, mode)) != APR_SUCCESS) {
+        if ((rv = ap_get_brigade(f->next, ctx->b, mode, readbytes)) != APR_SUCCESS) {
             return rv;
         }
     }
 
-    if (f->c->remain) {
+    if (*readbytes) {
         while (!APR_BRIGADE_EMPTY(ctx->b)) {
             const char *ignore;
 
@@ -580,12 +580,12 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode
                  * a time - don't assume that one call to apr_bucket_read()
                  * will return the full string.
                  */
-                if (f->c->remain < len) {
-                    apr_bucket_split(e, f->c->remain);
-                    f->c->remain = 0;
+                if (*readbytes < len) {
+                    apr_bucket_split(e, *readbytes);
+                    *readbytes = 0;
                 }
                 else {
-                    f->c->remain -= len;
+                    *readbytes -= len;
                 }
                 APR_BUCKET_REMOVE(e);
                 APR_BRIGADE_INSERT_TAIL(b, e);
@@ -593,7 +593,7 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode
             }
             apr_bucket_delete(e);
         }
-        if (f->c->remain == 0) {
+        if (*readbytes == 0) {
             apr_bucket *eos = apr_bucket_eos_create();
                 
             APR_BRIGADE_INSERT_TAIL(b, eos);
@@ -1258,7 +1258,7 @@ AP_DECLARE(int) ap_setup_client_block(request_rec *r, int read_policy)
             return HTTP_BAD_REQUEST;
         }
 
-        r->connection->remain = r->remaining = atol(lenp);
+        r->remaining = atol(lenp);
     }
 
     if ((r->read_body == REQUEST_NO_BODY) &&
@@ -1366,7 +1366,7 @@ AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer, int bufsiz)
 
     do {
         if (APR_BRIGADE_EMPTY(bb)) {
-            if (ap_get_brigade(r->input_filters, bb, AP_MODE_BLOCKING) != APR_SUCCESS) {
+            if (ap_get_brigade(r->input_filters, bb, AP_MODE_BLOCKING, &r->remaining) != 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 17b0de9d0ab3aeb2a6c3d11b10a9e0737c094660..56064c451bdb0203099945de8901b031db13f460 100644 (file)
--- a/modules/http/http_request.c
+++ b/modules/http/http_request.c
@@ -367,6 +367,7 @@ 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_size_t zero = 0;
 
     /* Flush the filter contents if:
      *
@@ -375,7 +376,7 @@ static void check_pipeline_flush(request_rec *r)
      */
     /* ### shouldn't this read from the connection input filters? */
     if (!r->connection->keepalive || 
-        ap_get_brigade(r->input_filters, bb, AP_MODE_PEEK) != APR_SUCCESS) {
+        ap_get_brigade(r->input_filters, bb, AP_MODE_PEEK, &zero) != 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 e9ff827ed1ad96f2bce8e829ce14d14f02e1d69c..29fef07b650ea629e1ab62339410a07972eb26e1 100644 (file)
--- a/modules/http/mod_core.h
+++ b/modules/http/mod_core.h
@@ -73,8 +73,8 @@ extern "C" {
 /*
  * These (input) filters are internal to the mod_core operation.
  */
-apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode);
-apr_status_t ap_dechunk_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode);
+apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode, apr_size_t *readbytes);
+apr_status_t ap_dechunk_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode, apr_size_t *readbytes);
 
 char *ap_response_code_string(request_rec *r, int error_index);
 

diff --git a/modules/tls/mod_tls.c b/modules/tls/mod_tls.c
index fda42408543c08b304f312598bdfd9a923bdfc86..f8f78302ffc144589c4b920b37722c8337eeebd3 100644 (file)
--- a/modules/tls/mod_tls.c
+++ b/modules/tls/mod_tls.c
@@ -186,14 +186,14 @@ static apr_status_t churn_output(TLSFilterCtx *pCtx)
     return APR_SUCCESS;
 }
 
-static apr_status_t churn(TLSFilterCtx *pCtx,apr_read_type_e eReadType)
+static apr_status_t churn(TLSFilterCtx *pCtx,apr_read_type_e eReadType,apr_size_t *readbytes)
 {
     ap_input_mode_t eMode=eReadType == APR_BLOCK_READ ? AP_MODE_BLOCKING
       : AP_MODE_NONBLOCKING;
     apr_bucket *pbktIn;
 
     if(APR_BRIGADE_EMPTY(pCtx->pbbInput)) {
-       ap_get_brigade(pCtx->pInputFilter->next,pCtx->pbbInput,eMode);
+       ap_get_brigade(pCtx->pInputFilter->next,pCtx->pbbInput,eMode,readbytes);
        if(APR_BRIGADE_EMPTY(pCtx->pbbInput))
            return APR_EOF;
     }
@@ -329,7 +329,7 @@ static apr_status_t tls_out_filter(ap_filter_t *f,apr_bucket_brigade *pbbIn)
 }
 
 static apr_status_t tls_in_filter(ap_filter_t *f,apr_bucket_brigade *pbbOut,
-                                 ap_input_mode_t eMode)
+                                 ap_input_mode_t eMode, apr_size_t *readbytes)
 {
     TLSFilterCtx *pCtx=f->ctx;
     apr_read_type_e eReadType=eMode == AP_MODE_BLOCKING ? APR_BLOCK_READ :
@@ -340,7 +340,7 @@ static apr_status_t tls_in_filter(ap_filter_t *f,apr_bucket_brigade *pbbOut,
     assert(eMode != AP_MODE_PEEK);
 
     // churn the state machine
-    ret=churn(pCtx,eReadType);
+    ret=churn(pCtx,eReadType,readbytes);
     if(ret != APR_SUCCESS)
        return ret;
 

diff --git a/server/core.c b/server/core.c
index 7dded2f8b7a1b0cfc0d31401363f79ff00c92ff5..355b55ed7cf7cfd187396ca1711ec043141e8d97 100644 (file)
--- a/server/core.c
+++ b/server/core.c
@@ -2995,7 +2995,7 @@ static int default_handler(request_rec *r)
     return ap_pass_brigade(r->output_filters, bb);
 }
 
-static int core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode)
+static int core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode, apr_size_t *readbytes)
 {
     apr_bucket *e;
     

diff --git a/server/protocol.c b/server/protocol.c
index 45395faabef96b572ec03601cdea83cbce0d1a0b..8b14fb7d31053682afa7ae5d118fa429f40fbfb8 100644 (file)
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -203,6 +203,7 @@ AP_CORE_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold)
     int retval;
     int total = 0;
     int looking_ahead = 0;
+    apr_size_t zero = 0;
     apr_size_t length;
     conn_rec *c = r->connection;
     core_request_config *req_cfg;
@@ -217,7 +218,7 @@ AP_CORE_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold)
 
     while (1) {
         if (APR_BRIGADE_EMPTY(b)) {
-            if ((retval = ap_get_brigade(c->input_filters, b, AP_MODE_BLOCKING)) != APR_SUCCESS ||
+            if ((retval = ap_get_brigade(c->input_filters, b, AP_MODE_BLOCKING, &zero)) != APR_SUCCESS ||
                 APR_BRIGADE_EMPTY(b)) {
                 apr_brigade_destroy(b);
                 return -1;
@@ -548,8 +549,6 @@ request_rec *ap_read_request(conn_rec *conn)
     const char *expect;
     int access_status;
 
-    AP_DEBUG_ASSERT(conn->remain == 0);
-
     apr_pool_create(&p, conn->pool);
     r = apr_pcalloc(p, sizeof(request_rec));
     r->pool            = p;

diff --git a/server/util_filter.c b/server/util_filter.c
index 381cf03b5195ac03d611101bb9da3715c7f06b5a..2291919eb9a26634f32d7cb7f020211413682099 100644 (file)
--- a/server/util_filter.c
+++ b/server/util_filter.c
@@ -209,10 +209,10 @@ AP_DECLARE(void) ap_remove_output_filter(ap_filter_t *f)
  * brigade especially for that use.
  */
 AP_DECLARE(apr_status_t) ap_get_brigade(ap_filter_t *next, apr_bucket_brigade *bb, 
-                                        ap_input_mode_t mode)
+                                        ap_input_mode_t mode, apr_size_t *readbytes)
 {
     if (next) {
-        return next->frec->filter_func.in_func(next, bb, mode);
+        return next->frec->filter_func.in_func(next, bb, mode, readbytes);
     }
     return AP_NOBODY_READ;
 }