Commit 6d57c7e3 authored by Jim Jagielski's avatar Jim Jagielski
Browse files

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
parent 85c7cd1a

STATUS

+0 −12
Original line number Diff line number Diff line
@@ -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 ]

modules/dav/main/mod_dav.c

+14 −13
Original line number Diff line number Diff line
@@ -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);

}

modules/dav/main/mod_dav.h

+3 −2
Original line number Diff line number Diff line
@@ -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