fb3f8d
diff --git a/modules/ssl/ssl_engine_io.c b/modules/ssl/ssl_engine_io.c
fb3f8d
index 018b667..4e3875a 100644
fb3f8d
--- a/modules/ssl/ssl_engine_io.c
fb3f8d
+++ b/modules/ssl/ssl_engine_io.c
fb3f8d
@@ -1598,18 +1598,32 @@ static apr_status_t ssl_io_filter_input(ap_filter_t *f,
fb3f8d
 }
fb3f8d
 
fb3f8d
 
fb3f8d
-/* ssl_io_filter_output() produces one SSL/TLS message per bucket
fb3f8d
+/* ssl_io_filter_output() produces one SSL/TLS record per bucket
fb3f8d
  * passed down the output filter stack.  This results in a high
fb3f8d
- * overhead (network packets) for any output comprising many small
fb3f8d
- * buckets.  SSI page applied through the HTTP chunk filter, for
fb3f8d
- * example, may produce many brigades containing small buckets -
fb3f8d
- * [chunk-size CRLF] [chunk-data] [CRLF].
fb3f8d
+ * overhead (more network packets & TLS processing) for any output
fb3f8d
+ * comprising many small buckets.  SSI output passed through the HTTP
fb3f8d
+ * chunk filter, for example, may produce many brigades containing
fb3f8d
+ * small buckets - [chunk-size CRLF] [chunk-data] [CRLF].
fb3f8d
  *
fb3f8d
- * The coalescing filter merges many small buckets into larger buckets
fb3f8d
- * where possible, allowing the SSL I/O output filter to handle them
fb3f8d
- * more efficiently. */
fb3f8d
+ * Sending HTTP response headers as a separate TLS record to the
fb3f8d
+ * response body also reveals information to a network observer (the
fb3f8d
+ * size of headers) which can be significant.
fb3f8d
+ *
fb3f8d
+ * The coalescing filter merges data buckets with the aim of producing
fb3f8d
+ * fewer, larger TLS records - without copying/buffering all content
fb3f8d
+ * and introducing unnecessary overhead.
fb3f8d
+ *
fb3f8d
+ * ### This buffering could be probably be done more comprehensively
fb3f8d
+ * ### in ssl_io_filter_output itself. 
fb3f8d
+ * 
fb3f8d
+ * ### Another possible performance optimisation in particular for the
fb3f8d
+ * ### [HEAP] [FILE] HTTP response case is using a brigade rather than
fb3f8d
+ * ### a char array to buffer; using apr_brigade_write() to append
fb3f8d
+ * ### will use already-allocated memory from the HEAP, reducing # of
fb3f8d
+ * ### copies.
fb3f8d
+ */
fb3f8d
 
fb3f8d
-#define COALESCE_BYTES (2048)
fb3f8d
+#define COALESCE_BYTES (AP_IOBUFSIZE)
fb3f8d
 
fb3f8d
 struct coalesce_ctx {
fb3f8d
     char buffer[COALESCE_BYTES];
fb3f8d
@@ -1622,11 +1636,12 @@ static apr_status_t ssl_io_filter_coalesce(ap_filter_t *f,
fb3f8d
     apr_bucket *e, *upto;
fb3f8d
     apr_size_t bytes = 0;
fb3f8d
     struct coalesce_ctx *ctx = f->ctx;
fb3f8d
+    apr_size_t buffered = ctx ? ctx->bytes : 0; /* space used on entry */
fb3f8d
     unsigned count = 0;
fb3f8d
 
fb3f8d
     /* The brigade consists of zero-or-more small data buckets which
fb3f8d
-     * can be coalesced (the prefix), followed by the remainder of the
fb3f8d
-     * brigade.
fb3f8d
+     * can be coalesced (referred to as the "prefix"), followed by the
fb3f8d
+     * remainder of the brigade.
fb3f8d
      *
fb3f8d
      * Find the last bucket - if any - of that prefix.  count gives
fb3f8d
      * the number of buckets in the prefix.  The "prefix" must contain
fb3f8d
@@ -1641,24 +1656,97 @@ static apr_status_t ssl_io_filter_coalesce(ap_filter_t *f,
fb3f8d
          e != APR_BRIGADE_SENTINEL(bb)
fb3f8d
              && !APR_BUCKET_IS_METADATA(e)
fb3f8d
              && e->length != (apr_size_t)-1
fb3f8d
-             && e->length < COALESCE_BYTES
fb3f8d
-             && (bytes + e->length) < COALESCE_BYTES
fb3f8d
-             && (ctx == NULL
fb3f8d
-                 || bytes + ctx->bytes + e->length < COALESCE_BYTES);
fb3f8d
+             && e->length <= COALESCE_BYTES
fb3f8d
+             && (buffered + bytes + e->length) <= COALESCE_BYTES;
fb3f8d
          e = APR_BUCKET_NEXT(e)) {
fb3f8d
         if (e->length) count++; /* don't count zero-length buckets */
fb3f8d
         bytes += e->length;
fb3f8d
     }
fb3f8d
+
fb3f8d
+    /* If there is room remaining and the next bucket is a data
fb3f8d
+     * bucket, try to include it in the prefix to coalesce.  For a
fb3f8d
+     * typical [HEAP] [FILE] HTTP response brigade, this handles
fb3f8d
+     * merging the headers and the start of the body into a single TLS
fb3f8d
+     * record. */
fb3f8d
+    if (bytes + buffered > 0
fb3f8d
+        && bytes + buffered < COALESCE_BYTES
fb3f8d
+        && e != APR_BRIGADE_SENTINEL(bb)
fb3f8d
+        && !APR_BUCKET_IS_METADATA(e)) {
fb3f8d
+        apr_status_t rv = APR_SUCCESS;
fb3f8d
+
fb3f8d
+        /* For an indeterminate length bucket (PIPE/CGI/...), try a
fb3f8d
+         * non-blocking read to have it morph into a HEAP.  If the
fb3f8d
+         * read fails with EAGAIN, it is harmless to try a split
fb3f8d
+         * anyway, split is ENOTIMPL for most PIPE-like buckets. */
fb3f8d
+        if (e->length == (apr_size_t)-1) {
fb3f8d
+            const char *discard;
fb3f8d
+            apr_size_t ignore;
fb3f8d
+
fb3f8d
+            rv = apr_bucket_read(e, &discard, &ignore, APR_NONBLOCK_READ);
fb3f8d
+            if (rv != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(rv)) {
fb3f8d
+                ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, f->c, APLOGNO(10232)
fb3f8d
+                              "coalesce failed to read from %s bucket",
fb3f8d
+                              e->type->name);
fb3f8d
+                return AP_FILTER_ERROR;
fb3f8d
+            }
fb3f8d
+        }
fb3f8d
+
fb3f8d
+        if (rv == APR_SUCCESS) {
fb3f8d
+            /* If the read above made the bucket morph, it may now fit
fb3f8d
+             * entirely within the buffer.  Otherwise, split it so it does
fb3f8d
+             * fit. */
fb3f8d
+            if (e->length > COALESCE_BYTES
fb3f8d
+                || e->length + buffered + bytes > COALESCE_BYTES) {
fb3f8d
+                rv = apr_bucket_split(e, COALESCE_BYTES - (buffered + bytes));
fb3f8d
+            }
fb3f8d
+
fb3f8d
+            if (rv == APR_SUCCESS && e->length == 0) {
fb3f8d
+                /* As above, don't count in the prefix if the bucket is
fb3f8d
+                 * now zero-length. */
fb3f8d
+            }
fb3f8d
+            else if (rv == APR_SUCCESS) {
fb3f8d
+                ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, f->c,
fb3f8d
+                              "coalesce: adding %" APR_SIZE_T_FMT " bytes "
fb3f8d
+                              "from split %s bucket, total %" APR_SIZE_T_FMT,
fb3f8d
+                              e->length, e->type->name, bytes + buffered);
fb3f8d
+
fb3f8d
+                count++;
fb3f8d
+                bytes += e->length;
fb3f8d
+                e = APR_BUCKET_NEXT(e);
fb3f8d
+            }
fb3f8d
+            else if (rv != APR_ENOTIMPL) {
fb3f8d
+                ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, f->c, APLOGNO(10233)
fb3f8d
+                              "coalesce: failed to split data bucket");
fb3f8d
+                return AP_FILTER_ERROR;
fb3f8d
+            }
fb3f8d
+        }
fb3f8d
+    }
fb3f8d
+
fb3f8d
+    /* The prefix is zero or more buckets.  upto now points to the
fb3f8d
+     * bucket AFTER the end of the prefix, which may be the brigade
fb3f8d
+     * sentinel. */
fb3f8d
     upto = e;
fb3f8d
 
fb3f8d
-    /* Coalesce the prefix, if:
fb3f8d
-     * a) more than one bucket is found to coalesce, or
fb3f8d
-     * b) the brigade contains only a single data bucket, or
fb3f8d
-     * c) the data bucket is not last but we have buffered data already.
fb3f8d
+    /* Coalesce the prefix, if any of the following are true:
fb3f8d
+     * 
fb3f8d
+     * a) the prefix is more than one bucket
fb3f8d
+     * OR
fb3f8d
+     * b) the prefix is the entire brigade, which is a single bucket
fb3f8d
+     *    AND the prefix length is smaller than the buffer size,
fb3f8d
+     * OR
fb3f8d
+     * c) the prefix is a single bucket
fb3f8d
+     *    AND there is buffered data from a previous pass.
fb3f8d
+     * 
fb3f8d
+     * The aim with (b) is to buffer a small bucket so it can be
fb3f8d
+     * coalesced with future invocations of this filter.  e.g.  three
fb3f8d
+     * calls each with a single 100 byte HEAP bucket should get
fb3f8d
+     * coalesced together.  But an invocation with a 8192 byte HEAP
fb3f8d
+     * should pass through untouched.
fb3f8d
      */
fb3f8d
     if (bytes > 0
fb3f8d
         && (count > 1
fb3f8d
-            || (upto == APR_BRIGADE_SENTINEL(bb))
fb3f8d
+            || (upto == APR_BRIGADE_SENTINEL(bb)
fb3f8d
+                && bytes < COALESCE_BYTES)
fb3f8d
             || (ctx && ctx->bytes > 0))) {
fb3f8d
         /* If coalescing some bytes, ensure a context has been
fb3f8d
          * created. */
fb3f8d
@@ -1669,7 +1757,8 @@ static apr_status_t ssl_io_filter_coalesce(ap_filter_t *f,
fb3f8d
 
fb3f8d
         ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, f->c,
fb3f8d
                       "coalesce: have %" APR_SIZE_T_FMT " bytes, "
fb3f8d
-                      "adding %" APR_SIZE_T_FMT " more", ctx->bytes, bytes);
fb3f8d
+                      "adding %" APR_SIZE_T_FMT " more (buckets=%u)",
fb3f8d
+                      ctx->bytes, bytes, count);
fb3f8d
 
fb3f8d
         /* Iterate through the prefix segment.  For non-fatal errors
fb3f8d
          * in this loop it is safe to break out and fall back to the
fb3f8d
@@ -1684,7 +1773,8 @@ static apr_status_t ssl_io_filter_coalesce(ap_filter_t *f,
fb3f8d
             if (APR_BUCKET_IS_METADATA(e)
fb3f8d
                 || e->length == (apr_size_t)-1) {
fb3f8d
                 ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, f->c, APLOGNO(02012)
fb3f8d
-                              "unexpected bucket type during coalesce");
fb3f8d
+                              "unexpected %s bucket during coalesce",
fb3f8d
+                              e->type->name);
fb3f8d
                 break; /* non-fatal error; break out */
fb3f8d
             }
fb3f8d