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