diff options
| -rw-r--r-- | modules/dav/fs/repos.c | 23 | ||||
| -rw-r--r-- | modules/dav/main/mod_dav.c | 173 | ||||
| -rw-r--r-- | modules/dav/main/mod_dav.h | 37 | ||||
| -rw-r--r-- | modules/dav/main/util.c | 15 | ||||
| -rw-r--r-- | modules/dav/main/util_lock.c | 18 |
5 files changed, 158 insertions, 108 deletions
diff --git a/modules/dav/fs/repos.c b/modules/dav/fs/repos.c index 68070351b9..a6a102ee24 100644 --- a/modules/dav/fs/repos.c +++ b/modules/dav/fs/repos.c @@ -591,11 +591,12 @@ static dav_error *dav_fs_deleteset(apr_pool_t *p, const dav_resource *resource) ** REPOSITORY HOOK FUNCTIONS */ -static dav_resource * dav_fs_get_resource( +static dav_error * dav_fs_get_resource( request_rec *r, const char *root_dir, const char *target, - int is_label) + int is_label, + dav_resource **result_resource) { dav_resource_private *ctx; dav_resource *resource; @@ -679,7 +680,10 @@ static dav_resource * dav_fs_get_resource( ** be in path_info. The resource is simply an error: it ** can't be a null or a locknull resource. */ - return NULL; /* becomes HTTP_NOT_FOUND */ + return dav_new_error(r->pool, HTTP_BAD_REQUEST, 0, + "The URL contains extraneous path " + "components. The resource could not " + "be identified."); } /* retain proper integrity across the structures */ @@ -689,10 +693,12 @@ static dav_resource * dav_fs_get_resource( } } - return resource; + *result_resource = resource; + return NULL; } -static dav_resource * dav_fs_get_parent_resource(const dav_resource *resource) +static dav_error * dav_fs_get_parent_resource(const dav_resource *resource, + dav_resource **result_parent) { dav_resource_private *ctx = resource->info; dav_resource_private *parent_ctx; @@ -706,8 +712,10 @@ static dav_resource * dav_fs_get_parent_resource(const dav_resource *resource) #else strcmp(ctx->pathname, "/") == 0 #endif - ) + ) { + *result_parent = NULL; return NULL; + } /* ### optimize this into a single allocation! */ @@ -740,7 +748,8 @@ static dav_resource * dav_fs_get_parent_resource(const dav_resource *resource) parent_resource->exists = 1; } - return parent_resource; + *result_parent = parent_resource; + return NULL; } static int dav_fs_is_same_resource( diff --git a/modules/dav/main/mod_dav.c b/modules/dav/main/mod_dav.c index 49d4102f0b..9e71897bf2 100644 --- a/modules/dav/main/mod_dav.c +++ b/modules/dav/main/mod_dav.c @@ -618,39 +618,54 @@ static int dav_get_overwrite(request_rec *r) * by either a DAV:version or DAV:label-name element (passed as * the target argument), or any Target-Selector header in the request. */ -static int dav_get_resource(request_rec *r, int target_allowed, - ap_xml_elem *target, dav_resource **res_p) +static dav_error * dav_get_resource(request_rec *r, int target_allowed, + ap_xml_elem *target, dav_resource **res_p) { void *data; dav_dir_conf *conf; const char *target_selector = NULL; int is_label = 0; int result; + dav_error *err; - /* go look for the resource if it isn't already present */ + /* only look for the resource if it isn't already present */ (void) apr_get_userdata(&data, DAV_KEY_RESOURCE, r->pool); if (data != NULL) { *res_p = data; - return OK; + return NULL; } /* if the request target can be overridden, get any target selector */ if (target_allowed) { + /* ### this should return a dav_error* */ if ((result = dav_get_target_selector(r, target, &target_selector, &is_label)) != OK) - return result; + return dav_new_error(r->pool, result, 0, + "Could not process the method target."); } conf = ap_get_module_config(r->per_dir_config, &dav_module); /* assert: conf->provider != NULL */ /* resolve the resource */ - *res_p = (*conf->provider->repos->get_resource)(r, conf->dir, - target_selector, is_label); + err = (*conf->provider->repos->get_resource)(r, conf->dir, + target_selector, is_label, + res_p); + if (err != NULL) { + err = dav_push_error(r->pool, HTTP_INTERNAL_SERVER_ERROR, 0, + "Could not fetch resource information.", err); + return err; + } + + /* Note: this shouldn't happen, but just be sure... */ if (*res_p == NULL) { - /* Apache will supply a default error for this. */ - return HTTP_NOT_FOUND; + /* ### maybe use HTTP_INTERNAL_SERVER_ERROR */ + return dav_new_error(r->pool, HTTP_NOT_FOUND, 0, + apr_psprintf(r->pool, + "The provider did not define a " + "resource for %s.", + ap_escape_html(r->pool, r->uri))); } (void) apr_set_userdata(*res_p, DAV_KEY_RESOURCE, apr_null_cleanup, @@ -661,7 +676,7 @@ static int dav_get_resource(request_rec *r, int target_allowed, * add it now */ dav_add_vary_header(r, r, *res_p); - return OK; + return NULL; } static dav_error * dav_open_lockdb(request_rec *r, int ro, dav_lockdb **lockdb) @@ -715,14 +730,15 @@ static int dav_method_get(request_rec *r) { dav_resource *resource; int result; + dav_error *err; /* This method should only be called when the resource is not * visible to Apache. We will fetch the resource from the repository, * then create a subrequest for Apache to handle. */ - result = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); if (!resource->exists) { /* Apache will supply a default error for this. */ return HTTP_NOT_FOUND; @@ -912,13 +928,11 @@ static int dav_method_post(request_rec *r) { dav_resource *resource; dav_error *err; - int result; /* Ask repository module to resolve the resource */ - result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); - if (result != OK) { - return result; - } + err = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); /* Note: depth == 0. Implies no need for a multistatus response. */ if ((err = dav_validate_request(r, resource, 0, NULL, NULL, @@ -953,10 +967,9 @@ static int dav_method_put(request_rec *r) } /* Ask repository module to resolve the resource */ - result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); - if (result != OK) { - return result; - } + err = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); /* If not a file or collection resource, PUT not allowed */ if (resource->type != DAV_RESOURCE_TYPE_REGULAR) { @@ -1168,9 +1181,9 @@ static int dav_method_delete(request_rec *r) } /* Ask repository module to resolve the resource */ - result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); if (!resource->exists) { /* Apache will supply a default error for this. */ return HTTP_NOT_FOUND; @@ -1522,11 +1535,12 @@ static int dav_method_options(request_rec *r) apr_array_header_t *uri_ary; ap_xml_doc *doc; const ap_xml_elem *elem; + dav_error *err; /* resolve the resource */ - result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); /* parse any request body */ if ((result = ap_xml_parse_input(r, &doc)) != OK) { @@ -1872,9 +1886,9 @@ static int dav_method_propfind(request_rec *r) dav_response *multi_status; /* Ask repository module to resolve the resource */ - result = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); if (dav_get_resource_state(r, resource) == DAV_RESOURCE_NULL) { /* Apache will supply a default error for this. */ @@ -2136,9 +2150,9 @@ static int dav_method_proppatch(request_rec *r) dav_prop_ctx *ctx; /* Ask repository module to resolve the resource */ - result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); if (!resource->exists) { /* Apache will supply a default error for this. */ return HTTP_NOT_FOUND; @@ -2336,9 +2350,9 @@ static int dav_method_mkcol(request_rec *r) &dav_module); /* Ask repository module to resolve the resource */ - result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); if (resource->exists) { /* oops. something was already there! */ @@ -2455,9 +2469,9 @@ static int dav_method_copymove(request_rec *r, int is_move) int resource_state; /* Ask repository module to resolve the resource */ - result = dav_get_resource(r, !is_move /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, !is_move /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); if (!resource->exists) { /* Apache will supply a default error for this. */ return HTTP_NOT_FOUND; @@ -2509,9 +2523,9 @@ static int dav_method_copymove(request_rec *r, int is_move) } /* Resolve destination resource */ - result = dav_get_resource(lookup.rnew, 0 /*target_allowed*/, NULL, &resnew); - if (result != OK) - return result; + err = dav_get_resource(lookup.rnew, 0 /*target_allowed*/, NULL, &resnew); + if (err != NULL) + return dav_handle_err(r, err, NULL); /* are the two resources handled by the same repository? */ if (resource->hooks != resnew->hooks) { @@ -2838,9 +2852,9 @@ static int dav_method_lock(request_rec *r) * so allow it, and let provider reject the lock attempt * on a version if it wants to. */ - result = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); /* ** Open writable. Unless an error occurs, we'll be @@ -3025,9 +3039,9 @@ static int dav_method_unlock(request_rec *r) * so allow it, and let provider reject the unlock attempt * on a version if it wants to. */ - result = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); resource_state = dav_get_resource_state(r, resource); @@ -3083,9 +3097,9 @@ static int dav_method_vsn_control(request_rec *r) return DECLINED; /* ask repository module to resolve the resource */ - result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); /* remember the pre-creation resource state */ resource_state = dav_get_resource_state(r, resource); @@ -3284,9 +3298,9 @@ static int dav_method_checkout(request_rec *r) } /* Ask repository module to resolve the resource */ - result = dav_get_resource(r, 1 /*target_allowed*/, target, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 1 /*target_allowed*/, target, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); if (!resource->exists) { /* Apache will supply a default error for this. */ return HTTP_NOT_FOUND; @@ -3349,9 +3363,9 @@ static int dav_method_uncheckout(request_rec *r) } /* Ask repository module to resolve the resource */ - result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); if (!resource->exists) { /* Apache will supply a default error for this. */ return HTTP_NOT_FOUND; @@ -3412,9 +3426,9 @@ static int dav_method_checkin(request_rec *r) } /* Ask repository module to resolve the resource */ - result = dav_get_resource(r, 0 /* target_allowed */, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 0 /* target_allowed */, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); if (!resource->exists) { /* Apache will supply a default error for this. */ return HTTP_NOT_FOUND; @@ -3518,9 +3532,9 @@ static int dav_method_set_target(request_rec *r) return DECLINED; /* Ask repository module to resolve the resource */ - result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); if (!resource->exists) { /* Apache will supply a default error for this. */ return HTTP_NOT_FOUND; @@ -3700,9 +3714,9 @@ static int dav_method_label(request_rec *r) return DECLINED; /* Ask repository module to resolve the resource */ - result = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); if (!resource->exists) { /* Apache will supply a default error for this. */ return HTTP_NOT_FOUND; @@ -3835,9 +3849,9 @@ static int dav_method_report(request_rec *r) * for this report. */ target_allowed = (*vsn_hooks->report_target_selector_allowed)(doc); - result = dav_get_resource(r, target_allowed, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, target_allowed, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); if (!resource->exists) { /* Apache will supply a default error for this. */ return HTTP_NOT_FOUND; @@ -3882,9 +3896,9 @@ static int dav_method_make_workspace(request_rec *r) return DECLINED; /* ask repository module to resolve the resource */ - result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); /* parse the request body (must be a mkworkspace element) */ if ((result = ap_xml_parse_input(r, &doc)) != OK) { @@ -3961,16 +3975,15 @@ static int dav_method_bind(request_rec *r) dav_response *multi_response = NULL; dav_lookup_result lookup; int overwrite; - int result; /* If no bindings provider, decline the request */ if (binding_hooks == NULL) return DECLINED; /* Ask repository module to resolve the resource */ - result = dav_get_resource(r, 0 /*!target_allowed*/, NULL, &resource); - if (result != OK) - return result; + err = dav_get_resource(r, 0 /*!target_allowed*/, NULL, &resource); + if (err != NULL) + return dav_handle_err(r, err, NULL); if (!resource->exists) { /* Apache will supply a default error for this. */ return HTTP_NOT_FOUND; @@ -4015,9 +4028,9 @@ static int dav_method_bind(request_rec *r) } /* resolve binding resource */ - result = dav_get_resource(lookup.rnew, 0 /*!target_allowed*/, NULL, &binding); - if (result != OK) - return result; + err = dav_get_resource(lookup.rnew, 0 /*!target_allowed*/, NULL, &binding); + if (err != NULL) + return dav_handle_err(r, err, NULL); /* are the two resources handled by the same repository? */ if (resource->hooks != binding->hooks) { diff --git a/modules/dav/main/mod_dav.h b/modules/dav/main/mod_dav.h index 6d553692d8..22902c9cf7 100644 --- a/modules/dav/main/mod_dav.h +++ b/modules/dav/main/mod_dav.h @@ -1566,32 +1566,41 @@ struct dav_hooks_repository */ int handle_get; - /* Get a resource descriptor for the URI in a request. - * A descriptor is returned even if the resource does not exist. - * The return value should only be NULL for some kind of fatal error. + /* Get a resource descriptor for the URI in a request. A descriptor + * should always be returned even if the resource does not exist. This + * repository has been identified as handling the resource given by + * the URI, so an answer must be given. If there is a problem with the + * URI or accessing the resource or whatever, then an error should be + * returned. * - * The root_dir is the root of the directory for which this repository - * is configured. - * The target is either a label, or a version URI, or NULL. If there - * is a target, then is_label specifies whether the target is a label - * or a URI. + * root_dir: the root of the directory for which this repository is + * configured. + * target: is either a label, or a version URI, or NULL. If there is + * a target, then is_label specifies whether the target is a + * label or a URI. * - * The provider may associate the request storage pool with the resource, - * to use in other operations on that resource. + * The provider may associate the request storage pool with the resource + * (in the resource->pool field), to use in other operations on that + * resource. */ - dav_resource * (*get_resource)( + dav_error * (*get_resource)( request_rec *r, const char *root_dir, const char *target, - int is_label + int is_label, + dav_resource **resource ); /* Get a resource descriptor for the parent of the given resource. * The resources need not exist. NULL is returned if the resource * is the root collection. + * + * An error should be returned only if there is a fatal error in + * fetching information about the parent resource. */ - dav_resource * (*get_parent_resource)( - const dav_resource *resource + dav_error * (*get_parent_resource)( + const dav_resource *resource, + dav_resource **parent_resource ); /* Determine whether two resource descriptors refer to the same resource. diff --git a/modules/dav/main/util.c b/modules/dav/main/util.c index 9aa6a4f237..51ca37a343 100644 --- a/modules/dav/main/util.c +++ b/modules/dav/main/util.c @@ -1345,13 +1345,15 @@ dav_error * dav_validate_request(request_rec *r, dav_resource *resource, /* (2) Validate the parent resource if requested */ if (err == NULL && (flags & DAV_VALIDATE_PARENT)) { - dav_resource *parent_resource = (*repos_hooks->get_parent_resource)(resource); + dav_resource *parent_resource; - if (parent_resource == NULL) { + err = (*repos_hooks->get_parent_resource)(resource, &parent_resource); + + if (err == NULL && parent_resource == NULL) { err = dav_new_error(r->pool, HTTP_FORBIDDEN, 0, "Cannot access parent of repository root."); } - else { + else if (err == NULL) { err = dav_validate_resource_state(r->pool, parent_resource, lockdb, if_header, flags | DAV_VALIDATE_IS_PARENT, @@ -1603,7 +1605,12 @@ dav_error *dav_ensure_resource_writable(request_rec *r, /* check parent resource if requested or if resource must be created */ if (!resource->exists || parent_only) { - dav_resource *parent = (*resource->hooks->get_parent_resource)(resource); + dav_resource *parent; + + if ((err = (*resource->hooks->get_parent_resource)(resource, + &parent)) != NULL) + return err; + if (parent == NULL || !parent->exists) { body = apr_psprintf(r->pool, "Missing one or more intermediate collections. " diff --git a/modules/dav/main/util_lock.c b/modules/dav/main/util_lock.c index 3a138dd16f..cd54fac2fd 100644 --- a/modules/dav/main/util_lock.c +++ b/modules/dav/main/util_lock.c @@ -460,6 +460,7 @@ static dav_error * dav_get_direct_resource(apr_pool_t *p, while (resource != NULL) { dav_error *err; dav_lock *lock; + dav_resource *parent; /* ** Find the lock specified by <locktoken> on <resource>. If it is @@ -488,7 +489,12 @@ static dav_error * dav_get_direct_resource(apr_pool_t *p, } /* the lock was indirect. move up a level in the URL namespace */ - resource = (*resource->hooks->get_parent_resource)(resource); + if ((err = (*resource->hooks->get_parent_resource)(resource, + &parent)) != NULL) { + /* ### add a higher-level desc? */ + return err; + } + resource = parent; } return dav_new_error(p, HTTP_INTERNAL_SERVER_ERROR, 0, @@ -625,14 +631,20 @@ static dav_error * dav_inherit_locks(request_rec *r, dav_lockdb *lockdb, dav_response *multi_status; if (use_parent) { - which_resource = (*repos_hooks->get_parent_resource)(resource); - if (which_resource == NULL) { + dav_resource *parent; + if ((err = (*repos_hooks->get_parent_resource)(resource, + &parent)) != NULL) { + /* ### add a higher-level desc? */ + return err; + } + if (parent == NULL) { /* ### map result to something nice; log an error */ return dav_new_error(r->pool, HTTP_INTERNAL_SERVER_ERROR, 0, "Could not fetch parent resource. Unable to " "inherit locks from the parent and apply " "them to this resource."); } + which_resource = parent; } else { which_resource = resource; |