<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Looks good to me Artem. Thanks for modifying!<br>
<pre class="moz-signature" cols="72">Regards,
Sean.</pre>
<div class="moz-cite-prefix">On 08/09/2015 18:00, Artem Smotrakov
wrote:<br>
</div>
<blockquote cite="mid:55EF1413.7050804@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Sean,<br>
<br>
I updated LDAPCertStore, please take a look<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Easmotrak/8134708/webrev.02/">http://cr.openjdk.java.net/~asmotrak/8134708/webrev.02/</a><br>
<br>
- updated exception messages<br>
- fixed typos<br>
- added @Override annotations<br>
- removed unused field<br>
- removed unused imports<br>
<br>
Artem<br>
<br>
On 09/02/2015 01:23 PM, Seán Coffey wrote:<br>
</div>
<blockquote cite="mid:55E6CE32.6040604@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Hi Artem, <br>
<br>
I'll let the main review to other reviewers but while we're
here, can you consider improving the original exception message
that was seen in this issue ?<br>
In LDAPCertStore constructor :<br>
<br>
} else {<br>
throw new InvalidAlgorithmParameterException(<br>
"parameters must be either
LDAPCertStoreParameters or " +<br>
"URICertStoreParameters");<br>
}<br>
<br>
Can we print the instance type of the 'params' variable in the
exception message ? params.getClass().getName() should be
sufficient.<br>
<br>
I see 2-3 other exceptions in LDAPCertStore that could be
improved there also. If you can change them, that would be
great - otherwise we can follow up with enhancement request.<br>
<br>
if (!u.getScheme().equalsIgnoreCase("ldap")) {<br>
throw new InvalidAlgorithmParameterException(<br>
"Only LDAP URIs are supported for LDAP
Certore");<br>
<br>
Let's print the scheme received!<br>
<br>
<br>
} else if (!(selector instanceof X509CertSelector)) {<br>
throw new CertStoreException("need X509CertSelector
to find certs");<br>
<br>
this code occurs twice. Let's print the selector class received.<br>
<br>
<pre class="moz-signature" cols="72">Regards,
Sean.</pre>
<div class="moz-cite-prefix">On 02/09/15 00:15, Artem Smotrakov
wrote:<br>
</div>
<blockquote cite="mid:55E6317A.7080401@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
Hello,<br>
<br>
Please review this fix for 9.<br>
<br>
Certpath validation fails to load certs and CRLs if AIA and
CRLDP extensions point to LDAP resources. This happens because
LDAPCertStore accepts only instances of
LDAPCertStoreParameters and URICertStoreParameters classes,
but sun.security.provider.certpath.URICertStore uses an inner
static URICertStoreParameters class. Please see details in the
bug.<br>
<br>
This fix removes URICertStore.URICertStoreParameters class,
and updates URICertStore and DistributionPointFetcher to use
new
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
java.security.cert.URICertStoreParameters class.<br>
<br>
A regression test starts a local name service which logs
requested host names. The test checks that host names from AIA
and CRLDP extensions were loaded and requested to resolve
during certpath validation.<br>
<br>
Bug: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8134708">https://bugs.openjdk.java.net/browse/JDK-8134708</a><br>
Webrev: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Easmotrak/8134708/webrev.01/">http://cr.openjdk.java.net/~asmotrak/8134708/webrev.01/</a><br>
<br>
Artem<br>
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>