Re: [xml] RE: problem with xmlSetProp in rev 1.366

[Rob Richards <rrichards ctindustries net>]

Buchcik, Kasimier wrote:
> Hi Rob, 
>
>   
> > -----Original Message-----
> > From: Rob Richards [mailto:rrichards ctindustries net] 
> >
> > Sorry I didn't catch this change earlier, but I think the 
> > change made to 
> > not add an attribute with a : in its name when no namespace 
> > is found is 
> > incorrect. The change I am referring to is rev 1.366 of tree.c:
> >     /*
> >     * If we get a QName and the prefix has no namespace-
> >     * binding in scope, then this is an error.
> >     * TODO: Previously this falled-back to non-ns handling.
> >     *   Should we revert this?
> >     */
> >     return(NULL);
> >
> > A colon is a legal character for an attribute name. Although 
> > not where 
> > this issue cropped up and why I didn't catch it before, take DOM for 
> > example. For DOM level 1 where there is no namespace support, the 
> > attribute should be added to the document with the colon in its name.
> >
> > I think this should be reverted.
> >     
>
> Yes, agreed.
>
> The general problem with xmlSetProp() is that it tries to work for
> both DOM levels, without actual control over it from the caller's side.
> If I want to put in a Level 1 attribute, then I might get a Level 2
> attribute back, since I happened to use a colon in my name and was
> unlucky
> that there was a ns-decl in scope with a prefix matching the left side
> of
> the colon :-)
> Maybe we should document very explicitely that this function should
> simply
> not be used. If someone wants a pure Level 1 attribute-setting function
> than we need yet to invent one.
>   
That might be a good idea (i.e. xmlSetNoNSProp).

Now, I was wondering about the ramifications of this breakage. The DOM 
example I gave was just something I thought of. The bug was actually 
caught by a PHP users trying to use ext/soap, which adds attributes in a 
bit of an interesting fashion, while using the latest libxml2 release. I 
should have caught this sooner as the PHP test suite did pick up the 
failure, but I missed checking the nature of the failure.

How common do you think using a : in an attribute name without a 
matching namespace would be in apps (since this does break apps wether 
using the library directly or via bindings that are expecting the 
original behavior)? This is not a critical bug, but wonder if a new 
release should be made with this reverted - when Daniel gets back from 
holiday of course :).

Rob