Merge r1764040 from trunk:

mod_dav: Fix a potential cause of unbounded memory usage or incorrect
behavior in a routine that sends <DAV:response>'s to the output filters.

The dav_send_one_response() function accepts the current head of the output
filter list as an argument, but the actual head can change between calls to
ap_pass_brigade().  This can happen with self-removing filters, e.g., with
the filter from mod_headers or mod_deflate.  Consequently, executing an
already removed filter can either cause unwanted memory usage or incorrect
behavior.

This patch changes the signature of the existing mod_dav's public API,
dav_send_one_response(), because this API is not yet a part of any 2.4.x
release.

* modules/dav/main/mod_dav.c
  (dav_send_one_response): Accept a request_rec instead of an ap_filter_t.
   Write the response to r->output_filters.
  (dav_send_multistatus, dav_stream_response): Update these calling sites
   of dav_send_one_response().

* modules/dav/main/mod_dav.h
  (dav_send_one_response): Adjust definition.

Submitted by: kotkov
Reviewed/backported by: jim

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1766683 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/STATUS b/STATUS
index 941aea484ec3dfc1cb6e0dbb77f9bbdb56c348b5..27a8f4e4597bc90ce384dc8fe39f5206cd4a13ca 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -117,18 +117,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_dav: Fix a potential cause of unbounded memory usage or incorrect
-     behavior in a routine that sends <DAV:response>'s to the output filters.
-     trunk patch: http://svn.apache.org/r1764040
-     2.4.x patch: trunk works (modulo CHANGES)
-     Note: this patch changes the signature of the existing mod_dav's public
-     API, dav_send_one_response(), because this API is not yet a part of any
-     2.4.x release (it was backported to 2.4.x in r1756561).  So, the change
-     should either go to the upcoming 2.4.24, or should be reworked in case
-     2.4.24 is released without it.  See the thread at
-     https://mail-archives.apache.org/mod_mbox/httpd-dev/201608.mbox/%3C20160822151917.GA22369%40redhat.com%3E
-     for additional details.
-     +1: kotkov, jorton, jim
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]

diff --git a/modules/dav/main/mod_dav.c b/modules/dav/main/mod_dav.c
index aed987dca8da238307b235fb1f153bd5f6ed5ebb..7599c6d8635d28fbbab207819f125ba6161c330c 100644 (file)
--- a/modules/dav/main/mod_dav.c
+++ b/modules/dav/main/mod_dav.c
@@ -438,30 +438,31 @@ static const char *dav_xml_escape_uri(apr_pool_t *p, const char *uri)
 
 /* Write a complete RESPONSE object out as a <DAV:repsonse> xml
    element.  Data is sent into brigade BB, which is auto-flushed into
-   OUTPUT filter stack.  Use POOL for any temporary allocations.
+   the output filter stack for request R.  Use POOL for any temporary
+   allocations.
 
    [Presumably the <multistatus> tag has already been written;  this
    routine is shared by dav_send_multistatus and dav_stream_response.]
 */
 DAV_DECLARE(void) dav_send_one_response(dav_response *response,
                                         apr_bucket_brigade *bb,
-                                        ap_filter_t *output,
+                                        request_rec *r,
                                         apr_pool_t *pool)
 {
     apr_text *t = NULL;
 
     if (response->propresult.xmlns == NULL) {
-      ap_fputs(output, bb, "<D:response>");
+      ap_fputs(r->output_filters, bb, "<D:response>");
     }
     else {
-      ap_fputs(output, bb, "<D:response");
+      ap_fputs(r->output_filters, bb, "<D:response");
       for (t = response->propresult.xmlns; t; t = t->next) {
-        ap_fputs(output, bb, t->text);
+        ap_fputs(r->output_filters, bb, t->text);
       }
-      ap_fputc(output, bb, '>');
+      ap_fputc(r->output_filters, bb, '>');
     }
 
-    ap_fputstrs(output, bb,
+    ap_fputstrs(r->output_filters, bb,
                 DEBUG_CR "<D:href>",
                 dav_xml_escape_uri(pool, response->href),
                 "</D:href>" DEBUG_CR,
@@ -472,7 +473,7 @@ DAV_DECLARE(void) dav_send_one_response(dav_response *response,
        * default to 500 Internal Server Error if first->status
        * is not a known (or valid) status code.
        */
-      ap_fputstrs(output, bb,
+      ap_fputstrs(r->output_filters, bb,
                   "<D:status>HTTP/1.1 ",
                   ap_get_status_line(response->status),
                   "</D:status>" DEBUG_CR,
@@ -481,7 +482,7 @@ DAV_DECLARE(void) dav_send_one_response(dav_response *response,
     else {
       /* assume this includes <propstat> and is quoted properly */
       for (t = response->propresult.propstats; t; t = t->next) {
-        ap_fputs(output, bb, t->text);
+        ap_fputs(r->output_filters, bb, t->text);
       }
     }
 
@@ -490,14 +491,14 @@ DAV_DECLARE(void) dav_send_one_response(dav_response *response,
        * We supply the description, so we know it doesn't have to
        * have any escaping/encoding applied to it.
        */
-      ap_fputstrs(output, bb,
+      ap_fputstrs(r->output_filters, bb,
                   "<D:responsedescription>",
                   response->desc,
                   "</D:responsedescription>" DEBUG_CR,
                   NULL);
     }
 
-    ap_fputs(output, bb, "</D:response>" DEBUG_CR);
+    ap_fputs(r->output_filters, bb, "</D:response>" DEBUG_CR);
 }
 
 
@@ -559,7 +560,7 @@ DAV_DECLARE(void) dav_send_multistatus(request_rec *r, int status,
 
     for (; first != NULL; first = first->next) {
       apr_pool_clear(subpool);
-      dav_send_one_response(first, bb, r->output_filters, subpool);
+      dav_send_one_response(first, bb, r, subpool);
     }
     apr_pool_destroy(subpool);
 
@@ -1166,7 +1167,7 @@ static void dav_stream_response(dav_walk_resource *wres,
         resp.propresult = *propstats;
     }
 
-    dav_send_one_response(&resp, ctx->bb, ctx->r->output_filters, pool);
+    dav_send_one_response(&resp, ctx->bb, ctx->r, pool);
 }
 
 

diff --git a/modules/dav/main/mod_dav.h b/modules/dav/main/mod_dav.h
index 63fd990463b1a132703611b60226824d6f23b967..80ad1176b4d3a243f57e26cdbd00fcf0e81ddaa2 100644 (file)
--- a/modules/dav/main/mod_dav.h
+++ b/modules/dav/main/mod_dav.h
@@ -536,14 +536,15 @@ typedef enum {
 
 /* Write a complete RESPONSE object out as a <DAV:response> xml
  * element.  Data is sent into brigade BB, which is auto-flushed into
- * OUTPUT filter stack.  Use POOL for any temporary allocations.
+ * the output filter stack for request R.  Use POOL for any temporary
+ * allocations.
  *
  * [Presumably the <multistatus> tag has already been written;  this
  * routine is shared by dav_send_multistatus and dav_stream_response.]
  */
 DAV_DECLARE(void) dav_send_one_response(dav_response *response,
                                         apr_bucket_brigade *bb,
-                                        ap_filter_t *output,
+                                        request_rec *r,
                                         apr_pool_t *pool);
 
 /* Factorized helper function: prep request_rec R for a multistatus