Should I continue this way? Simply to propose patches?
On Sat, Oct 19, 2013 at 4:13 PM, Eric Covener <cove...@gmail.com> wrote:
> Currently, when the body is consumed by a handler, a side effect is
> reading footers that might be present and copying them to
> r->headers_in.
>
> This presents a series of problems.
>
> * things that inspect r->headers_in expect it to be fluffed up much
> earlier than midway through the handler phase
>
> * if the handler looks at headers before reading the body, they could
> differ from what's logged
>
> * if the handler looks at headers after reading the body, mod_headers
> was out of the loop if configured.
>
> I am thinking:
>
> now:
>
> 1) add r->footers_in and use it in 2.2 and up by default
>
Do that mean no API/ABI change ?
> 2) add a directive to copy them up to r->headers_in (for those broken
> by the change)
>
> soon:
>
> 3) add a hook to parse footers
>
> later:
> 4) try to teach mod_headers to do something useful with that hook, but
> not with existing directives.
> 5) teach mod_log_config to log from footers_in
>
Not done (yet), maybe I could try or am I completely bogged down with the
design?
Maybe it worth also :
6) teach ap_expr/ssl_lookup to interpolate from footers
7) teach mod_rewrite (hooked) how to play with footers
8) teach mod_proxy_http to forward the footers
A proposition for 8) follows.
Regards.
Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c (revision 1534970)
+++ modules/proxy/proxy_util.c (working copy)
@@ -3114,11 +3114,7 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
char **old_te_val)
{
conn_rec *c = r->connection;
- int counter;
char *buf;
- const apr_array_header_t *headers_in_array;
- const apr_table_entry_t *headers_in;
- apr_table_t *headers_in_copy;
apr_bucket *e;
int do_100_continue;
conn_rec *origin = p_conn->connection;
@@ -3278,19 +3274,42 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
}
proxy_run_fixups(r);
+ return ap_proxy_fill_hdrbrgd(p, header_brigade, r->headers_in, r,
+ old_cl_val, old_te_val);
+}
+
+PROXY_DECLARE(int) ap_proxy_fill_hdrbrgd(apr_pool_t *p,
+ apr_bucket_brigade
*header_brigade,
+ const apr_table_t *header_table,
+ request_rec *r, char **old_cl_val,
+ char **old_te_val)
+{
+ const apr_array_header_t *headers_in_array;
+ const apr_table_entry_t *headers_in;
+ apr_table_t *headers_in_copy;
+ apr_bucket *e;
+ int counter;
+ char *buf;
+
+ /* easy path first */
+ if (apr_is_empty_table(header_table)) {
+ return OK;
+ }
+
/*
- * Make a copy of the headers_in table before clearing the connection
+ * Make a copy of the header_table before clearing the connection
* headers as we need the connection headers later in the http output
* filter to prepare the correct response headers.
*
* Note: We need to take r->pool for apr_table_copy as the key / value
- * pairs in r->headers_in have been created out of r->pool and
- * p might be (and actually is) a longer living pool.
+ * pairs in header_table (eg. r->headers/footers_in) have been created
+ * out of r->pool and p might be (and actually is) a longer living
pool.
* This would trigger the bad pool ancestry abort in apr_table_copy if
* apr is compiled with APR_POOL_DEBUG.
*/
- headers_in_copy = apr_table_copy(r->pool, r->headers_in);
+ headers_in_copy = apr_table_copy(r->pool, header_table);
ap_proxy_clear_connection(r, headers_in_copy);
+
/* send request headers */
headers_in_array = apr_table_elts(headers_in_copy);
headers_in = (const apr_table_entry_t *) headers_in_array->elts;
@@ -3317,7 +3336,7 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
* If we have used it then MAYBE: RFC2616 says we MAY propagate it.
* So let's make it configurable by env.
*/
- if (!strcasecmp(headers_in[counter].key,"Proxy-Authorization")) {
+ if (!strcasecmp(headers_in[counter].key, "Proxy-Authorization")) {
if (r->user != NULL) { /* we've authenticated */
if (!apr_table_get(r->subprocess_env, "Proxy-Chain-Auth"))
{
continue;
@@ -3328,17 +3347,21 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
/* Skip Transfer-Encoding and Content-Length for now.
*/
if (!strcasecmp(headers_in[counter].key, "Transfer-Encoding")) {
- *old_te_val = headers_in[counter].val;
+ if (old_te_val) {
+ *old_te_val = headers_in[counter].val;
+ }
continue;
}
if (!strcasecmp(headers_in[counter].key, "Content-Length")) {
- *old_cl_val = headers_in[counter].val;
+ if (old_cl_val) {
+ *old_cl_val = headers_in[counter].val;
+ }
continue;
}
/* for sub-requests, ignore freshness/expiry headers */
if (r->main) {
- if ( !strcasecmp(headers_in[counter].key, "If-Match")
+ if ( !strcasecmp(headers_in[counter].key, "If-Match")
|| !strcasecmp(headers_in[counter].key,
"If-Modified-Since")
|| !strcasecmp(headers_in[counter].key, "If-Range")
|| !strcasecmp(headers_in[counter].key,
"If-Unmodified-Since")
@@ -3351,9 +3374,11 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
headers_in[counter].val, CRLF,
NULL);
ap_xlate_proto_to_ascii(buf, strlen(buf));
- e = apr_bucket_pool_create(buf, strlen(buf), p, c->bucket_alloc);
+ e = apr_bucket_pool_create(buf, strlen(buf), p,
+ r->connection->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(header_brigade, e);
}
+
return OK;
}
Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c (revision 1534970)
+++ modules/proxy/mod_proxy_http.c (working copy)
@@ -339,11 +339,11 @@ static int stream_reqbody_chunked(apr_pool_t *p,
bb = input_brigade;
}
- e = apr_bucket_immortal_create(ASCII_ZERO ASCII_CRLF
- /* <trailers> */
- ASCII_CRLF,
- 5, bucket_alloc);
+ e = apr_bucket_immortal_create(ASCII_ZERO ASCII_CRLF, 3, bucket_alloc);
APR_BRIGADE_INSERT_TAIL(bb, e);
+ ap_proxy_fill_hdrbrgd(p, bb, r->footers_in, r, NULL, NULL);
+ e = apr_bucket_immortal_create(ASCII_CRLF, 2, bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
if (apr_table_get(r->subprocess_env, "proxy-sendextracrlf")) {
e = apr_bucket_immortal_create(ASCII_CRLF, 2, bucket_alloc);
@@ -994,6 +994,7 @@ static request_rec *make_fake_req(conn_rec *c, req
rp->status = HTTP_OK;
rp->headers_in = apr_table_make(pool, 50);
+ rp->footers_in = apr_table_make(pool, 5);
rp->subprocess_env = apr_table_make(pool, 50);
rp->headers_out = apr_table_make(pool, 12);
rp->err_headers_out = apr_table_make(pool, 5);
Index: modules/proxy/mod_proxy.h
===================================================================
--- modules/proxy/mod_proxy.h (revision 1534970)
+++ modules/proxy/mod_proxy.h (working copy)
@@ -971,6 +971,21 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
char **old_te_val);
/**
+ * Fill a HTTP request header brigade, old_cl_val and old_te_val as
required.
+ * @param p pool
+ * @param header_brigade header brigade to fill
+ * @param header_table header table to use
+ * @param r request
+ * @param old_cl_val stored old content-len val
+ * @param old_te_val stored old TE val
+ * @return OK
+ */
+PROXY_DECLARE(int) ap_proxy_fill_hdrbrgd(apr_pool_t *p,
+ apr_bucket_brigade
*header_brigade,
+ const apr_table_t *header_table,
+ request_rec *r, char **old_cl_val,
+ char **old_te_val);
+/**
* @param bucket_alloc bucket allocator
* @param r request
* @param p_conn proxy connection
[EOS]
Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c (revision 1534970)
+++ modules/proxy/proxy_util.c (working copy)
@@ -3114,11 +3114,7 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
char **old_te_val)
{
conn_rec *c = r->connection;
- int counter;
char *buf;
- const apr_array_header_t *headers_in_array;
- const apr_table_entry_t *headers_in;
- apr_table_t *headers_in_copy;
apr_bucket *e;
int do_100_continue;
conn_rec *origin = p_conn->connection;
@@ -3278,19 +3274,42 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
}
proxy_run_fixups(r);
+ return ap_proxy_fill_hdrbrgd(p, header_brigade, r->headers_in, r,
+ old_cl_val, old_te_val);
+}
+
+PROXY_DECLARE(int) ap_proxy_fill_hdrbrgd(apr_pool_t *p,
+ apr_bucket_brigade *header_brigade,
+ const apr_table_t *header_table,
+ request_rec *r, char **old_cl_val,
+ char **old_te_val)
+{
+ const apr_array_header_t *headers_in_array;
+ const apr_table_entry_t *headers_in;
+ apr_table_t *headers_in_copy;
+ apr_bucket *e;
+ int counter;
+ char *buf;
+
+ /* easy path first */
+ if (apr_is_empty_table(header_table)) {
+ return OK;
+ }
+
/*
- * Make a copy of the headers_in table before clearing the connection
+ * Make a copy of the header_table before clearing the connection
* headers as we need the connection headers later in the http output
* filter to prepare the correct response headers.
*
* Note: We need to take r->pool for apr_table_copy as the key / value
- * pairs in r->headers_in have been created out of r->pool and
- * p might be (and actually is) a longer living pool.
+ * pairs in header_table (eg. r->headers/footers_in) have been created
+ * out of r->pool and p might be (and actually is) a longer living pool.
* This would trigger the bad pool ancestry abort in apr_table_copy if
* apr is compiled with APR_POOL_DEBUG.
*/
- headers_in_copy = apr_table_copy(r->pool, r->headers_in);
+ headers_in_copy = apr_table_copy(r->pool, header_table);
ap_proxy_clear_connection(r, headers_in_copy);
+
/* send request headers */
headers_in_array = apr_table_elts(headers_in_copy);
headers_in = (const apr_table_entry_t *) headers_in_array->elts;
@@ -3317,7 +3336,7 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
* If we have used it then MAYBE: RFC2616 says we MAY propagate it.
* So let's make it configurable by env.
*/
- if (!strcasecmp(headers_in[counter].key,"Proxy-Authorization")) {
+ if (!strcasecmp(headers_in[counter].key, "Proxy-Authorization")) {
if (r->user != NULL) { /* we've authenticated */
if (!apr_table_get(r->subprocess_env, "Proxy-Chain-Auth")) {
continue;
@@ -3328,17 +3347,21 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
/* Skip Transfer-Encoding and Content-Length for now.
*/
if (!strcasecmp(headers_in[counter].key, "Transfer-Encoding")) {
- *old_te_val = headers_in[counter].val;
+ if (old_te_val) {
+ *old_te_val = headers_in[counter].val;
+ }
continue;
}
if (!strcasecmp(headers_in[counter].key, "Content-Length")) {
- *old_cl_val = headers_in[counter].val;
+ if (old_cl_val) {
+ *old_cl_val = headers_in[counter].val;
+ }
continue;
}
/* for sub-requests, ignore freshness/expiry headers */
if (r->main) {
- if ( !strcasecmp(headers_in[counter].key, "If-Match")
+ if ( !strcasecmp(headers_in[counter].key, "If-Match")
|| !strcasecmp(headers_in[counter].key, "If-Modified-Since")
|| !strcasecmp(headers_in[counter].key, "If-Range")
|| !strcasecmp(headers_in[counter].key, "If-Unmodified-Since")
@@ -3351,9 +3374,11 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
headers_in[counter].val, CRLF,
NULL);
ap_xlate_proto_to_ascii(buf, strlen(buf));
- e = apr_bucket_pool_create(buf, strlen(buf), p, c->bucket_alloc);
+ e = apr_bucket_pool_create(buf, strlen(buf), p,
+ r->connection->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(header_brigade, e);
}
+
return OK;
}
Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c (revision 1534970)
+++ modules/proxy/mod_proxy_http.c (working copy)
@@ -339,11 +339,11 @@ static int stream_reqbody_chunked(apr_pool_t *p,
bb = input_brigade;
}
- e = apr_bucket_immortal_create(ASCII_ZERO ASCII_CRLF
- /* <trailers> */
- ASCII_CRLF,
- 5, bucket_alloc);
+ e = apr_bucket_immortal_create(ASCII_ZERO ASCII_CRLF, 3, bucket_alloc);
APR_BRIGADE_INSERT_TAIL(bb, e);
+ ap_proxy_fill_hdrbrgd(p, bb, r->footers_in, r, NULL, NULL);
+ e = apr_bucket_immortal_create(ASCII_CRLF, 2, bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
if (apr_table_get(r->subprocess_env, "proxy-sendextracrlf")) {
e = apr_bucket_immortal_create(ASCII_CRLF, 2, bucket_alloc);
@@ -994,6 +994,7 @@ static request_rec *make_fake_req(conn_rec *c, req
rp->status = HTTP_OK;
rp->headers_in = apr_table_make(pool, 50);
+ rp->footers_in = apr_table_make(pool, 5);
rp->subprocess_env = apr_table_make(pool, 50);
rp->headers_out = apr_table_make(pool, 12);
rp->err_headers_out = apr_table_make(pool, 5);
Index: modules/proxy/mod_proxy.h
===================================================================
--- modules/proxy/mod_proxy.h (revision 1534970)
+++ modules/proxy/mod_proxy.h (working copy)
@@ -971,6 +971,21 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
char **old_te_val);
/**
+ * Fill a HTTP request header brigade, old_cl_val and old_te_val as required.
+ * @param p pool
+ * @param header_brigade header brigade to fill
+ * @param header_table header table to use
+ * @param r request
+ * @param old_cl_val stored old content-len val
+ * @param old_te_val stored old TE val
+ * @return OK
+ */
+PROXY_DECLARE(int) ap_proxy_fill_hdrbrgd(apr_pool_t *p,
+ apr_bucket_brigade *header_brigade,
+ const apr_table_t *header_table,
+ request_rec *r, char **old_cl_val,
+ char **old_te_val);
+/**
* @param bucket_alloc bucket allocator
* @param r request
* @param p_conn proxy connection
