So, Ben, Fitz and I dove into this, and saw that the is_xml_comment
flag in the lock was being clobbered. Then a light bulb went on in my
head. When I dropped svn_fs_attach_lock() in favor of a single
locking API (svn_fs_lock()), I failed to notice that not all the lock
fields (which would have made it into the repository by virtue of
being present in the lock passed in svn_fs_attach_lock()) were
represented as parameters to svn_fs_lock(). Specifically, there is no
'is_xml_comment' parameter to match the relevant lock field. The
result is that is_xml_comment is just lost in the ether between
mod_dav_svn and libsvn_repos/libsvn_fs. And that means that we're
doing extra escaping on a value that shouldn't be re-escaped.
That would be ... uh ... my fault. :-(
Ben Collins-Sussman <sussman@collab.net> writes:
> I think there's a latent bug in DAV autoversioning, one I hope to
> isolate and fix today. But I thought I'd alert more people to it...
> more eyes to see it, etc.
>
> Refresher:
>
> In the DAV universe, locks have a <DAV:owner> field, which actually
> has nothing to do with authn or authz; it's just a comment that the
> client expects the server to preserve. In fact, mod_dav_svn maps
> <DAV:owner> to svn_lock_t->comment.
>
> The problem with this is that if you do a lock with an svn client
> over ra_dav, a bunch of XML junk ends up being stored in svn_lock_t-
> >comment (due to mod_dav's design), which violates our principle of
> any RA layer leaving a nasty footprint behind.
>
> So our previous solution was to make mod_dav_svn notice whether the
> incoming LOCK request is coming from svn, or from a generic DAV
> client. In the former case, we strip the XML junk off of the comment
> and store the lock. In the latter case, we leave the XML junk in
> place, but set a flag reminding us it's there.
>
> When retrieving locks, we look at the flag. If not present, we re-
> add proper XML wrapping so as to create a compliant response. If the
> flag is set, we do nothing, because the XML wrapping is already there.
>
> All of this code is in mod_dav_svn/lock.c, functions
> svn_lock_to_dav_lock() and dav_lock_to_svn_lock().
>
>
> Bug:
>
> I'm looking at a Dreamweaver http session, and it's the first DAV
> client I've seen which sets the <DAV:owner> field. It's setting the
> value to "<DAV:href>blah</DAV:href>". Later on, when it does a
> PROPFIND to retrieve the lock, the value isn't coming back identical
> -- it's been accidentally xml-unescaped. (i.e.
> "<DAV:href>blah...") Dreameweaver then chokes and refuses to
> interoperate with mod_dav_svn.
>
>
> Impact of Bug:
>
> I think that any generic DAV client which sets the <DAV:owner> field
> is likely to fail against mod_dav_svn. This is serious enough that I
> might argue it's worth rolling an rc4 tarball for, delaying the final
> release by a day or two. After all, autoversioning is supposed to be
> one of the big new important features in 1.2.
>
>
> Reproducing:
>
> Does anyone know a free DAV client that allows me to set <DAV:owner>
> in a LOCK request? Cadaver won't let me do it, and I don't think
> Windows Webfolders or OSX does it either. Maybe I'll just do a
> 'netcat' simulation.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue May 10 19:54:57 2005