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

diff --git a/CHANGES b/CHANGES
index 1fa885dd8b72ac6594a39ba53414426cddd7e507..a9ed1ec32d149a2eafa7582426424e20989eefdc 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,9 @@
 
 Changes with Apache 2.4.5
 
+  *) mod_dav: Improve error handling in dav_method_put(), add new
+     dav_join_error() function.  PR 54145.  [Ben Reser <ben reser.org>]
+
   *) mod_dav: Sending a MERGE request against a URI handled by mod_dav_svn with
      the source href (sent as part of the request body as XML) pointing to a
      URI that is not configured for DAV will trigger a segfault. [Ben Reser

diff --git a/STATUS b/STATUS
index 1e2f3a9605e25859370d2490c3aa7a23b4d2384d..aa7e36d4bb9c87bb571d8fab408efbba26f8871f 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -90,12 +90,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
   
-    * 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
-      +1: minfrin, jim, jorton
-
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index b054ab4d77e21e6e807a34ef3a05e09e85681aff..7087fba9a2c84ee9469818ba9c1da27cb5c01d0d 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -401,9 +401,10 @@
  * 20120211.9 (2.4.4-dev)  Add fgrab() to ap_slotmem_provider_t.
  * 20120211.10 (2.4.4-dev) Add in bal_persist field to proxy_server_conf
  * 20120211.11 (2.4.4-dev) Add ap_bin2hex()
- * 20120211.12 (2.4.5-dev)  Add ap_remove_input|output_filter_byhandle()
- * 20120211.13 (2.4.5-dev)  Add ap_get_exec_line
- * 20120211.14 (2.4.5-dev)  Add ppinherit and inherit to proxy_server_conf
+ * 20120211.12 (2.4.5-dev) Add ap_remove_input|output_filter_byhandle()
+ * 20120211.13 (2.4.5-dev) Add ap_get_exec_line
+ * 20120211.14 (2.4.5-dev) Add ppinherit and inherit to proxy_server_conf
+ * 20120211.15 (2.4.5-dev) Add dav_join_error()
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
@@ -411,7 +412,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20120211
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 14                   /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 15                   /* 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 0f8886a323edf720825e7980c8e77f7a662f439e..9135cd96067105eb66161804e0ac2ceafec07761 100644 (file)
--- 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 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 ca82f9c54f1738b11a41f05f41ef074079e95d3c..743bedb43c09f9b60a819d3c09568f36f8ec339f 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)
 {