On Mon, 2004-11-15 at 20:50 +0100, Raul Benito wrote: > Samuel Meder wrote: > > >We just upgraded to the xmlsec 1.2 RC1 release and started noticing > >problems with the exclusive c14n implementation. In particular SAML > >assertion signature validation started to fail after the assertion was > >sent over the wire (using axis). > > > > > First of all thanks for the bug report. You have hit the code that is > not very well exercised by the test suite, so thanks a lot. Second you > are using the slow path (i.e. using xpath transformations) and that's good. > > >The assertion when initially generated declared several redundant > >default namespaces on the elements which Axis subsequently removed when > >sending the actual message. Furthermore, the code lists #default in the > >inclusive namespaces, so as far as I can tell this should make it ok for > >axis to remove redundant NS declarations. > > > > > > > The #default namespace means treat the xmlns prefix as it was done in > the inclusive c14n
Right, I understand that. > >Now in particular, I'm seeing the following problems with the code: > > > >* If the element being processed lacks a default ns declaration and does > >not use a prefix then xmlns="" is added unconditionally even if the > >element is declared in the default namespace and the immediate parent > >element defines a default NS. > > > > > > > That is the way is defined if xmlns is added in inclusive(and that's > thing seem to be stressed by the tests). I think I disagree here. The standard only says: "The XPath data model represents an empty default namespace with the absence of a node, not with the presence of a default namespace node having an empty value. Thus, with respect to the fact that element e3 in the following examples is not namespace qualified, we cannot tell the difference between <e1 xmlns="a:b"><e2 xmlns=""><e3/></e2></e1> versus <e1 xmlns="a:b"><e2><e3 xmlns=""/></e2></e1>. All we know is that e3 was not namespace qualified on input, so we preserve this information on output if e2 is omitted so that e3 does not take on the default namespace qualification of e1." Now my interpretation of the above is that xmlns="" should only be added if the element is in the empty namespace to begin with and it should not be added if the element is in e.g. xmlns="http://foobar.com";, although you may well have to add a xmlns="http://foobar.com"; depending on the visibility of the element declaring that default namespace. > >* CanonicalizerBase.java has the following code: > > > > Element currentElement = (Element) currentNode; > > OutputStream writer=this._writer; > > String tagName=currentElement.getTagName(); > > if (currentNodeIsVisible) { > > //This is an outputNode. > > ns.outputNodePush(); > > writer.write('<'); > > writeStringToUtf8(tagName,writer); > > } else { > > //Not an outputNode. > > ns.push(); > > } > > > > // we output all Attrs which are available > > Iterator attrs = handleAttributes(currentElement,ns); > > while (attrs.hasNext()) { > > Attr attr = (Attr) attrs.next(); > > outputAttrToWriter(attr.getNodeName(), attr.getNodeValue(), > > writer); > > } > > > >i.e. attributes are printed even if the element is not visible. This > >seems to result in spurious xmlns="" being output in random places. > >Maybe this code: > > > > > > > That's needed for rare xpath transformations but you have hit a bug > because it should only output when the xpath select the xmlns but not in > when it is added as inclusive(but i need to check with the spec to be sure). > > > Iterator it=this._inclusiveNSSet.iterator(); > > while (it.hasNext()) { > > String s=(String)it.next(); > > Attr key=ns.getMappingWithoutRendered(s); > > if (key==null) { > > continue; > > } > > result.add(key); > > } > > > >in Canonicalizer20010315Excl.java and the fact that the empty default > >map seems to be added to ns (why is that done btw? Seems wrong to me) by > >default has something to do with it? > > > > > > > The #default in inclusive prefix makes to do this behavior, as expected. Again, I'm not sure why you believe that. > >The inclusive code seems to behave a little better (not seeing > >validation failures anymore), but I am a bit skeptical about the > >following: > > > > if (isRealVisible) { > > //The element is visible, handle the xmlns definition > > Attr xmlns = E.getAttributeNodeNS(Constants.NamespaceSpecNS, > > "xmlns"); > > Node n=null; > > if (xmlns == null) { > > //No xmlns def just get the already defined. > > n=ns.getMapping(XMLNS); > > } else if ( !this._xpathNodeSet.contains(xmlns)) { > > //There is a definition but the xmlns is not selected by the > > xpath. > > //then xmlns="" > > n=ns.addMappingAndRenderXNodeSet(XMLNS,"",nullNode,true); > > > > } > > //output the xmlns def if needed. > > if (n!=null) { > > result.add(n); > > } > > //Float all xml:* attributes of the unselected parent elements to > > this one. > > addXmlAttributes(E,result); > > } > > > >Shouldn't the xmlns="" mapping only be added if this mapping is in scope > >and the ancestor element declaring the mapping is not visible (and since > >NameSpaceSymbolTable always adds that mapping...). Also I'm not quite > >sure that I understand the "else if": > >CanonicalizerBase:canonicalizeXPathNodeSet seems to throw a exception if > >it finds a attribute node in the _xpathNodeSet which would lead me to > >assume that the node set would never contain the xmlns node? Any comment on the above? /Sam > >Might also be that I am completely misunderstanding things. > >Canonicalization makes my brain hurt... > > > > > > > Yes it is really difficult,and now that I have arrived home I will try > to fix it, thanks for your report(but try to don't use the xpath code, > it will be good for your performance). > > Thanks again, > > Regards, > > > Raul >
