<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p>minor comment from me Max. <br>
</p>
<p>src/java.base/share/classes/sun/security/tools/KeyStoreUtil.java</p>
<p>
<blockquote type="cite">
<pre style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: 1; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> 293 throw new IllegalArgumentException("No provider found");</pre>
</blockquote>
Can you print the provider name that was being searched for in
your exception ?<br>
<br>
</p>
<pre class="moz-signature" cols="72">Regards,
Sean.</pre>
<div class="moz-cite-prefix">On 30/06/2016 04:18, Wang Weijun wrote:<br>
</div>
<blockquote
cite="mid:E7CEC2F2-6319-459E-A61E-334AB1FD95EE@oracle.com"
type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">On Jun 30, 2016, at 9:39 AM, Valerie Peng <a class="moz-txt-link-rfc2396E" href="mailto:valerie.peng@oracle.com"><valerie.peng@oracle.com></a> wrote:
Hi Max,
Changes look fine. Just some comments/questions as below.
<src/java.base/share/classes/sun/security/tools/keytool/Resources.java>
- line 46, fix this {0} as well?
</pre>
</blockquote>
<pre wrap="">
I don't see {0} in keytool/Resources.java.
There is one in jarsigner/Resources.java:
46 {"signerClass.is.not.a.signing.mechanism", "{0} is not a signing mechanism"},
You mean it's useless now?
</pre>
<blockquote type="cite">
<pre wrap="">
<test/sun/security/tools/jarsigner/AltProvider.java>
- line 151, maybe it should be calling the createUsingTestJDK? Otherwise the default is to use the compiler JDK
</pre>
</blockquote>
<pre wrap="">
Yes. I don't know this method before.
</pre>
<blockquote type="cite">
<pre wrap="">
<test/sun/security/tools/keytool/i18n.html>
- line 53, better to use -providerClass?
</pre>
</blockquote>
<pre wrap="">
Both should work.
-addprovider works because SUN is already a loaded provider.
-providerclass works because sun.security.provider.Sun is a public class in the same module.
I prefer -addprovider because -providerclass is only for legacy providers loaded with reflection.
In fact, I noticed that SUN is also not in ServiceLoader.load(Provider.class), which means it is not a service. Is that why you suggest the test load it with -providerclass?
Thanks
Max
</pre>
<blockquote type="cite">
<pre wrap="">
Thanks,
Valerie
On 6/28/2016 6:09 PM, Wang Weijun wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Ping again to security-dev. Anyone can approve it?
The latest webrev is at
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~weijun/8130302/webrev.06">http://cr.openjdk.java.net/~weijun/8130302/webrev.06</a>
Change from webrev.05 [1] is tiny.
Thanks
Max
[1] <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~weijun/8130302/webrev.06/interdiff.patch.html">http://cr.openjdk.java.net/~weijun/8130302/webrev.06/interdiff.patch.html</a>
</pre>
<blockquote type="cite">
<pre wrap="">On Jun 16, 2016, at 9:33 AM, Wang Weijun <a class="moz-txt-link-rfc2396E" href="mailto:weijun.wang@oracle.com"><weijun.wang@oracle.com></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On Jun 16, 2016, at 7:50 AM, Valerie Peng <a class="moz-txt-link-rfc2396E" href="mailto:valerie.peng@oracle.com"><valerie.peng@oracle.com></a> wrote:
No big difference to me.
</pre>
</blockquote>
<pre wrap="">Good, I'll remove the cast.
@security-dev, can someone approve the whole webrev.05?
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~weijun/8130302/webrev.05">http://cr.openjdk.java.net/~weijun/8130302/webrev.05</a>
Thanks
Max
</pre>
<blockquote type="cite">
<pre wrap="">Valerie
On 6/15/2016 8:40 AM, Wang Weijun wrote:
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">On Jun 15, 2016, at 10:57 PM, Mandy Chung<a class="moz-txt-link-rfc2396E" href="mailto:mandy.chung@oracle.com"><mandy.chung@oracle.com></a> wrote:
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">241 throw (InvalidParameterException)
This cast should not be needed?
</pre>
</blockquote>
<pre wrap="">} catch (UcryptoException ue) {
throw (InvalidParameterException)
new InvalidParameterException("Error using " + configArg).
initCause(ue.getCause());
}
initCause() returns Throwable but the method's signature throws InvalidParameterException.
</pre>
</blockquote>
<pre wrap="">Perhaps have a local variable for InvalidParameterException exception.
</pre>
</blockquote>
<pre wrap="">Valerie, are you OK with this?
--Max
</pre>
<blockquote type="cite">
<pre wrap="">Mandy
</pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<br>
</body>
</html>