<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p>Hi Svetlana,</p>
<p>It looks fine, but I am not an official reviewer.</p>
<p>"keystorePath" in readTest() can be a static field.</p>
<p>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.</p>
<p>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.<br>
</p>
<p>Artem</p>
<br>
<div class="moz-cite-prefix">On 08/29/2016 11:23 AM, Svetlana
Nikandrova wrote:<br>
</div>
<blockquote cite="mid:57C47DBB.2040505@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
Hi Artem,<br>
<br>
thank you for your replay. I've updated copyright and made
separate test for this bug.<br>
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. <br>
<br>
<a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Esnikandrova/8157404/webrev.01/">http://cr.openjdk.java.net/~snikandrova/8157404/webrev.01/</a>
<br>
<br>
Thanks,<br>
Svetlana<br>
<br>
<div class="moz-cite-prefix">On 26.08.2016 23:48, Artem Smotrakov
wrote:<br>
</div>
<blockquote
cite="mid:f6477183-d302-b0e5-bd8e-d5973ee033ee@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<p>Hi Svetlana,</p>
<p>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.</p>
I'll leave the main review to someone who is more knowledgeable
in this area, here are a couple of comments:<br>
- Please update copyright year<br>
- You may want to replace new byte[] + System.arraycopy() by
Arrays.copyOfRange()<br>
- 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.<br>
<br>
Artem<br>
<br>
<div class="moz-cite-prefix">On 08/26/2016 10:58 AM, Svetlana
Nikandrova wrote:<br>
</div>
<blockquote cite="mid:57C08329.10401@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
Hello,<br>
<br>
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. <br>
<br>
JBS:<br>
<a moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8157404">https://bugs.openjdk.java.net/browse/JDK-8157404</a><br>
Webrev:<br>
<a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Esnikandrova/8157404/webrev.00/">http://cr.openjdk.java.net/~snikandrova/8157404/webrev.00/</a><br>
<br>
Thanks,<br>
Svetlana<br>
<br>
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>