Re: svn commit: r10190 - trunk/subversion/libsvn_ra_dav

On Thu, Jul 08, 2004 at 07:03:50PM -0500, breser@tigris.org wrote:
> Author: breser
> Date: Thu Jul 8 19:03:44 2004
> New Revision: 10190
>
> Modified:
> trunk/subversion/libsvn_ra_dav/fetch.c
> Log:
> Allow clients to fetch revprops (other than svn: ones) that contain
> a colon in their name with propget. Issue #1807.
>
> * subversion/libsvn_ra_dav/fetch.c
> (make_ne_propname): No longer used, remove this function.
> (svn_ra_dav__change_rev_prop): Call svn_ra_dav__change_rev_proplist() and
> grab the result from the hash.

This commit, fixes Issue #1807. But the solution is singificantly
different than we originally envisioned. I did not actually solve the
XML namespace problem. I simply avoided the one case where it actually
caused an error.

So why not fix the namespace issue entirely:

a) The only possible solution is to encode properties like so;

For foo:bar it would be encoded as:
<P:bar xmlns:P="http://subversion.tigris.org/xmlns/customproperty/foo">

This looks great until you think about properties that end in a colon,
where for foo: you'd get:

<P: xmlns:P="http://subversion.tigirs.org/xmlns/customproperty/foo">

Which obviously is not valid.

b) The error case that 1807 covers is a propget on a revprop. Which
issues a PROPFIND. The error that results is a result of the invalid
XML in the body of the PROPFIND request. Which means the client needs
to know if the server can accept the new namespace format before it
sends the request.
 
One possible option would be to add something to the OPTIONS response.
However, there are many cases where we do not do an OPTIONS request
before doing a PROPFIND. So this wouldn't work.

The other alternative would be to try the new method and if it fails,
try the old method. However, doing the new method on an old server will
result in the following error message being printed to the error_log:
[Thu Jul 08 17:56:17 2004] [error] [client 127.0.0.1] Properties may
only be defined in the http://subversion.tigris.org/xmlns/svn/ and
http://subversion.tigris.org/xmlns/custom/ namespaces. [409, #0]

Which is downright ugly. Additionally we do gobs of PROPFIND requests.
Which means a new client against an old server would in many cases
increase the number of requests substantially. Reversing the order
would remove the error message, but would penalize new clients running
against new servers.

c) All of this would substantially complicate our code.

Considering the costs of the above, the solution I committed in r10190
has the following benefits above and beyond that:

a) Very simple.

b) Backportable.

c) Works for all permitted property names.

While it is true that it increases the cost of doing a propget on a
revprop to include fetching all the properties, not just the one you
want. It only does so when using the DAV access method. Additionally,
DAV is already doing this when asking for versioned properties.

I tested coloned properties against libneon built with libxml and
expat with this fix. Neither of them errored out on any operation. It
is possible that at some point in the future they'll do something to
break this.

But given the costs of a real solution and the benefits of the simpler
solution, I think it's better to punt this problem until and if it
becomes an issue.

-- 
Ben Reser <ben@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
For additional commands, e-mail: svn-help@subversion.tigris.org
!DSPAM:40edeab5239266592474!
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org