Hi Jamil, Thanks for your review! Reply inline.
> On Mar 13, 2020, at 12:24 PM, Jamil Nimeh <[email protected]> wrote: > > Hello Hai-May, > > The fix overall looks good. One or two comments about the test: > > 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 > (".”). Agree. Will update the comment. > 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. 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’. Line 95 test case will give us a different error from “a*.com”. That is, ‘DNSName with blank components is not permitted’. The existing badNames test case does not have “a*.com”, and I will add it too. Thanks, Hai-May > --Jamil > > On 3/13/2020 9:25 AM, Hai-May Chao wrote: >> Hi, >> >> I need a code review for - >> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8186143 >> <https://bugs.openjdk.java.net/browse/JDK-8186143> >> Webrev: http://cr.openjdk.java.net/~weijun/8186143/webrev.00/ >> <http://cr.openjdk.java.net/~weijun/8186143/webrev.00/> >> >> 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 >>
