Hi Svetlana,
It looks fine, but I am not an official reviewer.
"keystorePath" in readTest() can be a static field.
I also meant that one test with SequenceInputStream seems to be enough,
so you could just add a new test case to ReadP12Test.java. But it's fine.
I am not sure how DerIndefLenConverter works, but it looks a little
strange to me that it needs to extend an array before passing it to
DerIndefLenConverter. I see that convert() method also uses arraycopy()
method. But it seems to be out of scope here.
Artem
On 08/29/2016 11:23 AM, Svetlana Nikandrova wrote:
Hi Artem,
thank you for your replay. I've updated copyright and made separate
test for this bug.
As for Arrays.copyOfRange() unfortunately it won't simplify code in my
case. I need to extend an array, not to get a sub-array of existing one.
http://cr.openjdk.java.net/~snikandrova/8157404/webrev.01/
<http://cr.openjdk.java.net/%7Esnikandrova/8157404/webrev.01/>
Thanks,
Svetlana
On 26.08.2016 23:48, Artem Smotrakov wrote:
Hi Svetlana,
DerValue class may be implicitly used in different areas (x509,
SSL/TLS, keystores, maybe krb5, etc). Please make sure that tests
from jdk_security pass.
I'll leave the main review to someone who is more knowledgeable in
this area, here are a couple of comments:
- Please update copyright year
- You may want to replace new byte[] + System.arraycopy() by
Arrays.copyOfRange()
- It may be better to add a separate test case in ReadP12Test.java
for SequenceInputStream instead of loading a keystore twice in each
call to readTest(). One test with SequenceInputStream seems to be
enough, and it would make the logic of readTest() clearer.
Artem
On 08/26/2016 10:58 AM, Svetlana Nikandrova wrote:
Hello,
please review this fix. It's not possible to read PKCS12 keystore
with big undefined length DER value in it from SequenceInputStream.
Root cause of the problem is that sun.security.util.DerValue relays
on InputStream.available() to get a complete 'indefinite.length'
section length and then read it, but for SequenceInputStream this
method returns number of available bytes only for current input
stream, not the whole sequence. Fixed to read all available data.
JBS:
https://bugs.openjdk.java.net/browse/JDK-8157404
Webrev:
http://cr.openjdk.java.net/~snikandrova/8157404/webrev.00/
<http://cr.openjdk.java.net/%7Esnikandrova/8157404/webrev.00/>
Thanks,
Svetlana
