JDK 8 Code Review Request for 7195301: XML Signature DOM implementation should not use instanceof to determine type of Node (original) (raw)

Sean Mullan sean.mullan at oracle.com
Mon Sep 10 12:57:41 UTC 2012

On 9/7/12 11:45 PM, Xuelei Fan wrote:

Looks fine to me.

It takes me to think whether it is safe to cast an object of ELEMENTNODE type to org.w3c.dom.Element. For example, in RetrievalMethodResolver.java -------------------------------------------------------- 281 static Element getDocumentElement(Set set) { 282 Iterator it=set.iterator(); 283 Element e=null; 284 while (it.hasNext()) { 285 Node currentNode=it.next(); -286 if (currentNode instanceof Element) { +286 if (currentNode != null && currentNode.getNodeType() == Node.ELEMENTNODE) { 287 e=(Element)currentNode; 288 break; 289 } 290 291 } -------------------------------------------------------- It is safe because the element is come from the org.w3c.dom.Node. Using "instanceof" looks more straightforward. Maybe you want to use the same style to check the node type.

Actually, that could still cause issues. The reason this bug was found was because there is a DOM implementation that returns a Node that is an instanceof Element and Document, but the node type is Document (this is an odd design, but it is still technically compliant with the DOM API). This was causing the Document to be incorrectly casted to an Element.

Thanks, Sean

The same for CanonicalizerBase.java, Anyway, it is just a very very minor concern. The fix is fine to me. Xuelei On 9/7/2012 10:39 PM, Sean Mullan wrote: Hi Xuelei,

Would you be able to review my fix for 7195301? webrev: http://cr.openjdk.java.net/~mullan/webrevs/7195301/webrev.00/ No regression test (noreg-hard), requires complex setup, but I have attached a test to the bug report that can be run manually. Thanks, Sean

More information about the security-dev mailing list