> security/utils/JavaUtils.java
> =============================
The updated code is a straightforward move from other classes in order
to remove the duplicated code. Looks fine but we might want to make a
few little enhancements within this fix.
162 public static byte[] convertASN1toXMLDSIG(byte asn1Bytes[], ..
201 public static byte[] convertXMLDSIGtoASN1(byte xmldsigBytes[], ..
Minor comment about code conversions. I may prefer to use consistent
style to declare the array parameters.
byte asn1Bytes[] -> byte[] asn1Bytes
byte xmldsigBytes[] -> byte[] xmldsigBytes
convertASN1toXMLDSIG(byte asn1Bytes[], int size):
-------------------------------------------------
I may check the asn1Bytes.length against/and the "size" parameter, and
check first 3 bytes before getting the 4rd byte. Similarly, I may check
the first byte and the rest length of the array before try to parse the
s (the 2nd fragment of the array) value of the signature.
convertXMLDSIGtoASN1(byte xmldsigBytes[], int size):
----------------------------------------------------
I did not get the idea behind lines 212-215 and lines 220-223. Why try
to check and ignore the negative value?
225 byte asn1Bytes[] = new byte[6 + j + l];
By the line, the length of the target byte array is known. I might
prefer to allocate the exactly size for byte array here. Otherwise, the
2nd System.arraycopy() would need re-allocate the array for more space.
Xuelei
> On 4/30/2014 4:48 AM, Sean Mullan wrote:
>> Please review the following change which adds support for 2048-bit DSA
>> keys and the DSA-SHA256 algorithm to the XML Signature implementation:
>>
>> webrev: http://cr.openjdk.java.net/~mullan/webrevs/8038349/webrev.00/
>>
>> JDK 8 already includes the underlying support for both of these in the
>> Sun provider. For 2048 bit keys, the ASN.1 parsing code in the XML
>> Signature layer needed to be adapted to handle 2048 bit keys, and for
>> SHA-256 it was just a matter of registering the algorithm URI and
>> instantiating a Signature object with the SHA256WithDSA algorithm.
>>
>> Thanks,
>> Sean
>
