Hi,
For atomic-revprop, the client needs to know if the error was
SVN_ERR_BAD_OLD_VALUE or not. For ra_local and ra_svn this is
already done; this patch does it for DAV (with Neon).
With mod_dav, we only have 2 ways to affect the 207 multistatus
response from a failed PROPPATCH:
- We can set the text that appears in <D:responsedescription>,
including injecting arbitrary XML.
- We can set the <D:status> value to a (numeric) HTTP error code.
The approach that's been discussed on-list and on IRC was to
inject the error as XML inside <D:responsedescription>. I did
actually implement that, only to discover that Neon completely
rejects the XML in that case[1]. While we would obviously fix
this in the 1.7 client code, it could break compatibility between
the 1.7 server and 1.6 and older clients. The only way to handle
this would be to introduce a new client capability, so the server
could fall back to the old code for 1.6 clients. This means we'd
have two code paths for error-handling... this gets very complex.
However, there is a simpler way! The <D:status> element contains
the HTTP error code, usually "500 Internal Server Error". So we
could pick a special HTTP status code to mean SVN_ERR_BAD_OLD_VALUE.
I think of old_value_p as a precondition for the operation, a bit
like "If-Modified-Since:", so I'd suggest "412 Precondition Failed".
Note that generic DAV clients won't ever see this message, because
they won't be sending old_value_p.
This patch only does mod_dav_svn and Neon. I don't want to waste
time doing both HTTP libraries if this patch is rejected. (Also,
Serf is slightly harder because it doesn't parse <D:status> yet).
The patch is against the atomic-revprop branch, and requires
Patch 1 to be applied first. (It does apply to trunk, too).
[[[
Signal SVN_ERR_BAD_OLD_VALUE over DAV using a HTTP 412 status
code inside a 207 multi-status response.
* subversion/mod_dav_svn/util.c:
(dav_svn__convert_err): Map SVN_ERR_BAD_OLD_VALUE to 412.
* subversion/libsvn_ra_neon/util.c:
(multistatus_baton_t): New member contains_precondition_error.
(end_207_element): Check for HTTP 412 status code and create
a SVN_ERR_BAD_OLD_VALUE error if found.
Patch by: Jon Foster <jon.fos...@cabot.co.uk>
]]]
Kind regards,
Jon
[1] More precisely, libsvn_ra_neon/util.c:wrapper_startelm_cb()
raises a SVN_ERR_XML_MALFORMED "XML data was not well-formed"
error if <D:responsedescription> contains any XML tags. See
the ELEM_responsedescription row in multistatus_nesting_table[]
in the same file - it explicitly states that if there's any
XML tags inside <D:responsedescription> then the XML is invalid.
**********************************************************************
This email and its attachments may be confidential and are intended solely for
the use of the individual to whom it is addressed. Any views or opinions
expressed are solely those of the author and do not necessarily represent those
of Cabot Communications Ltd.
If you are not the intended recipient of this email and its attachments, you
must take no action based upon them, nor must you copy or show them to anyone.
Cabot Communications Limited
Verona House, Filwood Road, Bristol BS16 3RY, UK
+44 (0) 1179584232
Co. Registered in England number 02817269
Please contact the sender if you believe you have received this email in error.
**********************************************************************
______________________________________________________________________
This email has been scanned by the MessageLabs Email Security System.
For more information please visit http://www.messagelabs.com/email
______________________________________________________________________Index: subversion/mod_dav_svn/util.c
===================================================================
--- subversion/mod_dav_svn/util.c (revision 998620)
+++ subversion/mod_dav_svn/util.c (working copy)
@@ -107,6 +107,9 @@
case SVN_ERR_FS_PATH_ALREADY_LOCKED:
status = HTTP_LOCKED;
break;
+ case SVN_ERR_BAD_OLD_VALUE:
+ status = HTTP_PRECONDITION_FAILED;
+ break;
/* add other mappings here */
}
Index: subversion/libsvn_ra_neon/util.c
===================================================================
--- subversion/libsvn_ra_neon/util.c (revision 998620)
+++ subversion/libsvn_ra_neon/util.c (working copy)
@@ -167,6 +167,7 @@
svn_ra_neon__request_t *req;
svn_stringbuf_t *description;
svn_boolean_t contains_error;
+ svn_boolean_t contains_precondition_error;
} multistatus_baton_t;
/* Implements svn_ra_neon__startelm_cb_t. */
@@ -231,9 +232,12 @@
return svn_error_create(SVN_ERR_RA_DAV_REQUEST_FAILED, NULL,
_("The request response contained at least
"
"one error"));
+ else if (b->contains_precondition_error)
+ return svn_error_create(SVN_ERR_BAD_OLD_VALUE, NULL,
+ b->description->data);
else
return svn_error_create(SVN_ERR_RA_DAV_REQUEST_FAILED, NULL,
- b->description->data);
+ *b->description->data);
}
break;
@@ -260,6 +264,10 @@
else
b->propstat_has_error = (status.klass != 2);
+ /* Handle "412 Precondition Failed" specially */
+ if (status.code == 412)
+ b->contains_precondition_error = TRUE;
+
free(status.reason_phrase);
}
else
