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

Xuelei Fan xuelei.fan at oracle.com
Fri Sep 7 20:45:10 PDT 2012


Looks fine to me.

It takes me to think whether it is safe to cast an object of
ELEMENT_NODE type to org.w3c.dom.Element.

For example, in RetrievalMethodResolver.java
--------------------------------------------------------
 281    static Element getDocumentElement(Set<Node> set) {
 282            Iterator<Node> 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.ELEMENT_NODE) {
 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.

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