httpd-37145

Version:

2.0.55

How it is diagnosed (reproduced or source analysis)?

Source analysis.

Symptom:

When using apache as reverse proxy web server, large data from 'post' command does not reach to the real web server.

Root cause:

When copying the data from ‘post’ command into intermediate data-structure, Apache only copied the pointer instead of performing ‘deep copy’ to also duplicate the content, thus resulting the data was overwritten before used.

Fix: in file ‘modules/proxy/proxy_http.c’, they replaced ‘APR_BRIGADE_CONCAT’ function call with ap_save_brigade. ‘brigade’ is a data structure used in apache project.

@@ -1081,9 +1123,27 @@

        }

-        APR_BRIGADE_CONCAT(input_brigade, temp_brigade);

+        /*

+         * Save temp_brigade in input_brigade. (At least) in the SSL case

+         * temp_brigade contains transient buckets whose data would get

+         * overwritten during the next call of ap_get_brigade in the loop.

+         * ap_save_brigade ensures these buckets to be set aside.

+         * Calling ap_save_brigade with NULL as filter is OK, because

+         * input_brigade already has been created and does not need to get

+         * created by ap_save_brigade.

+         */

+        status = ap_save_brigade(NULL, &input_brigade, &temp_brigade, p);

+        if (status != APR_SUCCESS) {

+            ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server,

+                         "proxy: processing prefetched request body failed"

+                         " to %s from %s (%s)",

+                         p_conn->name ? p_conn->name: "",

+                         c->remote_ip, c->remote_host ? c->remote_host: "");

+            return status;

+        }

+

/* This is the copy-pointer function used by buggy version,

   not duplicating the content. */

424 #define APR_BRIGADE_CONCAT(a, b) do {                   \

425         APR_RING_CONCAT(&(a)->list, &(b)->list, apr_bucket, link);  \

427     } while (0)

/* This is to perform ‘deep copy’ -- not only copying pointer, but also duplicating the content.*/

AP_DECLARE(apr_status_t) ap_save_brigade(ap_filter_t *f,

                                        apr_bucket_brigade **saveto,

                                        apr_bucket_brigade **b, apr_pool_t *p)

{

   apr_bucket *e;

   apr_status_t rv;

   /* If have never stored any data in the filter, then we had better

    * create an empty bucket brigade so that we can concat.

    */

   if (!(*saveto)) {

       *saveto = apr_brigade_create(p, f->c->bucket_alloc);

   }

   

   APR_RING_FOREACH(e, &(*b)->list, apr_bucket, link) {

        /* This is to perform duplicate the content.*/

       rv = apr_bucket_setaside(e, p);

       if (rv != APR_SUCCESS

           /* ### this ENOTIMPL will go away once we implement setaside

              ### for all bucket types. */

           && rv != APR_ENOTIMPL) {

           return rv;

       }

   }

   APR_BRIGADE_CONCAT(*saveto, *b);

   return APR_SUCCESS;

}

As described in the added comment, previous APR_BRIGADE_CONCAT function copies the data without deep-copy while ap_save_brigade stores the data to be set aside(deep-copy). Therefore, the data is overwritten in the next cycle. It introduces the loss of data. Also, when the function doesn’t work, it returns the status.

Also in this part of the patch, ‘temp_brigade’ data is stored in ‘input_brigade’ using deep copy so that it prevents loss of data.

Is there Error Message?

No.

Can Errlog anticipate the error:

No. There is no difference than normal execution: should do sth but didn’t do.

So the execution is completely normal, only the data is wrong!