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

Re: svn log causes XML parse error (internal error)

From: <kfogel_at_collab.net>
Date: 2004-07-23 17:14:08 CEST

Ben Reser <ben@reser.org> writes:
> On Wed, Jul 21, 2004 at 04:06:36PM -0500, kfogel@collab.net wrote:
> > No, we didn't get the idea of properties from DAV. We knew we wanted
> > key/value pairs even before we'd heard of DAV. Now we are imposing
> > limitations on the keys, because of one (out of three) RA protocols.
>
> Well I didn't set the limitations. juliafoad added it in r7528.

Oh, I'm not blaming you at all. Just correcting the misimpression
that our properties were meant to be DAV-restricted from the beginning
(and adding in some unnecessary griping, I'll admit, but that's not
directed at you).

> Do we really want to go down paths like this? It sounds really really
> ugly. And for what? How many people have asked for property names to
> be able to support slashes? So far we've had one complaint on IRC
> because he was trying to change a property that SVN::Mirror set. As
> ghudson is fond of saying I don't think it's worth the complexity to
> solve until it proves to be a problem.
>
> And this isn't a problem. The restriction on the names has been in
> there for nearly a year and hardly anyone has noticed or complained.
> How difficult is it to pick another name in the valid namespace? Use -
> or . or _ instead of /.
>
> Further, your suggested solution is a totally drive by suggestion. How
> exactly are you going to quote it? You could use base64, but jeez how
> ugly. Can't use URI encoding because % isn't a valid character in XML
> element names. You could make up your own encoding scheme, but again
> how ugly.

Hey, now :-). Neglecting to specify the encoding scheme doesn't make
it a drive-by suggestion. Obviously we *can* encode data safely,
using whatever minimal set of characters we're allowed as long as
there are at least two distinct characters available.

I'm not denying the ugliness of the solution, any more than of the
problem that motivates it. But there's a lot of room between
"ready-to-publish RFC spec" and "drive-by suggestion". If we insisted
that every proposal made on this list be a complete and
ready-to-implement specification, we'd never have gotten a lot of the
features and bugfixes we have today. Leave some room for discussion,
please.

You can disagree with a proposal on technical or aesthetic grounds,
without libeling it as "drive-by".

> The only other option is to use another special namespace where you
> place all the of the name in the xmlns URI (which of course is URI
> encoded) and then decode that and throw away the element name, e.g.:
>
> <P:customprop
> xmlns="http://subversion.tigris.org/xmlns/encoded/foo/bar%20whatever">
>
> But ohh we did something really silly and we barf in the old clients if
> they see a namespace they don't know about. Whoops can't do that.
>
> What about putting them in the existing namespace won't that work?
>
> <P:customprop
> xmlns="http://subversion.tigris.org/xmlns/svn/quoted-propname/foo/bar%20whatever">
>
> Oops that won't work either. Old clients will once again think it's a
> different namespace. And even if it did work they'd think there were
> gobs of properties named "customprop".
>
> So why don't we do the xmlns trick I showed above and make servers and
> clients negotiate if they know about it. What a great idea. Except DAV
> is stateless. We have the OPTIONS method. But we don't always call it
> and the only way we have of putting flags in it is also really ugly.
>
> We could try the new way and if it fails fallback on the old way. But
> we do dozens of PROPFINDs in some operations and that could
> significantly increase our round trips against old servers.
>
> There is no way to do this that is reasonably backwards compatable and
> elegent. Believe me I tried. I finally gave up and worked around the
> colon issue.

Yeah, but wait a sec.

You just proposed, and tossed, three solutions based on namespaces.
None of them were the solution I proposed, which is also based on a
namespace, but as it's *Subversion*'s namespace, it maintains
compatibility (as far as we know) -- which is exactly why I used it.

There are an infinite number of non-working solutions for any problem.
Proposing three of them here, only to shoot them down, doesn't add
anything to the discussion. There's surely some Latin name for this
rhetorical technique :-), but I don't know it. All I know is, it
doesn't mean anything. The number of non-working solutions thought of
has *no relevance* to the solubility of a problem. Once you know they
don't work, there's just no reason to bring them up again.

Regarding my proposal: you didn't like it because of inelegantness,
not incorrectness. I pretty much agree, and if the only practical
problem we have is that once a year someone posts saying they can't
use "/" in a property name, then I can see this not being worth the
trouble to fix. You're right that I don't like the creeping way these
restrictions came upon us. I wish I'd been paying more attention
before :-(.

> > I'm not saying this enhancement should delay 1.1 or even 1.2. I am
> > merely asking if anyone disagrees that the situation is a bug, and
> > secondarily asking if the above method seems like it would fix it.
>
> No I don't agree it is a bug. We've explicitly disallowed these names.
> Right or wrong that's the way it is until 2.0.

Okay.

> > It seems quite reasonable to me that people want "/" in property
> > names, just as they want ":". We've already had real-world examples
> > of both.
>
> Sure it seems reasonable enough. But here's the deal. The colon
> properties were already being allowed by the client software. They only
> failed to work in one specific case and then only as a revprop. So many
> people were probably using them completely unaware of the problems with
> them. I suspect very few people set revprops other than the ones we use
> internally (svn:log, svn:author, etc...).
>
> But the / is an entirely different situation. It's disallowed by the
> client library already. It has much more serious consequences if it
> gets into a repo. It doesn't just fail in some obscure case. It will
> cause operations involving it to have completely invalid XML.
>
> The bug is that we're allowing people driving the repos/fs layers
> directly to set properties that are invalid as far as our client libs
> are concerned. I suppose someone else might want to use these libs for
> something completely different that might not care about our RA issues.
> But I don't see what choice we have.

These three paragraphs, and the one preceding them, were all the
response I was actually asking for.

> > I certainly agree that we should decide on a valid syntax for property
> > names (and values, for that matter), among other things. But the
> > limitations encouraged by DAV are far too narrow, and we shouldn't
> > settle for them unless we have absolutely no choice.
>
> Well I think we have. You just don't like it. :)

No, I don't, you're right :-). But you've persuaded me to live with
it, though I'm embarrassed that I didn't notice it was happening until
it was too late. Sigh.

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jul 23 18:45:31 2004

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