<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi Jamil,<div class=""><br class=""></div><div class="">Thanks for your review! Reply inline.<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Mar 13, 2020, at 12:24 PM, Jamil Nimeh <<a href="mailto:jamil.j.nimeh@oracle.com" class="">jamil.j.nimeh@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" class="">
<div class=""><p class="">Hello Hai-May,</p><p class="">The fix overall looks good. One or two comments about the test:</p>
<ul class="">
<li class="">103: I think the comment might be more clear saying something
like "partial wildcard disallowed" since it's not the "*" in and
of itself that's the issue, it's that the next character
following it isn't a domain separator (".”).</li></ul></div></div></blockquote><div><br class=""></div>Agree. Will update the comment.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><ul class="">
<li class="">A similar badSanNames test case (I think) that walks a
different code path would be something like "a*.com". Although
the test on line 95 might walk the same codepath...If so then no
need to add anything else.</li></ul></div></div></blockquote><div><br class=""></div>I’ll add “a*.com” to badSanNames test case as it will detect the error for ‘DNSName components must consist of letters, digits, and hyphens’.</div><div>Line 95 test case will give us a different error from “a*.com”. That is, ‘DNSName with blank components is not permitted’.</div><div>The existing badNames test case does not have “a*.com”, and I will add it too. </div><div><br class=""></div><div>Thanks,</div><div>Hai-May</div><div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><ul class="">
</ul><p class="">--Jamil<br class="">
</p>
<div class="moz-cite-prefix">On 3/13/2020 9:25 AM, Hai-May Chao
wrote:<br class="">
</div>
<blockquote type="cite" cite="mid:9E9A90E4-42A2-4375-B2CD-9D64A3BEBD07@oracle.com" class="">
<pre class="moz-quote-pre" wrap="">Hi,
I need a code review for -
Bug: <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8186143">https://bugs.openjdk.java.net/browse/JDK-8186143</a>
Webrev: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~weijun/8186143/webrev.00/">http://cr.openjdk.java.net/~weijun/8186143/webrev.00/</a>
The keytool -ext option doesn’t accept wildcards for DNS subject alternatives names in certificates. Certificates with wildcarded domains are useful for allowing domain names under a common subdomain to share the same certificate.
The fix involves adding a new DNSName constructor with an additional boolean flag ‘allowWildcard’.
Thank you,
Hai-May
</pre>
</blockquote>
</div>
</div></blockquote></div><br class=""></div></body></html>