7094155 Code Review Request (original) (raw)

Xuelei Fan xuelei.fan at oracle.com
Thu Oct 27 14:57:35 UTC 2011


On 10/27/2011 9:43 PM, Sean Mullan wrote:

On 10/26/11 10:50 PM, Xuelei Fan wrote:

Looks fine to me.

I also look at some other places that use the Node.getLocalName(). As this method may return null, I was wondering it would be nice if we also make changes to com/sun/org/apache/xml/internal/security/utils/ElementCheckerImpl.java, line 10: 9 if ((el == null) || 10 ns!=el.getNamespaceURI() || !el.getLocalName().equals(type)){ 11 return false; 12 } and signature/Manifest.java, line 337: 334 if ((n.getNodeType() == Node.ELEMENTNODE) && ((Element) n) 335 .getNamespaceURI() 336 .equals(Constants.SignatureSpecNS) && ((Element) n) 337 .getLocalName().equals(Constants.TAGMANIFEST)) { and signature/XMLSignature.java, line 318: 317 if ((keyInfoElem != null) && (keyInfoElem.getNamespaceURI().equals (Constants.SignatureSpecNS) && 318 keyInfoElem.getLocalName().equals(Constants.TAGKEYINFO)) ) { 319 this.keyInfo = new KeyInfo(keyInfoElem, BaseURI); If Node.getLocalName() returns, the above lines might throw NPE. Possibly, but I would prefer to hold off on that unless I have a specific test case. These code paths above are parsing standard XML Signature elements which must be defined using the DOM APIs with namespace parameters. This is in contrast to the bug fix in which we may be traversing/parsing XML content with no namespace information. Sounds fine to me.

Xuelei

Thanks, Sean

Xuelei On 10/26/2011 8:54 PM, Sean Mullan wrote: Hi Xuelei,

Could you please review my fix for 7094155 (JSR105 code throws javax.xml.crypto.URIReference when running into Java 7 VM)? webrev: http://cr.openjdk.java.net/~mullan/webrevs/7094155/webrev.00/ Thanks, Sean



More information about the security-dev mailing list