use bucket brigades directly when reading PUT data. This avoids

problems with content-length-modifying input filter (like deflate).

revision 1.99:
drop the test, whether apr_brigade_create returns NULL. It does never
(dumps core instead :-)

Submitted by: Cliff Woolley

revision 1.100:
get rid of _FOREACH
Submitted by: Cliff Woolley

(Needed 1.100 too as APR_BRIGADE_FOREACH is now gone in APR-util 1.0+.)

Backport of modules/dav/main/mod_dav.c r1.98, r1.99, r1.100
PR: 22104
Some stuff submitted by: tim@robbins.dropbear.id.au (Tim Robbins)
Reviewed by:	Andr� Malo, Jeff Trawick, Justin Erenkrantz

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

diff --git a/CHANGES b/CHANGES
index 40272e389910beaaf4e24860d76798bb58d3780e..191bbc5ada66131e2cfd5e61e8b625025606be42 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,9 @@
 Changes with Apache 2.0.49
 
+  *) mod_dav: Use bucket brigades when reading PUT data. This avoids
+     problems if the data stream is modified by an input filter. PR 22104.
+     [Tim Robbins <tim robbins.dropbear.id.au>, André Malo]
+
   *) Fix RewriteBase directive to not add double slashes.  [André Malo]
 
   *) Improve 'configure --help' output for some modules.  [Astrid Keßler]

diff --git a/modules/dav/main/mod_dav.c b/modules/dav/main/mod_dav.c
index e1e87dbee7d83db200802fae331edbc2d453b151..06043a94681d672b97442bdf31a5504afa6ea2e2 100644 (file)
--- a/modules/dav/main/mod_dav.c
+++ b/modules/dav/main/mod_dav.c
@@ -896,7 +896,6 @@ static int dav_method_put(request_rec *r)
     const char *body;
     dav_error *err;
     dav_error *err2;
-    int result;
     dav_stream_mode mode;
     dav_stream *stream;
     dav_response *multi_response;
@@ -904,10 +903,6 @@ static int dav_method_put(request_rec *r)
     apr_off_t range_start;
     apr_off_t range_end;
 
-    if ((result = ap_setup_client_block(r, REQUEST_CHUNKED_DECHUNK)) != OK) {
-        return result;
-    }
-
     /* Ask repository module to resolve the resource */
     err = dav_get_resource(r, 0 /* label_allowed */, 0 /* use_checked_in */,
                            &resource);
@@ -982,40 +977,58 @@ static int dav_method_put(request_rec *r)
     }
 
     if (err == NULL) {
-        if (ap_should_client_block(r)) {
-            char *buffer = apr_palloc(r->pool, DAV_READ_BLOCKSIZE);
-            long len;
-
-            /*
-             * Once we start reading the request, then we must read the
-             * whole darn thing. ap_discard_request_body() won't do anything
-             * for a partially-read request.
-             */
+        apr_bucket_brigade *bb;
+        apr_bucket *b;
+        int seen_eos = 0;
+
+        bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
 
-            while ((len = ap_get_client_block(r, buffer,
-                                              DAV_READ_BLOCKSIZE)) > 0) {
-                   if (err == NULL) {
-                       /* write whatever we read, until we see an error */
-                       err = (*resource->hooks->write_stream)(stream,
-                                                              buffer, len);
-                   }
+        do {
+            apr_status_t rc;
+
+            rc = ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES,
+                                APR_BLOCK_READ, DAV_READ_BLOCKSIZE);
+
+            if (rc != APR_SUCCESS) {
+                err = dav_new_error(r->pool, HTTP_INTERNAL_SERVER_ERROR, 0,
+                                    "Could not get next bucket brigade");
+                break;
             }
 
-            /*
-             * ### what happens if we read more/less than the amount
-             * ### specified in the Content-Range? eek...
-             */
+            for (b = APR_BRIGADE_FIRST(bb);
+                 b != APR_BRIGADE_SENTINEL(bb);
+                 b = APR_BUCKET_NEXT(b))
+            {
+                const char *data;
+                apr_size_t len;
+
+                if (APR_BUCKET_IS_EOS(b)) {
+                    seen_eos = 1;
+                    break;
+                }
+
+                if (APR_BUCKET_IS_METADATA(b)) {
+                    continue;
+                }
 
-            if (len == -1) {
-                /*
-                 * Error reading request body. This has precedence over
-                 * prior errors.
-                 */
-                err = dav_new_error(r->pool, HTTP_BAD_REQUEST, 0,
-                                    "An error occurred while reading the "
-                                    "request body.");
+                rc = apr_bucket_read(b, &data, &len, APR_BLOCK_READ);
+                if (rc != APR_SUCCESS) {
+                    err = dav_new_error(r->pool, HTTP_BAD_REQUEST, 0,
+                                        "An error occurred while reading "
+                                        "the request body.");
+                    break;
+                }
+
+                if (err == NULL) {
+                    /* write whatever we read, until we see an error */
+                    err = (*resource->hooks->write_stream)(stream, data, len);
+                }
             }
-        }
+
+            apr_brigade_cleanup(bb);
+        } while (!seen_eos);
+
+        apr_brigade_destroy(bb);
 
         err2 = (*resource->hooks->close_stream)(stream,
                                                 err == NULL /* commit */);