From bae26cee5f31b4e831eb17a25bc943918c05309f Mon Sep 17 00:00:00 2001 From: Graham Leggett Date: Sun, 26 May 2013 20:07:04 +0000 Subject: [PATCH] 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 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 --- CHANGES | 3 +++ STATUS | 6 ------ include/ap_mmn.h | 9 +++++---- modules/dav/main/mod_dav.c | 31 +++++++++++++++---------------- modules/dav/main/mod_dav.h | 15 +++++++++++++++ modules/dav/main/util.c | 24 ++++++++++++++++++++++++ 6 files changed, 62 insertions(+), 26 deletions(-) diff --git a/CHANGES b/CHANGES index 1fa885dd8b..a9ed1ec32d 100644 --- 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 ] + *) 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 1e2f3a9605..aa7e36d4bb 100644 --- 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 b054ab4d77..7087fba9a2 100644 --- 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 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. +** +** is the error stack that the error will be added to. +** +** 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) { -- 2.50.1