[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH 3/3] atomic-revprop: Signal the error as a HTTP status code

From: Jon Foster <Jon.Foster_at_cabot.co.uk>
Date: Mon, 20 Sep 2010 11:01:02 +0100


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.foster_at_cabot.co.uk>

Kind regards,


[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

Received on 2010-09-20 12:01:41 CEST

This is an archived mail posted to the Subversion Dev mailing list.