Follow up to r1840149: core input filter pending data.

Since r1840149 ap_core_input_filter() can't use use f->[priv->]bb directly, so
ap_filter_input_pending() stopped accounting for its pending data.

But ap_core_input_filter() can't (and doesn't need to) setaside its socket
bucket, so ap_filter_setaside_brigade() is not an option. This commit adds
ap_filter_adopt_brigade() which simply moves the given buckets (brigade) into
f->priv->bb, and since this is not something to be done blindly (the buckets
need to have c->pool/bucket_alloc lifetime, which is the case in the core
filter) the function is not AP_DECLAREd/exported thus can be used in core only.

With ap_filter_adopt_brigade() and ap_filter_reinstate_brigade(), the core
input is now ap_filter_input_pending() friendly.

Also, ap_filter_recycle() is no more part of the API (AP_DECLARE removed too),
there really is no point to call it outside core code. MAJOR bumped once again
because of this.

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

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 6fd05bfb0f70ad1b735152ab0b88544414027a6c..090211d9a7d9a9f9783e5b787628b8c8198ba513 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -606,12 +606,13 @@
  *                         ap_acquire_brigade()/ap_release_brigade(), and
  *                         in ap_filter_t replace pending/bb/deferred_pool
  *                         fields by struct ap_filter_private *priv
+ * 20180906.1 (2.5.1-dev)  Don't export ap_filter_recycle() anymore
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
 
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
-#define MODULE_MAGIC_NUMBER_MAJOR 20180905
+#define MODULE_MAGIC_NUMBER_MAJOR 20180906
 #endif
 #define MODULE_MAGIC_NUMBER_MINOR 1                 /* 0...n */
 

diff --git a/include/util_filter.h b/include/util_filter.h
index 135ff35f42ab58a3d0093011dad7a5059a1be87f..064d0cee35d6b034057394a797534a6ca3408946 100644 (file)
--- a/include/util_filter.h
+++ b/include/util_filter.h
@@ -596,6 +596,16 @@ AP_DECLARE(int) ap_filter_prepare_brigade(ap_filter_t *f);
 AP_DECLARE(apr_status_t) ap_filter_setaside_brigade(ap_filter_t *f,
                                                     apr_bucket_brigade *bb);
 
+/*
+ * Adopt a bucket brigade as is (no setaside nor copy).
+ * @param f The current filter
+ * @param bb The bucket brigade adopted.  This brigade is always empty
+ *          on return
+ * @remark httpd internal, not exported, needed by
+ *               ap_core_input_filter
+ */
+void ap_filter_adopt_brigade(ap_filter_t *f, apr_bucket_brigade *bb);
+
 /**
  * Reinstate a brigade setaside earlier, and calculate the amount of data we
  * should write based on the presence of flush buckets, size limits on in
@@ -656,14 +666,17 @@ AP_DECLARE_NONSTD(int) ap_filter_output_pending(conn_rec *c);
  */
 AP_DECLARE_NONSTD(int) ap_filter_input_pending(conn_rec *c);
 
-/**
+/*
  * Recycle removed request filters so that they can be reused for filters
  * added later on the same connection. This typically should happen after
  * each request handling.
  *
  * @param c The connection.
+ * @remark httpd internal, not exported, needed by
+ *         ap_process_request_after_handler
+ *         
  */
-AP_DECLARE(void) ap_filter_recycle(conn_rec *c);
+void ap_filter_recycle(conn_rec *c);
 
 /**
  * Flush function for apr_brigade_* calls.  This calls ap_pass_brigade

diff --git a/modules/http/http_request.c b/modules/http/http_request.c
index 039dd25a2652d28cf67444f43e4c8f15d72e1cb8..78171a849e8dd38b44e1caadb424682c83227d46 100644 (file)
--- a/modules/http/http_request.c
+++ b/modules/http/http_request.c
@@ -402,10 +402,7 @@ AP_DECLARE(void) ap_process_request_after_handler(request_rec *r)
     (void)ap_check_pipeline(c, bb, DEFAULT_LIMIT_BLANK_LINES);
 
     ap_release_brigade(c, bb);
-
-    if (!c->aborted) {
-        ap_filter_recycle(c);
-    }
+    ap_filter_recycle(c);
 
     if (c->cs) {
         if (c->aborted) {

diff --git a/server/core_filters.c b/server/core_filters.c
index 9704fcc095638f74490bef0fe0c68562979c9ffe..53a623d1667b6c841c6e32a17f01748484597b8e 100644 (file)
--- a/server/core_filters.c
+++ b/server/core_filters.c
@@ -94,7 +94,7 @@ apr_status_t ap_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_status_t rv;
+    apr_status_t rv = APR_SUCCESS;
     core_net_rec *net = f->ctx;
     core_ctx_t *ctx = net->in_ctx;
     const char *str;
@@ -124,8 +124,11 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
         if (rv != APR_SUCCESS)
             return rv;
     }
-    else if (APR_BRIGADE_EMPTY(ctx->bb)) {
-        return APR_EOF;
+    else {
+        ap_filter_reinstate_brigade(f, ctx->bb, NULL);
+        if (APR_BRIGADE_EMPTY(ctx->bb)) {
+            return APR_EOF;
+        }
     }
 
     /* ### This is bad. */
@@ -151,7 +154,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
         if (APR_STATUS_IS_EAGAIN(rv) && block == APR_NONBLOCK_READ) {
             rv = APR_SUCCESS;
         }
-        return rv;
+        goto cleanup;
     }
 
     /* ### AP_MODE_PEEK is a horrific name for this mode because we also
@@ -171,15 +174,16 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
          * mean that there is another request, just a blank line.
          */
         while (1) {
-            if (APR_BRIGADE_EMPTY(ctx->bb))
-                return APR_EOF;
+            if (APR_BRIGADE_EMPTY(ctx->bb)) {
+                rv = APR_EOF;
+                goto cleanup;
+            }
 
             e = APR_BRIGADE_FIRST(ctx->bb);
-
             rv = apr_bucket_read(e, &str, &len, APR_NONBLOCK_READ);
-
-            if (rv != APR_SUCCESS)
-                return rv;
+            if (rv != APR_SUCCESS) {
+                goto cleanup;
+            }
 
             c = str;
             while (c < str + len) {
@@ -188,7 +192,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
                 else if (*c == APR_ASCII_CR && *(c + 1) == APR_ASCII_LF)
                     c += 2;
                 else
-                    return APR_SUCCESS;
+                    goto cleanup;
             }
 
             /* If we reach here, we were a bucket just full of CRLFs, so
@@ -196,7 +200,9 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
             /* FIXME: Is this the right thing to do in the core? */
             apr_bucket_delete(e);
         }
-        return APR_SUCCESS;
+
+        /* UNREACHABLE */
+        ap_assert(0);
     }
 
     /* If mode is EXHAUSTIVE, we want to just read everything until the end
@@ -222,7 +228,9 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
          * must be EOS. */
         e = apr_bucket_eos_create(f->c->bucket_alloc);
         APR_BRIGADE_INSERT_TAIL(b, e);
-        return APR_SUCCESS;
+
+        rv = APR_SUCCESS;
+        goto cleanup;
     }
 
     /* read up to the amount they specified. */
@@ -233,14 +241,13 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
 
         e = APR_BRIGADE_FIRST(ctx->bb);
         rv = apr_bucket_read(e, &str, &len, block);
-
-        if (APR_STATUS_IS_EAGAIN(rv) && block == APR_NONBLOCK_READ) {
-            /* getting EAGAIN for a blocking read is an error; for a
-             * non-blocking read, return an empty brigade. */
-            return APR_SUCCESS;
-        }
-        else if (rv != APR_SUCCESS) {
-            return rv;
+        if (rv != APR_SUCCESS) {
+            if (APR_STATUS_IS_EAGAIN(rv) && block == APR_NONBLOCK_READ) {
+                /* getting EAGAIN for a blocking read is an error; not for a
+                 * non-blocking read, return an empty brigade. */
+                rv = APR_SUCCESS;
+            }
+            goto cleanup;
         }
         else if (block == APR_BLOCK_READ && len == 0) {
             /* We wanted to read some bytes in blocking mode.  We read
@@ -257,14 +264,13 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
                 e = apr_bucket_eos_create(f->c->bucket_alloc);
                 APR_BRIGADE_INSERT_TAIL(b, e);
             }
-            return APR_SUCCESS;
+            goto cleanup;
         }
 
         /* Have we read as much data as we wanted (be greedy)? */
         if (len < readbytes) {
             apr_size_t bucket_len;
 
-            rv = APR_SUCCESS;
             /* We already registered the data in e in len */
             e = APR_BUCKET_NEXT(e);
             while ((len < readbytes) && (rv == APR_SUCCESS)
@@ -298,7 +304,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
 
         rv = apr_brigade_partition(ctx->bb, readbytes, &e);
         if (rv != APR_SUCCESS) {
-            return rv;
+            goto cleanup;
         }
 
         /* Must do move before CONCAT */
@@ -316,7 +322,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
             {
                 rv = apr_bucket_copy(e, &copy_bucket);
                 if (rv != APR_SUCCESS) {
-                    return rv;
+                    goto cleanup;
                 }
                 APR_BRIGADE_INSERT_TAIL(b, copy_bucket);
             }
@@ -325,7 +331,10 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
         /* Take what was originally there and place it back on ctx->bb */
         APR_BRIGADE_CONCAT(ctx->bb, ctx->tmpbb);
     }
-    return APR_SUCCESS;
+
+cleanup:
+    ap_filter_adopt_brigade(f, ctx->bb);
+    return rv;
 }
 
 static apr_status_t send_brigade_nonblocking(apr_socket_t *s,

diff --git a/server/util_filter.c b/server/util_filter.c
index 0beb6642ded9b612aa674fa91b207a7116ea0c95..b8dbfc9c39cb56686a7d9f918c93325f049f3b63 100644 (file)
--- a/server/util_filter.c
+++ b/server/util_filter.c
@@ -321,7 +321,8 @@ static struct ap_filter_conn_ctx *get_conn_ctx(conn_rec *c)
     return x;
 }
 
-static void make_spare_ring(struct spare_ring **ring, apr_pool_t *p)
+static APR_INLINE
+void make_spare_ring(struct spare_ring **ring, apr_pool_t *p)
 {
     if (!*ring) {
         *ring = apr_palloc(p, sizeof(**ring));
@@ -403,7 +404,7 @@ static apr_status_t request_filter_cleanup(void *arg)
     return APR_SUCCESS;
 }
 
-AP_DECLARE(void) ap_filter_recycle(conn_rec *c)
+void ap_filter_recycle(conn_rec *c)
 {
     struct ap_filter_conn_ctx *x = c->filter_conn_ctx;
 
@@ -983,6 +984,22 @@ AP_DECLARE(apr_status_t) ap_filter_setaside_brigade(ap_filter_t *f,
     return APR_SUCCESS;
 }
 
+void ap_filter_adopt_brigade(ap_filter_t *f, apr_bucket_brigade *bb)
+{
+    struct ap_filter_private *fp = f->priv;
+
+    ap_log_cerror(APLOG_MARK, APLOG_TRACE6, 0, f->c,
+                  "adopt %s brigade to %s brigade in '%s' output filter",
+                  APR_BRIGADE_EMPTY(bb) ? "empty" : "full",
+                  (!fp->bb || APR_BRIGADE_EMPTY(fp->bb)) ? "empty" : "full",
+                  f->frec->name);
+
+    if (!APR_BRIGADE_EMPTY(bb)) {
+        ap_filter_prepare_brigade(f);
+        APR_BRIGADE_CONCAT(fp->bb, bb);
+    }
+}
+
 AP_DECLARE(apr_status_t) ap_filter_reinstate_brigade(ap_filter_t *f,
                                                      apr_bucket_brigade *bb,
                                                      apr_bucket **flush_upto)
@@ -1308,6 +1325,7 @@ AP_DECLARE_NONSTD(apr_status_t) ap_fprintf(ap_filter_t *f,
     va_end(args);
     return rv;
 }
+
 AP_DECLARE(void) ap_filter_protocol(ap_filter_t *f, unsigned int flags)
 {
     f->frec->proto_flags = flags ;