Merge r591393 from trunk:

Fix handling of buffered request body for per-location SSL
renegotiation when an internal redirect occurs:

* modules/ssl/ssl_engine_io.c (ssl_io_buffer_fill): Remove
protocol-level filters before inserting the buffering filter.
(ssl_io_filter_buffer): Return an EOS if invoked with an empty
brigade; do not remove the filter after exhausting the buffer.
(ssl_io_filter_buffer): Increase the type of the buffer filter to be
AP_FTYPE_PROTOCOL.

PR: 43738
Reviewed by: rpluem, wrowe, jorton

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@608787 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index df1a936398450c30e10cd5f98fd58e4999c2f8a4..591d799ee15a59fbd482bbd660af6f98b4dd9513 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -28,6 +28,10 @@ Changes with Apache 2.2.7
      and etag. PR 44152.
      [Michael Clark <michael metaparadigm.com>, Ruediger Pluem]
 
+  *) mod_ssl: Fix handling of the buffered request body during a per-location
+     renegotiation, when an internal redirect occurs.  PR 43738.
+     [Joe Orton]
+
   *) mod_ldap: Try to establish a new backend LDAP connection when the
      Microsoft LDAP client library returns LDAP_UNAVAILABLE, e.g. after the
      LDAP server has closed the connection due to a timeout.

diff --git a/STATUS b/STATUS
index 50a829a9b24a9d2a361de717a6381f4af9d096be..83babfb8f8017192b61ee2e15117d75e1f4f104b 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -138,14 +138,6 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
      rpluem: I am +1 once we have a confirmation from Werner that this patch fixes
      his problem as this patch has some differences to his original patch.
 
-  * mod_ssl: Fix handling of buffered request body for per-location SSL
-    renegotiation when an internal redirect occurs. PR 43738. [Joe Orton]
-      Trunk version of patch:
-         http://svn.apache.org/viewvc?rev=591393&view=rev
-      Backport version for 2.2.x of patch:
-         Trunk version of patch works
-      +1: rpluem, wrowe
-
   * various modules: Silence some compiler warnings
       Trunk version of patch:
          http://svn.apache.org/viewvc?rev=599651&view=rev

diff --git a/modules/ssl/ssl_engine_io.c b/modules/ssl/ssl_engine_io.c
index 644b57c3ecc3329b35a572e6428d5095fbafbacd..1264c289e3c357174fa24a3b2cd99ff9ad2a3a5e 100644 (file)
--- a/modules/ssl/ssl_engine_io.c
+++ b/modules/ssl/ssl_engine_io.c
@@ -1541,14 +1541,25 @@ int ssl_io_buffer_fill(request_rec *r)
 
     apr_brigade_destroy(tempb);
 
-    /* Insert the filter which will supply the buffered data. */
+    /* After consuming all protocol-level input, remove all protocol-level
+     * filters.  It should strictly only be necessary to remove filters
+     * at exactly ftype == AP_FTYPE_PROTOCOL, since this filter will 
+     * precede all > AP_FTYPE_PROTOCOL anyway. */
+    while (r->proto_input_filters->frec->ftype < AP_FTYPE_CONNECTION) {
+        ap_remove_input_filter(r->proto_input_filters);
+    }
+
+    /* Insert the filter which will supply the buffered content. */
     ap_add_input_filter(ssl_io_buffer, ctx, r, c);
 
     return 0;
 }
 
 /* This input filter supplies the buffered request body to the caller
- * from the brigade stored in f->ctx. */
+ * from the brigade stored in f->ctx.  Note that the placement of this
+ * filter in the filter stack is important; it must be the first
+ * r->proto_input_filter; lower-typed filters will not be preserved
+ * across internal redirects (see PR 43738).  */
 static apr_status_t ssl_io_filter_buffer(ap_filter_t *f,
                                          apr_bucket_brigade *bb,
                                          ap_input_mode_t mode,
@@ -1567,6 +1578,19 @@ static apr_status_t ssl_io_filter_buffer(ap_filter_t *f,
         return APR_ENOTIMPL;
     }
 
+    if (APR_BRIGADE_EMPTY(ctx->bb)) {
+        /* Suprisingly (and perhaps, wrongly), the request body can be
+         * pulled from the input filter stack more than once; a
+         * handler may read it, and ap_discard_request_body() will
+         * attempt to do so again after *every* request.  So input
+         * filters must be prepared to give up an EOS if invoked after
+         * initially reading the request. The HTTP_IN filter does this
+         * with its ->eos_sent flag. */
+
+        APR_BRIGADE_INSERT_TAIL(bb, apr_bucket_eos_create(f->c->bucket_alloc));
+        return APR_SUCCESS;
+    }
+
     if (mode == AP_MODE_READBYTES) {
         apr_bucket *e;
 
@@ -1621,8 +1645,9 @@ static apr_status_t ssl_io_filter_buffer(ap_filter_t *f,
         }
 
         ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, f->c,
-                      "buffered SSL brigade now exhausted; removing filter");
-        ap_remove_input_filter(f);
+                      "buffered SSL brigade exhausted");
+        /* Note that the filter must *not* be removed here; it may be
+         * invoked again, see comment above. */
     }
 
     return APR_SUCCESS;
@@ -1694,7 +1719,7 @@ void ssl_io_filter_register(apr_pool_t *p)
     ap_register_input_filter  (ssl_io_filter, ssl_io_filter_input,  NULL, AP_FTYPE_CONNECTION + 5);
     ap_register_output_filter (ssl_io_filter, ssl_io_filter_output, NULL, AP_FTYPE_CONNECTION + 5);
 
-    ap_register_input_filter  (ssl_io_buffer, ssl_io_filter_buffer, NULL, AP_FTYPE_PROTOCOL - 1);
+    ap_register_input_filter  (ssl_io_buffer, ssl_io_filter_buffer, NULL, AP_FTYPE_PROTOCOL);
 
     return;
 }