[8] Request for review: 8012288: XML DSig API allows wrong tag names and extra elements in SignedInfo

Xuelei Fan Xuelei.Fan at Oracle.COM
Thu Jul 25 22:42:03 UTC 2013


On 7/26/2013 3:28 AM, Sean Mullan wrote:
> On 07/24/2013 10:16 AM, Xuelei Fan wrote:
>> DOMUtils.java
>> =============
>> It might be a little bit better to reuse the node local name checking
>> with a new private static method.
>
> Good suggestion.
>
>> Otherwise, looks fine.
>>
>> A minor comment is a very personal preference. I may prefer to return
>> null instead of throw exception in the new methods of class DOMUtils,
>> when there is no such node. And the caller throw the exception instead.
>> For example:
>>      public static Element getFirstChildElement(Node node,
>>          String localName) throws MarshalException
>
> I considered that, but most of the time null is an exception, so it's
> more concise to have the method handle this case, than the other way
> around.
>
ok.

> I have updated the webrev:
> http://cr.openjdk.java.net/~mullan/webrevs/8012288/webrev.01/
>
> Can you take a quick look at DOMUtils to see if it looks ok?
>
Looks fine to me.

Xuelei

> Thanks,
> Sean
>
>>
>>
>> Xuelei
>>
>> On 7/23/2013 9:05 PM, Sean Mullan wrote:
>>> Xuelei,
>>>
>>> Could you please review my fix for 8012288:
>>>
>>>    http://cr.openjdk.java.net/~mullan/webrevs/8012288/webrev.00/
>>>
>>> There is already an SQE test for this, so I have added the noreg-sqe
>>> label.
>>>
>>> Thanks,
>>> Sean
>>
>




More information about the security-dev mailing list