mod_dav: Improve error handling in dav_method_put(), add new

dav_join_error() function.  PR 54145.

trunk patch: http://svn.apache.org/r1464241
2.4.x patch: http://people.apache.org/~minfrin/httpd-mod_dav-errorhandling.patch
Submitted by: Ben Reser <ben reser.org>
Reviewed by: minfrin, jim, jorton


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

3 files changed, 54 insertions, 16 deletions

diff --git a/modules/dav/main/mod_dav.c b/modules/dav/main/mod_dav.c
index 0f8886a323..9135cd9606 100644
--- a/modules/dav/main/mod_dav.c
+++ b/modules/dav/main/mod_dav.c
@@ -1003,8 +1003,8 @@ static int dav_method_put(request_rec *r)
                 else {
                     /* XXX: should this actually be HTTP_BAD_REQUEST? */
                     http_err = HTTP_INTERNAL_SERVER_ERROR;
-                    msg = apr_psprintf(r->pool, "Could not get next bucket "
-                                       "brigade (URI: %s)", msg);
+                    msg = apr_psprintf(r->pool, "An error occurred while reading"
+                                       " the request body (URI: %s)", msg);
                 }
                 err = dav_new_error(r->pool, http_err, 0, rc, msg);
                 break;
@@ -1026,18 +1026,19 @@ static int dav_method_put(request_rec *r)
                     continue;
                 }
 
-                rc = apr_bucket_read(b, &data, &len, APR_BLOCK_READ);
-                if (rc != APR_SUCCESS) {
-                    err = dav_new_error(r->pool, HTTP_BAD_REQUEST, 0, rc,
-                                        apr_psprintf(r->pool,
-                                                    "An error occurred while reading"
-                                                    " the request body (URI: %s)",
-                                                    ap_escape_html(r->pool, r->uri)));
-                    break;
-                }
-
                 if (err == NULL) {
                     /* write whatever we read, until we see an error */
+                    rc = apr_bucket_read(b, &data, &len, APR_BLOCK_READ);
+                    if (rc != APR_SUCCESS) {
+                       err = dav_new_error(r->pool, HTTP_BAD_REQUEST, 0, rc,
+                                           apr_psprintf(r->pool,
+                                                        "An error occurred while"
+                                                        " reading the request body"
+                                                        " from the bucket (URI: %s)",
+                                                        ap_escape_html(r->pool, r->uri)));
+                        break;
+                    }
+
                     err = (*resource->hooks->write_stream)(stream, data, len);
                 }
             }
@@ -1049,10 +1050,7 @@ static int dav_method_put(request_rec *r)
 
         err2 = (*resource->hooks->close_stream)(stream,
                                                 err == NULL /* commit */);
-        if (err2 != NULL && err == NULL) {
-            /* no error during the write, but we hit one at close. use it. */
-            err = err2;
-        }
+        err = dav_join_error(err, err2);
     }
 
     /*
@@ -1070,6 +1068,7 @@ static int dav_method_put(request_rec *r)
 
     /* check for errors now */
     if (err != NULL) {
+        err = dav_join_error(err, err2); /* don't forget err2 */
         return dav_handle_err(r, err, NULL);
     }
 
diff --git a/modules/dav/main/mod_dav.h b/modules/dav/main/mod_dav.h
index 768638c379..7b91b63cf2 100644
--- a/modules/dav/main/mod_dav.h
+++ b/modules/dav/main/mod_dav.h
@@ -169,6 +169,21 @@ DAV_DECLARE(dav_error*) dav_push_error(apr_pool_t *p, int status, int error_id,
                                        const char *desc, dav_error *prev);
 
 
+/*
+** Join two errors together.
+**
+** This function is used to add a new error stack onto an existing error so
+** that subsequent errors can be reported after the first error.  It returns
+** the correct error stack to use so that the caller can blindly call it
+** without checking that both dest and src are not NULL.
+** 
+** <dest> is the error stack that the error will be added to.
+**
+** <src> is the error stack that will be appended.
+*/
+DAV_DECLARE(dav_error*) dav_join_error(dav_error* dest, dav_error* src);
+
+
 /* error ID values... */
 
 /* IF: header errors */
diff --git a/modules/dav/main/util.c b/modules/dav/main/util.c
index ca82f9c54f..743bedb43c 100644
--- a/modules/dav/main/util.c
+++ b/modules/dav/main/util.c
@@ -77,6 +77,30 @@ DAV_DECLARE(dav_error*) dav_push_error(apr_pool_t *p, int status,
     return err;
 }
 
+DAV_DECLARE(dav_error*) dav_join_error(dav_error *dest, dav_error *src)
+{
+    dav_error *curr = dest;
+
+    /* src error doesn't exist so nothing to join just return dest */
+    if (src == NULL) {
+        return dest;
+    }
+
+    /* dest error doesn't exist so nothing to join just return src */
+    if (curr == NULL) {
+        return src;
+    }
+
+    /* find last error in dest stack */
+    while (curr->prev != NULL) {
+        curr = curr->prev;
+    }
+
+    /* add the src error onto end of dest stack and return it */
+    curr->prev = src;
+    return dest;
+}
+
 DAV_DECLARE(void) dav_check_bufsize(apr_pool_t * p, dav_buffer *pbuf,
                                     apr_size_t extra_needed)
 {