mod_request: Insert the KEPT_BODY filter via the insert_filter

hook instead of during fixups. Add a safety check to ensure the
filters cannot be inserted more than once. [Graham Leggett,
Ruediger Pluem]

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

diff --git a/CHANGES b/CHANGES
index 2ee0c6e9927a41c9a1404c23bc11b9c4a793b0dd..59fa1db041bd0673caece03c6cbc2b9de4b31942 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,11 @@
 Changes with Apache 2.3.0
 [ When backported to 2.2.x, remove entry from this file ]
 
+  *) mod_request: Insert the KEPT_BODY filter via the insert_filter
+     hook instead of during fixups. Add a safety check to ensure the
+     filters cannot be inserted more than once. [Graham Leggett,
+     Rüdiger Pluem]
+
   *) core: Do not allow Options ALL if not all options are allowed to be
      overwritten. PR 44262 [Michał Grzędzicki <lazy iq.pl>]
 

diff --git a/modules/filters/mod_request.c b/modules/filters/mod_request.c
index 32a9d9ef7825067e7f23eba3e40d834890c2894b..9e7c05d0678e583bf8af6b434e07dd8024900fb4 100644 (file)
--- a/modules/filters/mod_request.c
+++ b/modules/filters/mod_request.c
@@ -185,7 +185,7 @@ static int kept_body_filter_init(ap_filter_t *f) {
     if (kept_body) {
         apr_table_unset(r->headers_in, "Transfer-Encoding");
         apr_brigade_length(kept_body, 1, &length);
-        apr_table_set(r->headers_in, "Content-Length", apr_off_t_toa(r->pool, length));
+        apr_table_setn(r->headers_in, "Content-Length", apr_off_t_toa(r->pool, length));
     }
 
     return OK;
@@ -307,13 +307,11 @@ typedef enum {
  * NOTE: File upload is not yet supported, but can be without change
  * to the function call.
  */
-AP_DECLARE(int) ap_parse_request_form(request_rec * r, apr_array_header_t ** ptr,
+AP_DECLARE(int) ap_parse_request_form(request_rec * r, ap_filter_t * f, 
+                                      apr_array_header_t ** ptr,
                                       apr_size_t num, apr_size_t size)
 {
-    request_dir_conf *dconf;
-    apr_off_t left = 0;
-    apr_bucket_brigade *bb = NULL, *kept_body = NULL;
-    apr_bucket *e;
+    apr_bucket_brigade *bb = NULL;
     int seen_eos = 0;
     char buffer[HUGE_STRING_LEN + 1];
     const char *ct;
@@ -333,18 +331,15 @@ AP_DECLARE(int) ap_parse_request_form(request_rec * r, apr_array_header_t ** ptr
         return ap_discard_request_body(r);
     }
 
-    dconf = ap_get_module_config(r->per_dir_config,
-                                     &request_module);
-    if (dconf->keep_body > 0) {
-        left = dconf->keep_body;
-        kept_body = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+    if (!f) {
+        f = r->input_filters;
     }
 
     bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
     do {
         apr_bucket *bucket = NULL, *last = NULL;
 
-        int rv = ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES,
+        int rv = ap_get_brigade(f, bb, AP_MODE_READBYTES,
                                 APR_BLOCK_READ, HUGE_STRING_LEN);
         if (rv != APR_SUCCESS) {
             apr_brigade_destroy(bb);
@@ -460,24 +455,6 @@ AP_DECLARE(int) ap_parse_request_form(request_rec * r, apr_array_header_t ** ptr
                 }
             }
 
-            /* If we have been asked to, keep the data up until the
-             * configured limit. If the limit is exceeded, we return an
-             * HTTP_REQUEST_ENTITY_TOO_LARGE response so the caller is
-             * clear the server couldn't handle their request.
-             */
-            if (kept_body) {
-                if (len <= left) {
-                    apr_bucket_copy(bucket, &e);
-                    APR_BRIGADE_INSERT_TAIL(kept_body, e);
-                    left -= len;
-                }
-                else {
-                    apr_brigade_destroy(bb);
-                    apr_brigade_destroy(kept_body);
-                    return HTTP_REQUEST_ENTITY_TOO_LARGE;
-                }
-            }
-
         }
 
         apr_brigade_cleanup(bb);
@@ -492,34 +469,69 @@ AP_DECLARE(int) ap_parse_request_form(request_rec * r, apr_array_header_t ** ptr
         APR_BRIGADE_INSERT_TAIL(pair->value, b);
     }
 
-    if (kept_body) {
-        r->kept_body = kept_body;
-    }
-
     return OK;
 
 }
 
 /**
- * Fixups hook.
+ * Check whether this filter is not already present.
+ */
+static int request_is_filter_present(request_rec * r, ap_filter_rec_t *fn)
+{
+    ap_filter_t * f = r->input_filters;
+    while (f) {
+        if (f->frec == fn) {
+            return 1;
+        }
+        f = f->next;
+    }
+    return 0;
+}
+
+/**
+ * Insert filter hook.
  * 
  * Add the KEEP_BODY filter to the request, if the admin wants to keep
  * the body using the KeptBodySize directive.
  * 
+ * As a precaution, any pre-existing instances of either the kept_body or
+ * keep_body filters will be removed before the filter is added.
+ * 
  * @param r The request
  */
-static int request_fixups(request_rec * r)
+AP_DECLARE(void) ap_request_insert_filter(request_rec * r)
 {
     request_dir_conf *conf = ap_get_module_config(r->per_dir_config,
                                                   &request_module);
 
-    if (conf->keep_body) {
-        ap_add_input_filter_handle(ap_keep_body_input_filter_handle,
-                                   NULL, r, r->connection);
+    if (r->kept_body) {
+        if (!request_is_filter_present(r, ap_kept_body_input_filter_handle)) {
+            ap_add_input_filter_handle(ap_kept_body_input_filter_handle,
+                                       NULL, r, r->connection);
+        }
+    }
+    else if (conf->keep_body) {
+        if (!request_is_filter_present(r, ap_kept_body_input_filter_handle)) {
+            ap_add_input_filter_handle(ap_keep_body_input_filter_handle,
+                                       NULL, r, r->connection);
+        }
     }
 
-    return OK;
+}
 
+/**
+ * Remove the kept_body and keep body filters from this specific request.
+ */
+AP_DECLARE(void) ap_request_remove_filter(request_rec * r)
+{
+    ap_filter_t * f = r->input_filters;
+    while (f) {
+        if (f->frec->filter_func.in_func == ap_kept_body_filter ||
+                f->frec->filter_func.in_func == ap_keep_body_filter) {
+            ap_remove_input_filter(f);
+        }
+        f = f->next;
+    }
 }
 
 static void *create_request_dir_config(apr_pool_t *p, char *dummy)
@@ -573,8 +585,10 @@ static void register_hooks(apr_pool_t *p)
     ap_kept_body_input_filter_handle =
         ap_register_input_filter(KEPT_BODY_FILTER, ap_kept_body_filter,
                                  kept_body_filter_init, AP_FTYPE_RESOURCE);
-    ap_hook_fixups(request_fixups, NULL, NULL, APR_HOOK_MIDDLE);
+    ap_hook_insert_filter(ap_request_insert_filter, NULL, NULL, APR_HOOK_LAST);
     APR_REGISTER_OPTIONAL_FN(ap_parse_request_form);
+    APR_REGISTER_OPTIONAL_FN(ap_request_insert_filter);
+    APR_REGISTER_OPTIONAL_FN(ap_request_remove_filter);
 }
 
 module AP_MODULE_DECLARE_DATA request_module = {

diff --git a/modules/filters/mod_request.h b/modules/filters/mod_request.h
index 53afdfb1e2b98a92b3a0c2c6b7d78b902475e4a5..8f83a7177507081db6b3e07d579b4dcf53670123 100644 (file)
--- a/modules/filters/mod_request.h
+++ b/modules/filters/mod_request.h
@@ -66,6 +66,12 @@ AP_DECLARE(apr_status_t) ap_kept_body_filter(ap_filter_t *f, apr_bucket_brigade
                                              ap_input_mode_t mode, apr_read_type_e block,
                                              apr_off_t readbytes);
 
+/* Optional function to add either the keep body filter or kept body filter as appropriate */
+AP_DECLARE(void) ap_request_insert_filter(request_rec * r);
+
+/* Optional function to remove either the keep body filter or kept body filter as appropriate */
+AP_DECLARE(void) ap_request_remove_filter(request_rec * r);
+
 /**
  * Structure to store the contents of an HTTP form of the type
  * application/x-www-form-urlencoded.
@@ -111,15 +117,21 @@ typedef struct {
  * NOTE: File upload is not yet supported, but can be without change
  * to the function call.
  */
-AP_DECLARE(int) ap_parse_request_form(request_rec * r, apr_array_header_t ** ptr,
+AP_DECLARE(int) ap_parse_request_form(request_rec * r, ap_filter_t * f, 
+                                      apr_array_header_t ** ptr,
                                       apr_size_t num, apr_size_t size);
 
-APR_DECLARE_OPTIONAL_FN(int, ap_parse_request_form, (request_rec * r, apr_array_header_t ** ptr,
+APR_DECLARE_OPTIONAL_FN(int, ap_parse_request_form, (request_rec * r, ap_filter_t * f, 
+                                                     apr_array_header_t ** ptr,
                                                      apr_size_t num, apr_size_t size));
 
+APR_DECLARE_OPTIONAL_FN(void, ap_request_insert_filter, (request_rec * r));
+
+APR_DECLARE_OPTIONAL_FN(void, ap_request_remove_filter, (request_rec * r));
+
 #ifdef __cplusplus
 }
 #endif
 
-#endif /* !MOD_REQUEST_H */
+#endif /* !MOD_REQUEST_H */
 /** @} */

diff --git a/server/request.c b/server/request.c
index 1c58a398cc96288b1e7510325c71e6b78d2a30e3..5d4c9aa4b62b0989ba885dd78a0a7dc326abcec6 100644 (file)
--- a/server/request.c
+++ b/server/request.c
@@ -1645,13 +1645,6 @@ static request_rec *make_sub_request(const request_rec *r,
     /* Pass on the kept body (if any) into the new request. */
     rnew->kept_body = r->kept_body;
 
-    /*
-     * Add the KEPT_BODY filter, which will insert any body marked to be
-     * kept for the use of a subrequest, into the subrequest.
-     */
-    ap_add_input_filter(KEPT_BODY_FILTER,
-                        NULL, rnew, rnew->connection);
-
     return rnew;
 }