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

dav_join_error() function.

PR: 54145

Submitted by: Ben Reser <ben reser.org>
Reviewed by: trawick

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

diff --git a/CHANGES b/CHANGES
index 37b9bf5eae349c98b573c56d4dc90a7dc3066e9e..070de15a626678ab49231c00d2eea72e9ea13754 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_dav: Improve error handling in dav_method_put(), add new
+     dav_join_error() function.  PR 54145.  [Ben Reser <ben reser.org>]
+
   *) mod_auth_digest: Fix crashes if shm initialization failed. [Stefan
      Fritsch]
  

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index b6c29cb3d3f5fa5ced54177576f9c871e749c486..2db39c108b685c12f249c09a2473d80108d55333 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -419,6 +419,7 @@
  * 20121222.6 (2.5.0-dev)  Add ap_proxy_create_hdrbrgd() and
  *                         ap_proxy_pass_brigade()
  * 20121222.7 (2.5.0-dev)  Add ap_remove_input|output_filter_byhandle()
+ * 20121222.8 (2.5.0-dev)  Add dav_join_error()
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
@@ -426,7 +427,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20121222
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 7                   /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 8                   /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

diff --git a/modules/dav/main/mod_dav.c b/modules/dav/main/mod_dav.c
index b5059175ca31a4020144ae483695bea407e94ddb..f1882a97e7ef93aec48eb6f746ad3c1c1242bdc3 100644 (file)
--- a/modules/dav/main/mod_dav.c
+++ b/modules/dav/main/mod_dav.c
@@ -995,8 +995,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;
@@ -1018,18 +1018,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);
                 }
             }
@@ -1041,10 +1042,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);
     }
 
     /*
@@ -1062,6 +1060,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 768638c37949362737945d9d7f18e7bd0c5a38d6..7b91b63cf2cd0990ed6d279642ffb7b34f5e3802 100644 (file)
--- 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 aa0858410295f438e23370e255c7d1fcc2165dc4..2f4ce8d8cfeb8e26943a510dade9d133669750bc 100644 (file)
--- 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)
 {