7094155 Code Review Request

[Sean Mullan]

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.ELEMENT_NODE) && ((Element) n)
> 335    .getNamespaceURI()
> 336    .equals(Constants.SignatureSpecNS) && ((Element) n)
> 337    .getLocalName().equals(Constants._TAG_MANIFEST)) {
> 
> and signature/XMLSignature.java, line 318:
> 317 if ((keyInfoElem != null) && (keyInfoElem.getNamespaceURI().equals
> (Constants.SignatureSpecNS) &&
> 318    keyInfoElem.getLocalName().equals(Constants._TAG_KEYINFO)) ) {
> 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.

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
>