code review request: 8012971 PKCS11Test hiding exception failures

Valerie (Yu-Ching) Peng valerie.peng at oracle.com
Thu Jul 25 01:37:41 UTC 2013


On 07/24/13 13:53, Anthony Scarpino wrote:
> On 07/23/2013 06:00 PM, Valerie (Yu-Ching) Peng wrote:
>>
>> <PKCS11Test.java>
>>
>> 263                 System.arraycopy(data, 900, data, 0, 100);
>> 264                 is.read(data, 0,  900);
>>
>> Do you really mean to overwrite the data[0..99] that you just copied on
>> line 263 with line 264?
>
>
> Good catch.. I would have thought all my testing would have tripped me 
> up on this, but it appears it didn't.
>
>> In addition, don't you want to know how much is read in order to exclude
>> the data from earlier read(...) calls in case that the current read only
>> returns a few bytes?
>
> Yeah, the end of the file read is a bit sloppy, even though it should 
> never see the end of the file as all the nss libraries have the header 
> in it.

Hmm, I think the String object should be constructed w/ the bytes that's 
just read, i.e. new String(data, 0, read)

  256                 s = new String(data);

And line 272 should be update to "read = 100 + is.read(data, 100, 900);"
Also, the value of variable s is only set once on line 256 and yet the 
content of 'data' may change on line 271-272. This may lead to 
inconsistency between 's' and 'data' if the while-loop (line 255) is 
exited due to read <= 0 on line 272.

>>
>> After seeing all these checks and list of known bugs for testing against
>> NSS, I think we probably need a README or some wiki page to keep track
>> all this...
>
> I can throw a readme in the directory with the bug IDs and even my 
> theory that the DER is 480280.
>
I didn't find the NSS pre-3.12 check in TestECDH.java?
Thanks,
Valerie

> webrev updated at:
> http://cr.openjdk.java.net/~ascarpino/8020424/webrev.01/
>
>> Thanks,
>> Valerie
>>
>> On 07/17/13 13:51, Anthony Scarpino wrote:
>>> JDK-8020424 The NSS version should be detected before running crypto
>>> tests
>>> http://cr.openjdk.java.net/~ascarpino/8020424/webrev.00/
>>>
>>> Tony
>>>
>>
>




More information about the security-dev mailing list