Looks good~
Thanks,
Valerie
On 07/25/13 15:17, Anthony Scarpino wrote:
On 07/24/2013 06:37 PM, Valerie (Yu-Ching) Peng wrote:
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.
I wasn't overly concerned about what happens at the end of the file
because it shouldn't get to that point. There should always be a
header and if one was not found, the check has for the most part failed.
'data' is not used outside the loop, so any inconsistency with the
String isn't a problem.
That being said, I'm fine with changing it. I couldn't add 100 to the
read because the while-loop looks for read to equal 0, and the
inconsistency on the read length made the original code troublesome,
so I rewrote it and I think it looks cleaner now.
http://cr.openjdk.java.net/~ascarpino/8020424/webrev.02/
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?
Yeah, that one has a different check. I'll remove it from that comment.
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
