You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2020/06/28 13:28:19 UTC
svn commit: r1879307 - /httpd/httpd/trunk/modules/dav/main/mod_dav.c
Author: minfrin
Date: Sun Jun 28 13:28:19 2020
New Revision: 1879307
URL: http://svn.apache.org/viewvc?rev=1879307&view=rev
Log:
Add implementation of deliver_report and gather_reports to mod_dav.c.
Modified:
httpd/httpd/trunk/modules/dav/main/mod_dav.c
Modified: httpd/httpd/trunk/modules/dav/main/mod_dav.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/mod_dav.c?rev=1879307&r1=1879306&r2=1879307&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/dav/main/mod_dav.c (original)
+++ httpd/httpd/trunk/modules/dav/main/mod_dav.c Sun Jun 28 13:28:19 2020
@@ -1491,92 +1491,95 @@ static dav_error *dav_gen_supported_live
return err;
}
+
/* generate DAV:supported-report-set OPTIONS response */
static dav_error *dav_gen_supported_reports(request_rec *r,
const dav_resource *resource,
const apr_xml_elem *elem,
- const dav_hooks_vsn *vsn_hooks,
apr_text_header *body)
{
apr_xml_elem *child;
apr_xml_attr *attr;
- dav_error *err;
+ dav_error *err = NULL;
char *s;
+ apr_array_header_t *reports;
+ const dav_report_elem *rp;
apr_text_append(r->pool, body, "<D:supported-report-set>" DEBUG_CR);
- if (vsn_hooks != NULL) {
- const dav_report_elem *reports;
- const dav_report_elem *rp;
+ reports = apr_array_make(r->pool, 5, sizeof(const char *));
+ dav_run_gather_reports(r, resource, reports, &err);
+ if (err != NULL) {
+ return dav_push_error(r->pool, err->status, 0,
+ "DAV:supported-report-set could not be "
+ "determined due to a problem fetching the "
+ "available reports for this resource.",
+ err);
+ }
+
+ if (elem->first_child == NULL) {
+ int i;
+
+ /* show all supported reports */
+ rp = (const dav_report_elem *)reports->elts;
+ for (i = 0; i < reports->nelts; i++, rp++) {
+ /* Note: we presume reports->namespace is
+ * properly XML/URL quoted */
+ s = apr_pstrcat(r->pool,
+ "<D:supported-report D:name=\"",
+ rp->name,
+ "\" D:namespace=\"",
+ rp->nmspace,
+ "\"/>" DEBUG_CR, NULL);
+ apr_text_append(r->pool, body, s);
+ }
+ }
+ else {
+ /* check for support of specific report */
+ for (child = elem->first_child; child != NULL; child = child->next) {
+ if (child->ns == APR_XML_NS_DAV_ID
+ && strcmp(child->name, "supported-report") == 0) {
+ const char *name = NULL;
+ const char *nmspace = NULL;
+ int i;
+
+ /* go through attributes to find name and namespace */
+ for (attr = child->attr; attr != NULL; attr = attr->next) {
+ if (attr->ns == APR_XML_NS_DAV_ID) {
+ if (strcmp(attr->name, "name") == 0)
+ name = attr->value;
+ else if (strcmp(attr->name, "namespace") == 0)
+ nmspace = attr->value;
+ }
+ }
- if ((err = (*vsn_hooks->avail_reports)(resource, &reports)) != NULL) {
- return dav_push_error(r->pool, err->status, 0,
- "DAV:supported-report-set could not be "
- "determined due to a problem fetching the "
- "available reports for this resource.",
- err);
- }
-
- if (reports != NULL) {
- if (elem->first_child == NULL) {
- /* show all supported reports */
- for (rp = reports; rp->nmspace != NULL; ++rp) {
- /* Note: we presume reports->namespace is
- * properly XML/URL quoted */
- s = apr_pstrcat(r->pool,
- "<D:supported-report D:name=\"",
- rp->name,
- "\" D:namespace=\"",
- rp->nmspace,
- "\"/>" DEBUG_CR, NULL);
- apr_text_append(r->pool, body, s);
+ if (name == NULL) {
+ return dav_new_error(r->pool, HTTP_BAD_REQUEST, 0, 0,
+ "A DAV:supported-report element "
+ "does not have a \"name\" attribute");
}
- }
- else {
- /* check for support of specific report */
- for (child = elem->first_child; child != NULL; child = child->next) {
- if (child->ns == APR_XML_NS_DAV_ID
- && strcmp(child->name, "supported-report") == 0) {
- const char *name = NULL;
- const char *nmspace = NULL;
-
- /* go through attributes to find name and namespace */
- for (attr = child->attr; attr != NULL; attr = attr->next) {
- if (attr->ns == APR_XML_NS_DAV_ID) {
- if (strcmp(attr->name, "name") == 0)
- name = attr->value;
- else if (strcmp(attr->name, "namespace") == 0)
- nmspace = attr->value;
- }
- }
-
- if (name == NULL) {
- return dav_new_error(r->pool, HTTP_BAD_REQUEST, 0, 0,
- "A DAV:supported-report element "
- "does not have a \"name\" attribute");
- }
-
- /* default namespace to DAV: */
- if (nmspace == NULL)
- nmspace = "DAV:";
-
- for (rp = reports; rp->nmspace != NULL; ++rp) {
- if (strcmp(name, rp->name) == 0
- && strcmp(nmspace, rp->nmspace) == 0) {
- /* Note: we presume reports->nmspace is
- * properly XML/URL quoted
- */
- s = apr_pstrcat(r->pool,
- "<D:supported-report "
- "D:name=\"",
- rp->name,
- "\" D:namespace=\"",
- rp->nmspace,
- "\"/>" DEBUG_CR, NULL);
- apr_text_append(r->pool, body, s);
- break;
- }
- }
+
+ /* default namespace to DAV: */
+ if (nmspace == NULL) {
+ nmspace = "DAV:";
+ }
+
+ rp = (const dav_report_elem *)reports->elts;
+ for (i = 0; i < reports->nelts; i++, rp++) {
+ if (strcmp(name, rp->name) == 0
+ && strcmp(nmspace, rp->nmspace) == 0) {
+ /* Note: we presume reports->nmspace is
+ * properly XML/URL quoted
+ */
+ s = apr_pstrcat(r->pool,
+ "<D:supported-report "
+ "D:name=\"",
+ rp->name,
+ "\" D:namespace=\"",
+ rp->nmspace,
+ "\"/>" DEBUG_CR, NULL);
+ apr_text_append(r->pool, body, s);
+ break;
}
}
}
@@ -1946,7 +1949,7 @@ static int dav_method_options(request_re
core_option = 1;
}
else if (strcmp(elem->name, "supported-report-set") == 0) {
- err = dav_gen_supported_reports(r, resource, elem, vsn_hooks, &body);
+ err = dav_gen_supported_reports(r, resource, elem, &body);
core_option = 1;
}
}
@@ -4186,21 +4189,60 @@ static int dav_method_label(request_rec
return DONE;
}
+static int dav_core_deliver_report(request_rec *r,
+ const dav_resource *resource,
+ const apr_xml_doc *doc,
+ ap_filter_t *output, dav_error **err)
+{
+ const dav_hooks_vsn *vsn_hooks = DAV_GET_HOOKS_VSN(r);
+
+ if (vsn_hooks) {
+ *err = (*vsn_hooks->deliver_report)(r, resource, doc,
+ r->output_filters);
+ return OK;
+ }
+
+ return DECLINED;
+}
+
+static void dav_core_gather_reports(
+ request_rec *r,
+ const dav_resource *resource,
+ apr_array_header_t *reports,
+ dav_error **err)
+{
+ const dav_hooks_vsn *vsn_hooks = DAV_GET_HOOKS_VSN(r);
+
+ if (vsn_hooks) {
+ const dav_report_elem *rp;
+
+ (*err) = (*vsn_hooks->avail_reports)(resource, &rp);
+ while (rp && rp->name) {
+
+ dav_report_elem *report = apr_array_push(reports);
+
+ report->nmspace = rp->nmspace;
+ report->name = rp->name;
+
+ rp++;
+ }
+ }
+
+}
+
static int dav_method_report(request_rec *r)
{
dav_resource *resource;
const dav_hooks_vsn *vsn_hooks = DAV_GET_HOOKS_VSN(r);
- int result;
- int label_allowed;
apr_xml_doc *doc;
- dav_error *err;
+ dav_error *err = NULL;
- /* If no versioning provider, decline the request */
- if (vsn_hooks == NULL)
- return DECLINED;
+ int result;
+ int label_allowed;
- if ((result = ap_xml_parse_input(r, &doc)) != OK)
+ if ((result = ap_xml_parse_input(r, &doc)) != OK) {
return result;
+ }
if (doc == NULL) {
/* This supplies additional information for the default msg. */
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00614)
@@ -4212,11 +4254,12 @@ static int dav_method_report(request_rec
* First determine whether a Target-Selector header is allowed
* for this report.
*/
- label_allowed = (*vsn_hooks->report_label_header_allowed)(doc);
+ label_allowed = vsn_hooks ? (*vsn_hooks->report_label_header_allowed)(doc) : 0;
err = dav_get_resource(r, label_allowed, 0 /* use_checked_in */,
&resource);
- if (err != NULL)
+ if (err != NULL) {
return dav_handle_err(r, err, NULL);
+ }
if (!resource->exists) {
/* Apache will supply a default error for this. */
@@ -4228,24 +4271,36 @@ static int dav_method_report(request_rec
ap_set_content_type(r, DAV_XML_CONTENT_TYPE);
/* run report hook */
- if ((err = (*vsn_hooks->deliver_report)(r, resource, doc,
- r->output_filters)) != NULL) {
- if (! r->sent_bodyct)
- /* No data has been sent to client yet; throw normal error. */
- return dav_handle_err(r, err, NULL);
-
- /* If an error occurred during the report delivery, there's
- basically nothing we can do but abort the connection and
- log an error. This is one of the limitations of HTTP; it
- needs to "know" the entire status of the response before
- generating it, which is just impossible in these streamy
- response situations. */
- err = dav_push_error(r->pool, err->status, 0,
- "Provider encountered an error while streaming"
- " a REPORT response.", err);
- dav_log_err(r, err, APLOG_ERR);
- r->connection->aborted = 1;
+ result = dav_run_deliver_report(r, resource, doc,
+ r->output_filters, &err);
+ switch (result) {
+ case OK:
return DONE;
+ case DECLINED:
+ /* No one handled the report */
+ return HTTP_NOT_IMPLEMENTED;
+ default:
+ if ((err) != NULL) {
+
+ if (! r->sent_bodyct) {
+ /* No data has been sent to client yet; throw normal error. */
+ return dav_handle_err(r, err, NULL);
+ }
+
+ /* If an error occurred during the report delivery, there's
+ basically nothing we can do but abort the connection and
+ log an error. This is one of the limitations of HTTP; it
+ needs to "know" the entire status of the response before
+ generating it, which is just impossible in these streamy
+ response situations. */
+ err = dav_push_error(r->pool, err->status, 0,
+ "Provider encountered an error while streaming"
+ " a REPORT response.", err);
+ dav_log_err(r, err, APLOG_ERR);
+ r->connection->aborted = 1;
+
+ return DONE;
+ }
}
return DONE;
@@ -4963,6 +5018,11 @@ static void register_hooks(apr_pool_t *p
dav_hook_insert_all_liveprops(dav_core_insert_all_liveprops,
NULL, NULL, APR_HOOK_MIDDLE);
+ dav_hook_deliver_report(dav_core_deliver_report,
+ NULL, NULL, APR_HOOK_LAST);
+ dav_hook_gather_reports(dav_core_gather_reports,
+ NULL, NULL, APR_HOOK_LAST);
+
dav_core_register_uris(p);
}
Re: svn commit: r1879307 -
/httpd/httpd/trunk/modules/dav/main/mod_dav.c
Posted by Graham Leggett <mi...@sharp.fm>.
On 29 Jun 2020, at 12:10, Ruediger Pluem <rp...@apache.org> wrote:
>> + result = dav_run_deliver_report(r, resource, doc,
>> + r->output_filters, &err);
>> + switch (result) {
>> + case OK:
>> return DONE;
>
> What happens if err != NULL. Should we handle that here like in the default case or
> should the hook implementer ensure to not return OK if err != NULL?
Hmmm… this needs tightening up.
>> + case DECLINED:
>> + /* No one handled the report */
>> + return HTTP_NOT_IMPLEMENTED;
>
> Previously we were returning DECLINED in case there was no vsn_hooks and thus nothing to report. Now we return
> HTTP_NOT_IMPLEMENTED. Why this change?
When mod_dav was written, it was assumed that the REPORT method was exclusive to https://tools.ietf.org/html/rfc3253. As a result, if a versioning implementation was present, the versioning implementation (eg svn) was expected to handle REPORT, and no other.
Other WebDAV extensions arrived, and they also used REPORT. But if a versioning implementation was present, that implementation would consume the report, not recognise the report, and then return an error. The mod_caldav code that Nokia worked on in the late 2000s hacked around this by reading the REPORT body first, then creating an input filter to re-insert the body if mod_caldav realised the REPORT wasn’t for it. Parsing XML bodies over and over is very ugly.
What this change does is formally recognise in code that multiple RFCs handle the REPORT method. We parse the XML body, then offer this parsed body to anyone who wants it via the deliver_report hook. Modules get their chance to handle the report. As a last resort, the mod_dav core passes the body to the versioning implementaton (eg svn) which then responds as previous, maintaining existing behaviour.
To answer you direct question above, at the point we return HTTP_NOT_IMPLEMENTED we have consumed the REPORT body. We can’t return DECLINED in this case.
There are a significant number of RFCs that mod_dav doesn't support, such as https://tools.ietf.org/html/rfc5689. We have RFC compliance work to do.
Regards,
Graham
—
Re: svn commit: r1879307 -
/httpd/httpd/trunk/modules/dav/main/mod_dav.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 6/28/20 3:28 PM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Sun Jun 28 13:28:19 2020
> New Revision: 1879307
>
> URL: http://svn.apache.org/viewvc?rev=1879307&view=rev
> Log:
> Add implementation of deliver_report and gather_reports to mod_dav.c.
>
> Modified:
> httpd/httpd/trunk/modules/dav/main/mod_dav.c
>
> Modified: httpd/httpd/trunk/modules/dav/main/mod_dav.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/mod_dav.c?rev=1879307&r1=1879306&r2=1879307&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/dav/main/mod_dav.c (original)
> +++ httpd/httpd/trunk/modules/dav/main/mod_dav.c Sun Jun 28 13:28:19 2020
> @@ -4228,24 +4271,36 @@ static int dav_method_report(request_rec
> ap_set_content_type(r, DAV_XML_CONTENT_TYPE);
>
> /* run report hook */
> - if ((err = (*vsn_hooks->deliver_report)(r, resource, doc,
> - r->output_filters)) != NULL) {
> - if (! r->sent_bodyct)
> - /* No data has been sent to client yet; throw normal error. */
> - return dav_handle_err(r, err, NULL);
> -
> - /* If an error occurred during the report delivery, there's
> - basically nothing we can do but abort the connection and
> - log an error. This is one of the limitations of HTTP; it
> - needs to "know" the entire status of the response before
> - generating it, which is just impossible in these streamy
> - response situations. */
> - err = dav_push_error(r->pool, err->status, 0,
> - "Provider encountered an error while streaming"
> - " a REPORT response.", err);
> - dav_log_err(r, err, APLOG_ERR);
> - r->connection->aborted = 1;
> + result = dav_run_deliver_report(r, resource, doc,
> + r->output_filters, &err);
> + switch (result) {
> + case OK:
> return DONE;
What happens if err != NULL. Should we handle that here like in the default case or
should the hook implementer ensure to not return OK if err != NULL?
> + case DECLINED:
> + /* No one handled the report */
> + return HTTP_NOT_IMPLEMENTED;
Previously we were returning DECLINED in case there was no vsn_hooks and thus nothing to report. Now we return
HTTP_NOT_IMPLEMENTED. Why this change?
> + default:
> + if ((err) != NULL) {
> +
> + if (! r->sent_bodyct) {
> + /* No data has been sent to client yet; throw normal error. */
> + return dav_handle_err(r, err, NULL);
> + }
> +
> + /* If an error occurred during the report delivery, there's
> + basically nothing we can do but abort the connection and
> + log an error. This is one of the limitations of HTTP; it
> + needs to "know" the entire status of the response before
> + generating it, which is just impossible in these streamy
> + response situations. */
> + err = dav_push_error(r->pool, err->status, 0,
> + "Provider encountered an error while streaming"
> + " a REPORT response.", err);
> + dav_log_err(r, err, APLOG_ERR);
> + r->connection->aborted = 1;
> +
> + return DONE;
> + }
> }
>
> return DONE;
Regards
Rüdiger