Code Review Request : signed module digest verification

Sean Mullan sean.mullan at oracle.com
Thu Mar 10 14:25:51 PST 2011


On 3/10/11 3:55 PM, Mandy Chung wrote:
>>> line 1786: read() may not read hashLength number of bytes.
>>> should you use readFully() instead?
>>
>> I don't think it is necessary. The underlying InputStream is a
>> ByteArrayInputStream so it should not block, right?
>
> That's right. ByteArrayInputStream will read hashLength number of bytes if
> available.
>
> Should you check the returned value == hashLength? Is it possible that the
> signedData is corrupted (i.e. the number of bytes is less than hashLength)?

Yes, good point. I added a check.

>>> line 1804: should it do the sanity check on expectedHashes? Perhaps
>>> do the check when parseSignedData returns.
>>
>> I could but I feel like that check is really part of verifying the hashes, so
>> it belongs in the verifyHashesStart method.
>
> What I meant was that it should check if expectedHashes.size() < 2 and throws an
> exception in case the signedData is invalid. It's fine to add that check in the
> verifyHashesStart method.

Ok, I got you now. I added this check in parseSignedData.

I think I addressed all of your other comments.

Updated webrev: 
http://cr.openjdk.java.net/~mullan/jigsaw/webrevs/digest-verification/webrev.01/

--Sean



More information about the jigsaw-dev mailing list