You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ben Collins-Sussman <su...@collab.net> on 2005/02/14 16:51:25 UTC

locking: serious RA interoperability problems.

Hey folks, remember the discussion I brought up last month, regarding
the fact that mod_dav pushes extra XML junk to mod_dav_svn when it hands
down a lock object?  There was some chat between me, gstein, and
Julian Reschke, and we came up with a scheme to deal with this.

But after some discussion with kfogel and cmpilato, it seems that the
problem is much nastier than I suspected.  I want to describe the
problem (and potential solution) here.  Developers, please read and
comment on this.  It's about general RA and locking design... no
DAV-specific knowledge is required.

The Goal:

We want the new locking feature to be well-designed in its own right,
but we also want it to be interoperable with generic DAV clients, so
that autoversioning really gets finished.  Up till now, we've been
able to balance this tension pretty well.  We're close to being done.

The Problem:

The DAV spec requires the server to preserve *all* information
surrounding a DAV-lock's "author" field, which we map to
svn_lock_t->comment.  So by default, mod_dav passes this field to
mod_dav_svn wrapped up in the original <D:author> xml tag, and
mod_dav_svn is expected to save it and retrieve it verbatim, as a big
opaque block.  The theory is that a DAV client like MSWord might
create extra attributes on the tag, or put all sorts of custom XML
data within the tag's cdata.  mod_dav must blindly preserve all that,
with no attempt at interpretation.

So, there are actually *two* problems right now.

1. mod_dav always passes an incoming lock structure to mod_dav_svn
    with the lock->comment field "wrapped" in XML.  As a result, when
    other RA layers (or svnlook/svnserve) examine the repository, they
    see this extra junk in the lock structure, and even marshall the
    noise back to the client.

    (This was the original problem we pointed out last month.)

2. When a non-dav RA layer stores a lock structure in the repository,
    the lock->comment field has no XML wrapping.  Later on, mod_dav
    reads the lock and sends it to some DAV client (svn or otherwise),
    but the HTTP response is invalid, because there's no XML wrapping
    around the field!  mod_dav doesn't realize that it needs to add XML
    wrapping.

So this is a case of an RA layer having "special needs" that poke into
the general svn design, and causes it to be non-interoperable with
other RA layers.


Proposed Solution:

The Chicago office came up with several ideas, but this one seemed to
be the least nasty and intrusive.

   A. Add a new boolean field to the svn_lock_t structure.  It would be
      named "xml-packaged-comment", or something similar.  It defaults to
      0, and if set, it means that lock->comment is packaged in XML.

   B. When writing locks to the repository:

      1.  svnserve (and other non-dav writers) continue normally,
          creating repository locks with the new boolean==0.  They
          effectively ignore the new field.

      2.  mod_dav_svn notices whether the incoming lock was created by
          an svn client or a generic DAV client.  (There are many ways
          to do this, and they've already been discussed elsewhere.)
          If the lock comes from an svn client, the XML-packaging is
          stripped and the lock is written with the boolean==0.  if the
          lock comes from a generic DAV client, the packaging is
          preserved and the lock is written with the boolean==1.

   C. When reading locks from the repository:

      1.  svnserve (and other non-dav readers) continue to read locks
          normally, ignoring the new boolean field.  If extra XML junk
          ends up being marshalled back to the client, that simply
          means the lock was created by a generic DAV client.  No big
          deal.

      2.  mod_dav_svn notices the boolean field when reading.  If 0, it
          re-adds the necessary XML wrapper.  If 1, it sends the data 
as-is.


I'll let this idea sit out there for a day or two, so we can get some
nice feedback.  But this really isn't something we can punt.  I used
to think that this problem was a mere annoyance: "ew, extra dav junk
is visible through svnserve."  But it's a lot worse than that.  It's
now the case that svnserve is creating locks in the repository that
ra_dav *cannot* fetch and parse!


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by John Peacock <jp...@rowman.com>.
Ben Collins-Sussman wrote:

> 
> Exactly, that's point C.2. in the proposed solution.
> 

Thanks for explaining it slowly for me. ;-)

John

-- 
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4501 Forbes Boulevard
Suite H
Lanham, MD  20706
301-459-3366 x.5010
fax 301-429-5748

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by Ben Collins-Sussman <su...@collab.net>.
On Feb 14, 2005, at 11:17 AM, John Peacock wrote:
>
>> So yeah, I guess we're just trying to minimize code here.  Normal RA 
>> layers ignore the new boolean field.  mod_dav_svn pays careful 
>> attention to it, as a means of staying interoperable with the rest of 
>> svn.
>
> And then the reverse case would be to just wrap the non-DAV comment in 
> <D:author something> here, even though that data isn't _useful_, but 
> it is at least "well formed" and the XML parser doesn't freak out.
>

Exactly, that's point C.2. in the proposed solution.



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by John Peacock <jp...@rowman.com>.
Ben Collins-Sussman wrote:

> svnserve certainly *could*, but I'm not sure it would be worth it.

Fair enough, I was just asking. ;-)

> So yeah, I guess we're just trying to minimize code here.  Normal RA 
> layers ignore the new boolean field.  mod_dav_svn pays careful attention 
> to it, as a means of staying interoperable with the rest of svn.

And then the reverse case would be to just wrap the non-DAV comment in 
<D:author something> here, even though that data isn't _useful_, but it 
is at least "well formed" and the XML parser doesn't freak out.

John

-- 
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4501 Forbes Boulevard
Suite H
Lanham, MD  20706
301-459-3366 x.5010
fax 301-429-5748

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by Ben Collins-Sussman <su...@collab.net>.
On Feb 14, 2005, at 11:01 AM, John Peacock wrote:

> Ben Collins-Sussman wrote:
>
>>   C. When reading locks from the repository:
>>      1.  svnserve (and other non-dav readers) continue to read locks
>>          normally, ignoring the new boolean field.  If extra XML junk
>>          ends up being marshalled back to the client, that simply
>>          means the lock was created by a generic DAV client.  No big
>>          deal.
>
> Is there some reason why the boolean == 1 cannot mean that svnserve 
> strips the XML wrapper before marshalling the data back to the client? 
> Or is this just about fixing mod_dav_svn to DTRT to minimize the 
> amount of code that has to care about <D:author>?
>

svnserve certainly *could*, but I'm not sure it would be worth it.

For example, if MSWord creates a lock with a comment that looks like

<D:author foo="bar" aoeuth="crpylwh">
   <tag1>
    <tag2>
      ... 37 other complicated tags full of attributes and weird cdata 
...
    </tag2>
   <tag1>
</D:author>


Then the most svnserve could do is strip the <D:author> stuff when 
sending the lock back to the client.  It's not really going to make the 
comment much more readable.  :-)

So yeah, I guess we're just trying to minimize code here.  Normal RA 
layers ignore the new boolean field.  mod_dav_svn pays careful 
attention to it, as a means of staying interoperable with the rest of 
svn.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by John Peacock <jp...@rowman.com>.
Ben Collins-Sussman wrote:

>   C. When reading locks from the repository:
> 
>      1.  svnserve (and other non-dav readers) continue to read locks
>          normally, ignoring the new boolean field.  If extra XML junk
>          ends up being marshalled back to the client, that simply
>          means the lock was created by a generic DAV client.  No big
>          deal.

Is there some reason why the boolean == 1 cannot mean that svnserve 
strips the XML wrapper before marshalling the data back to the client? 
Or is this just about fixing mod_dav_svn to DTRT to minimize the amount 
of code that has to care about <D:author>?

John

-- 
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4501 Forbes Boulevard
Suite H
Lanham, MD  20706
301-459-3366 x.5010
fax 301-429-5748

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by Julian Reschke <ju...@gmx.de>.
Greg Hudson wrote:
> On Mon, 2005-02-14 at 11:51, Ben Collins-Sussman wrote:
> 
>>      2.  mod_dav_svn notices whether the incoming lock was created by
>>          an svn client or a generic DAV client.
> 
> 
> I'd rather see us test whether the author field is minimal or not.  If
> it looks like <D:author>blah</D:author>, it would be nice to unpackage
> it regardless of whether the client is a generic DAV client.  My theory
> is that most DAV clients will send a minimal author spec, and it would
> be nice not to display their lock fields as junk to non-dav clients.
> 
> On another note, if the author field is using namespace tags which were
> defined elsewhere, how can we be sure those namespace tags are defined
> the same way in the response?  How does the DAV spec address this
> problem?

It doesn't.

As far as I can tell, the consensus is that *placement* of namespace 
declarations should never matter as long the XML Infoset is kept intact.

Best regards, Julian

-- 
<green/>bytes GmbH -- http://www.greenbytes.de -- tel:+492512807760

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by Ben Collins-Sussman <su...@collab.net>.
On Feb 14, 2005, at 2:28 PM, Peter N. Lundblad wrote:
>  OK, this doesn't work if an svn client passes <D:owner
> ...>blah</D:owner> as the comment (wrapped in another D:owner for
> marshalling), but would that be a problem in practice?
>

Probably not, but we need to be resilient.  We shouldn't design 
loopholes.  This is exactly the scenario that kfogel brought up that 
made me realize we need the boolean flag.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Mon, 14 Feb 2005, Greg Stein wrote:

> On Mon, Feb 14, 2005 at 01:05:40PM -0500, Greg Hudson wrote:
> > On Mon, 2005-02-14 at 11:51, Ben Collins-Sussman wrote:
> > >       2.  mod_dav_svn notices whether the incoming lock was created by
> > >           an svn client or a generic DAV client.
> >
> > I'd rather see us test whether the author field is minimal or not.  If
> > it looks like <D:author>blah</D:author>, it would be nice to unpackage
> > it regardless of whether the client is a generic DAV client.  My theory
> > is that most DAV clients will send a minimal author spec, and it would
> > be nice not to display their lock fields as junk to non-dav clients.
>
> Agreed. This is/was the original solution that Ben and I worked thru via
> AIM. We came up with an idea to detect "minimal" and do the stripping.
>
> I think the current problem is that if you're looking at a lock->comment,
> then how does mod_dav_svn determine whether it was stripped (and needs
> wrapping), or should be passed along unchanged?
>
> A simple detection rule might be, "is the first character '<' ?" But what
> if a comment can legitimately contain that? Then you could end up a
> DAV:owner tag wrapped around some content that it shouldn't have wrapped.
>
A more sophisticated solution would be to try to parse the lock->comment
as XML. If it is well-formed and consists of an <D:owner> element with
arbitrary contents and attributes, just pass it through. Else wrap it in
<D:owner>. OK, this doesn't work if an svn client passes <D:owner
...>blah</D:owner> as the comment (wrapped in another D:owner for
marshalling), but would that be a problem in practice?

This is so obvious, I must be missing something fundamental...

//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Feb 14, 2005 at 01:05:40PM -0500, Greg Hudson wrote:
> On Mon, 2005-02-14 at 11:51, Ben Collins-Sussman wrote:
> >       2.  mod_dav_svn notices whether the incoming lock was created by
> >           an svn client or a generic DAV client.
> 
> I'd rather see us test whether the author field is minimal or not.  If
> it looks like <D:author>blah</D:author>, it would be nice to unpackage
> it regardless of whether the client is a generic DAV client.  My theory
> is that most DAV clients will send a minimal author spec, and it would
> be nice not to display their lock fields as junk to non-dav clients.

Agreed. This is/was the original solution that Ben and I worked thru via
AIM. We came up with an idea to detect "minimal" and do the stripping.

I think the current problem is that if you're looking at a lock->comment,
then how does mod_dav_svn determine whether it was stripped (and needs
wrapping), or should be passed along unchanged?

A simple detection rule might be, "is the first character '<' ?" But what
if a comment can legitimately contain that? Then you could end up a
DAV:owner tag wrapped around some content that it shouldn't have wrapped.

> On another note, if the author field is using namespace tags which were
> defined elsewhere, how can we be sure those namespace tags are defined
> the same way in the response?  How does the DAV spec address this
> problem?

mod_dav passes a serialized XML fragment which is internally consistent
with the namespaces/prefixes. It was designed this way so that the result
could simply be dropped into the output XML stream without worrying about
a bunch of namespace crap (as you rightly were concerned about).


In any case, I think the question comes down to: is there a detection rule
that is workable for "should this be wrapped?" A heuristic is available;
is that good enough? If not, then a boolean as Ben suggests is a good way
out of the problem.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by Greg Hudson <gh...@MIT.EDU>.
On Mon, 2005-02-14 at 11:51, Ben Collins-Sussman wrote:
>       2.  mod_dav_svn notices whether the incoming lock was created by
>           an svn client or a generic DAV client.

I'd rather see us test whether the author field is minimal or not.  If
it looks like <D:author>blah</D:author>, it would be nice to unpackage
it regardless of whether the client is a generic DAV client.  My theory
is that most DAV clients will send a minimal author spec, and it would
be nice not to display their lock fields as junk to non-dav clients.

On another note, if the author field is using namespace tags which were
defined elsewhere, how can we be sure those namespace tags are defined
the same way in the response?  How does the DAV spec address this
problem?


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by Ben Collins-Sussman <su...@collab.net>.
On Feb 17, 2005, at 5:38 PM, Julian Foad wrote:

> Ben Collins-Sussman wrote:
>> We want the new locking feature to be well-designed in its own right,
>> but we also want it to be interoperable with generic DAV clients, so
>> that autoversioning really gets finished.
>
> This is my reading of the situation:
>
> The locking system stores and retrieves a piece of opaque data that it 
> calls the "comment".  Generic DAV clients need a DAV "owner" field 
> stored in this way, and Subversion clients need a human-readable 
> comment stored.  A Subversion RA-DAV client is an overlap of the two, 
> and the comment is in a DAV "owner" field.
>
> Ignoring the XML wrapping for a moment, and tinking about the 
> information content, DAV "owner" and Subversion "comment" are not 
> compatible with each other (neither type of client can understand the 
> other type of comment).

This is really an exaggeration.  *Semantically*, they're the same 
thing.  They're both just 'comments' describing the lock.  They're both 
meant to be human-readable.

The problem I ran into was the fact that mod_dav is trying to store the 
thing in a funny way, with some extra syntax.


> Therefore we need to store meta-data that says either "This is a 
> Subversion comment" or "This is a (non-Subversion) DAV owner field", 
> and we need to send the comment to the client only when the comment 
> type matches the client type.
>

Not at all.  Any generic DAV client may examine the lock, and will very 
much want to see a human-readable comment created by an svn client.  
And similarly, if a generic DAV client creates a lock, 'svn info URL' 
will want to display the comment it creates.

It's *possible* that some DAV clients may store structured data in the 
comment, but not guaranteed, and not even that commonly.  And when that 
happens, it's not as though we have some obligation to screen the svn 
client's eyes from that.  If the svn client sees it, it's no big deal:  
"hey, I guess program <foo> made that lock."


>
> My assumptions:
>
> + It is possible to divide the clients into distinct non-overlapping 
> types based on the form of their "comment" data.  I think there are 
> two types: Subversion, and (non-Subversion) DAV.

It's possible, but entirely easy.  I've solved the problem by making 
the svn client send a special http header in the LOCK request which 
says, "hey, I'm an svn client!"


>
> + It is possible to recognise which type of client we are talking to, 
> at all relevant times.

Only because of the header above, and only when locking.  There's no 
easy way to tell in a PROPFIND, though.


>
> + A non-Subversion DAV client never needs or wants to see a Subversion 
> "comment".
>

Not true.  I have a java DAV client right here that happily displays 
the locks created by svn.  It's quite pleasant to see all the fields, 
especially the comment.

>
> If I'm right, then the new meta-data should mean "This comment is a 
> (Subversion comment | non-Subversion DAV owner field)", rather than 
> "This comment is [not] valid XML".  It should always be set correctly. 
>  (Is there any problem with getting RA-local etc. to set it?)  It 
> should possibly be a multi-valued setting rather than a Boolean, so 
> that we can add other comment types in future.
>
> Strip the XML from Subversion comments RA-DAV, but leave it intact for 
> generic DAV owner fields, like you said.

This is in fact what I've already committed, essentially.  The new 
boolean is defined as "is the comment wrapped with extra mod_dav XML?"  
The only time mod_dav_svn stores the extra XML wrapper (and sets the 
boolean) is when it knows the lock creation is coming from a non-svn 
client.

>
> On reading, I recommend that all RA methods take note of the "comment 
> type" field and don't read the comment at all if it's not the type 
> they want.

Nope, the comment is always a good thing.  There's a good semantic 
match between DAV and SVN locks here.  The trick is just to make sure 
that mod_dav_svn creates lock-comments that are always usable by other 
RA layers, and that it always sends back valid HTTP to all clients.  
This burden is now entirely implemented within mod_dav_svn, and no 
other library or layer needs to take notice.  The problem is cleanly 
solved.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by Julian Foad <ju...@btopenworld.com>.
Ben Collins-Sussman wrote:
> We want the new locking feature to be well-designed in its own right,
> but we also want it to be interoperable with generic DAV clients, so
> that autoversioning really gets finished.

This is my reading of the situation:

The locking system stores and retrieves a piece of opaque data that it calls 
the "comment".  Generic DAV clients need a DAV "owner" field stored in this 
way, and Subversion clients need a human-readable comment stored.  A Subversion 
RA-DAV client is an overlap of the two, and the comment is in a DAV "owner" field.

Ignoring the XML wrapping for a moment, and tinking about the information 
content, DAV "owner" and Subversion "comment" are not compatible with each 
other (neither type of client can understand the other type of comment). 
Therefore we need to store meta-data that says either "This is a Subversion 
comment" or "This is a (non-Subversion) DAV owner field", and we need to send 
the comment to the client only when the comment type matches the client type.


My assumptions:

+ It is possible to divide the clients into distinct non-overlapping types 
based on the form of their "comment" data.  I think there are two types: 
Subversion, and (non-Subversion) DAV.

+ It is possible to recognise which type of client we are talking to, at all 
relevant times.

+ A non-Subversion DAV client never needs or wants to see a Subversion "comment".

+ A Subversion client never needs to see a DAV "comment".  (The only reason it 
might want to is to display it for diagnostic puposes, in the hope that a human 
might recognise something in it.  I'm assuming that either we can live without 
this, or we can always send the "comment" to Subversion clients and they can 
decide whether to display it, if they know by some means whether it is a 
Subversion or a DAV comment.)


If I'm right, then the new meta-data should mean "This comment is a 
(Subversion comment | non-Subversion DAV owner field)", rather than "This 
comment is [not] valid XML".  It should always be set correctly.  (Is there any 
problem with getting RA-local etc. to set it?)  It should possibly be a 
multi-valued setting rather than a Boolean, so that we can add other comment 
types in future.

Strip the XML from Subversion comments RA-DAV, but leave it intact for generic 
DAV owner fields, like you said.

On reading, I recommend that all RA methods take note of the "comment type" 
field and don't read the comment at all if it's not the type they want.

Does this make sense?

- Julian

> 
> Proposed Solution:
> 
>   A. Add a new boolean field to the svn_lock_t structure.  It would be
>      named "xml-packaged-comment", or something similar.  It defaults to
>      0, and if set, it means that lock->comment is packaged in XML.
> 
>   B. When writing locks to the repository:
> 
>      1.  svnserve (and other non-dav writers) continue normally,
>          creating repository locks with the new boolean==0.  They
>          effectively ignore the new field.
> 
>      2.  mod_dav_svn notices whether the incoming lock was created by
>          an svn client or a generic DAV client.  (There are many ways
>          to do this, and they've already been discussed elsewhere.)
>          If the lock comes from an svn client, the XML-packaging is
>          stripped and the lock is written with the boolean==0.  if the
>          lock comes from a generic DAV client, the packaging is
>          preserved and the lock is written with the boolean==1.
> 
>   C. When reading locks from the repository:
> 
>      1.  svnserve (and other non-dav readers) continue to read locks
>          normally, ignoring the new boolean field.  If extra XML junk
>          ends up being marshalled back to the client, that simply
>          means the lock was created by a generic DAV client.  No big
>          deal.
> 
>      2.  mod_dav_svn notices the boolean field when reading.  If 0, it
>          re-adds the necessary XML wrapper.  If 1, it sends the data as-is.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Feb 14, 2005 at 01:53:50PM -0600, Ben Collins-Sussman wrote:
> 
> On Feb 14, 2005, at 1:37 PM, Daniel Patterson wrote:
> >
> >  If DAV locks always store lock-comment = "<D:author>blah blah 
> >blah</D:author>"
> 
> They don't.  They store
> 
>   <D:author ...a possible random list of xml namespaces and tag 
> attributes>
>     opaque cdata
>   </D:author>

note that it is technically D:owner, but the point is still valid.

> So it's not safe to blindly strip away <D:author>.  Furthermore, 
> mod_dav has an obligation to preserve the entire XML block... not just 
> the cdata, but the attributes on <D:author>.

Right. Note also that it is an XML fragment. There may be child elements,
and those elements may have attributes which need preservation. And
ordering, and xml:lang values, and...

And to answer another of Daniel's questions: yes, they are always wrapped
in the D:owner element. The most minimal form would look like:

  <ns0:owner xmlns:ns0="DAV:">gstein</ns0:owner>

The ns### prefix is constructed by mod_dav's internal namespace mgmt code
when it serializes a DOM representation of the original owner element.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by Ben Collins-Sussman <su...@collab.net>.
On Feb 14, 2005, at 1:37 PM, Daniel Patterson wrote:
>
>   If DAV locks always store lock-comment = "<D:author>blah blah 
> blah</D:author>"

They don't.  They store

   <D:author ...a possible random list of xml namespaces and tag 
attributes>
     opaque cdata
   </D:author>

So it's not safe to blindly strip away <D:author>.  Furthermore, 
mod_dav has an obligation to preserve the entire XML block... not just 
the cdata, but the attributes on <D:author>.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: locking: serious RA interoperability problems.

Posted by Daniel Patterson <da...@danpat.net>.
Ben Collins-Sussman wrote:
> 2. When a non-dav RA layer stores a lock structure in the repository,
>    the lock->comment field has no XML wrapping.  Later on, mod_dav
>    reads the lock and sends it to some DAV client (svn or otherwise),
>    but the HTTP response is invalid, because there's no XML wrapping
>    around the field!  mod_dav doesn't realize that it needs to add XML
>    wrapping.

   I'm a little confused, but I don't see a great big problem here....

   If DAV locks always store lock-comment = "<D:author>blah blah 
blah</D:author>", then surely it's safe for mod_dav_svn to strip
   "<D:author>", store only "blah blah blah", and re-wrap "<D:author>"
   *always*.

   That way, if a non-DAV client gets a lock, mod_dav_svn still serves
   back XML, even though the opaque data may not make sense to all
   DAV clients (but this is the case when using different DAV clients
   anyway isn't it?  they don't all store the same data inside 
D:author..).  Sure, non-DAV clients might see pretty noisy lock
   comments, but I think that's the price of working with DAV clients
   at the same time.  Either that, or as you suggested, create a new
   lock property like "lock-metadata" or something that a non-DAV
   ra layer doesn't use.

   If DAV locks do *not* always come from the DAV client wrapped by 
"<D:author>", then it's also a non-issue.  If DAV doesn't guarantee
   that the author field is wrapped by "<D:author>", then mod_dav_svn
   could just XML escape the lock-comment data (s/</&lt;/ for example)
   and serve it back as-is, +/- some smarts to not escape it if it's
   got D:author at the start.


daniel

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org