On Tue, May 06, 2014 at 07:45:42PM -0400, Eric Covener wrote:
> On Tue, May 6, 2014 at 6:44 PM, Yann Ylavic <ylavic....@gmail.com> wrote:
> > This patch (still) does not propose to merge splitted trailers into
> > headers (for those broken by the change), but when (and why) would we
> > do that?
>
> For 2.2, I would personally want to have the ability to restore the
> old merging behavior before any release. Joe drew his line there as
> well.
I hacked the attached up yesterday, before seeing Yann's mail, untested.
Yann, in your smaller patch is it deliberate that you only handle one of
the two places trailers are read in ap_http_filter()? Either that is an
oversight or I am missing something.
I'm also daunted by the size of change required to a rewrite core
function like ap_rgetline* to be non-blocking, at least in 2.[24]; that
seems really like an orthogonal issue.
Regards, Joe
Index: include/httpd.h
===================================================================
--- include/httpd.h (revision 1592743)
+++ include/httpd.h (working copy)
@@ -1032,6 +1032,10 @@
*/
apr_sockaddr_t *useragent_addr;
char *useragent_ip;
+
+ /* MIME headers from the "trailer" from a chunked request,
+ * received only after the request body has been read. */
+ apr_table_t *trailers_in;
};
/**
Index: modules/http/http_filters.c
===================================================================
--- modules/http/http_filters.c (revision 1592743)
+++ modules/http/http_filters.c (working copy)
@@ -230,7 +230,29 @@
return get_remaining_chunk_line(ctx, b, linelimit);
}
+static apr_status_t read_chunked_trailers(http_ctx_t *ctx, ap_filter_t *f,
+ apr_bucket_brigade *b)
+{
+ apr_bucket *e;
+ apr_table_t *tmp;
+ /* Handle trailers by calling ap_get_mime_headers again! */
+ ctx->state = BODY_NONE;
+
+ /* Temporarily fool ap_get_mime_headers() into using the trailers
+ * table. Ugly but simple. */
+ tmp = f->r->headers_in;
+ f->r->headers_in = f->r->trailers_in;
+ ap_get_mime_headers(f->r);
+ f->r->headers_in = tmp;
+
+ e = apr_bucket_eos_create(f->c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(b, e);
+ ctx->eos_sent = 1;
+
+ return APR_SUCCESS;
+}
+
/* This is the HTTP_INPUT filter for HTTP requests and responses from
* proxied servers (mod_proxy). It handles chunked and content-length
* bodies. This can only be inserted/used after the headers
@@ -425,13 +447,7 @@
}
if (!ctx->remaining) {
- /* Handle trailers by calling ap_get_mime_headers again! */
- ctx->state = BODY_NONE;
- ap_get_mime_headers(f->r);
- e = apr_bucket_eos_create(f->c->bucket_alloc);
- APR_BRIGADE_INSERT_TAIL(b, e);
- ctx->eos_sent = 1;
- return APR_SUCCESS;
+ return read_chunked_trailers(ctx, f, b);
}
}
}
@@ -534,13 +550,7 @@
}
if (!ctx->remaining) {
- /* Handle trailers by calling ap_get_mime_headers again! */
- ctx->state = BODY_NONE;
- ap_get_mime_headers(f->r);
- e = apr_bucket_eos_create(f->c->bucket_alloc);
- APR_BRIGADE_INSERT_TAIL(b, e);
- ctx->eos_sent = 1;
- return APR_SUCCESS;
+ return read_chunked_trailers(ctx, f, b);
}
}
break;
Index: modules/http/http_request.c
===================================================================
--- modules/http/http_request.c (revision 1592743)
+++ modules/http/http_request.c (working copy)
@@ -463,6 +463,7 @@
new->main = r->main;
new->headers_in = r->headers_in;
+ new->trailers_in = r->headers_in;
new->headers_out = apr_table_make(r->pool, 12);
if (ap_is_HTTP_REDIRECT(new->status)) {
const char *location = apr_table_get(r->headers_out, "Location");
Index: server/protocol.c
===================================================================
--- server/protocol.c (revision 1592743)
+++ server/protocol.c (working copy)
@@ -921,6 +921,7 @@
r->headers_out = apr_table_make(r->pool, 12);
r->err_headers_out = apr_table_make(r->pool, 5);
r->notes = apr_table_make(r->pool, 5);
+ r->trailers_in = apr_table_make(r->pool, 5);
r->request_config = ap_create_request_config(r->pool);
/* Must be set before we run create request hook */
@@ -1185,6 +1186,7 @@
rnew->status = HTTP_OK;
rnew->headers_in = apr_table_copy(rnew->pool, r->headers_in);
+ rnew->trailers_in = apr_table_copy(rnew->pool, r->trailers_in);
/* did the original request have a body? (e.g. POST w/SSI tags)
* if so, make sure the subrequest doesn't inherit body headers
