httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: stop copying footers to r->headers_in?
Date Wed, 23 Oct 2013 12:04:52 GMT
Should I continue this way? Simply to propose patches?

On Sat, Oct 19, 2013 at 4:13 PM, Eric Covener <covener@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]

Mime
View raw message