On Thu, Mar 20, 2003 at 12:17:36AM -0800, Greg Stein wrote:
> On Wed, Mar 19, 2003 at 09:40:46PM -0600, Ben Collins-Sussman wrote:
> >...
> > $ svn ls -v http://svn.collab.net/repos/svn/branches
> > Segmentation fault (core dumped)
> >
> > The sefault happens on line 73 of ls-cmd.c; svn_utf_cstring_from_utf8
> > tries to convert a NULL dirent->last_author. Most of the dirent's
> > fields are NULL, actually, which ain't right.
> >
> > The server hasn't changed: ethereal is still sending the
> > creator-displayname DAV property, but fetch.c:set_special_wc_prop()
> > isn't being called at all, according to gstein. (He put a printf()
> > into that function, and never saw the message!)
>
> In retrospect, that was the wrong place to look. After further searching, I
> isolated the problem to some property handling code in ra_dav/props.c. I
> patched it up in rev 5400, but the net result was disabling reception of
> binary properties in the client.
>
> I talked with Mike about it for a bit. We've got a couple things going on in
> ra_dav w.r.t property handling:
>
> 1) some properties are handled via Neon's prop handling
> 2) some properties are handled by working with the XML directly
I call these "flat" and "complex" properties in ne_props.h, the
distinction being whether the property value is just a string, or some
structured XML. "structured" is a better name so I'll use that here.
> Specifically, we use (2) to extract URLs out of properties that look like:
>
> <D:baseline-collection><D:href>/repos/!svn/bc/5/</D:href></D:baseline-collection>
> If we had used Neon's property handling, we'd have to parse out the D:href
> elements to get at the CDATA.
(which is not really possible to do correctly, because if the property
is treated as a flat string you lose the namespace prefix mapping)
> The problem is that we want to recognize properties like:
>
> <S:user-prop V:encoding="base64">dkalkjfljfd</S:user-prop>
>
> and extract that V:encoding attribute. However, we can only do that through
> the (2) code rather than Neon's prop handling. So Mike made the alterations,
> and everything look fine and dandy. But it also altered Neon's prop
> handling, so it stopped passing stuff to our (1) code. That dropped out the
> recognition and storage of DAV:creator-displayname, and so the author was
> never set, so the 'svn ls' command tanked.
>
> Mike and I tentatively decide to forget about binary prop reception in 0.20
> for now. The fix is non-obvious, so we're thinking about reviewing our DAV
> prop handling code. Pending a better idea, I'm inclined to use Neon's XML
> handling, but to do our own property response parsing.
>
> Another possibility might be a review of the Neon APIs and some discussion
> and patches back to Joe. The basic issue is this part of a DAV response:
>
> <D:prop>
> <S:property V:encoding="foo" a:some-attr="bar">baz</S:property>
> </D:prop>
>
> Strictly speaking, the attributes on S:property are not part of the property
> value, so there is an argument that they shouldn't be passed along to the
> property-value handling code. However, those attributes can certainly
> include metadata to control the interpretation of the value. We want an
> encoding property, but there are also standard props like xml:lang which
> most definitely alter how to view that prop.
If you are treating this property as a "flat" property, then you don't
get attributes, no. If the property value element has attributes which
you want to parse, it is by definition a structured property (at least
by my definition :). If you want to treat this property as structured,
then you *can* already handle the attributes if you want.
The problem is that this new code is trying have two classes of "flat"
properties; ones which ra_dav handles, and ones which ne_props handles.
But this can't be done currently, since if ra_dav returns VALID for
NE_ELM_unknown, ne_props will never see them.
I would have thought this problem can be fixed in ra_dav without
actually needing a neon API change. Specifically, in
props.c:end_element(), change:
case NE_ELM_unknown:
/* if this is not a user-visible property, we don't care about it. */
if (! ((strcmp(elm->nspace, SVN_DAV_PROP_NS_CUSTOM) == 0)
|| (strcmp(elm->nspace, SVN_DAV_PROP_NS_SVN) == 0)))
return 0;
to do the job of add_to_hash() rather than return 0. (And then the
process_results callback really can be removed and passed as NULL).
A change I've been contemplating for a while is to remove the validation
callback (should never have got into that business), and let
start_element return accept/decline/abort. This would let you handle
only unknown elements within a particular namespace, for instance.
> Going further, the XML spec says that xml:lang is scoped. Thus, a property
> handler really needs to "hear about it" from any possible scope. Not just
> attrs on the property element. It may be that Neon needs to treat that one
> special, so then you ask how to create a general solution -- are there other
> metadata items that might have to scope, too?
I wonder about xml:space too. There is an ne_propset_lang() to handle
this for xml:lang, though as you say it doesn't handle the scoping
properly. (there's a comment in ne_props.c to this effect)
> If we decide to manage the prop response parsing, then we might be able to
> suggest some new Neon APIs to Joe to help us leverage the existing code
> within Neon rather than duplicating all of it. (of course, we can fine-tune
> a lot of our handling rather than a general solution like Neon)
Regards,
joe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 20 12:36:12 2003