On Thu, 18 May 2023 04:04:17 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:
>> Kevin Driver has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> rework based upon code review comments
>
> src/java.base/share/classes/sun/security/ssl/CertificateAuthoritiesExtension.java
> line 289:
>
>> 287: shc.peerSupportedAuthorities = spec.getAuthorities();
>> 288: } catch (IllegalArgumentException iae) {
>> 289: shc.conContext.fatal(Alert.DECODE_ERROR, "X500Principal
>> could not be parsed");
>
> To easy debugging, I may use the exception with fatal(Alert alert, String
> diagnostic, Throwable cause). Considering this point, I may prefer to throw
> SSLException in getAuthorities() method.
>
>
> X500Principal[] getAuthorities() throws SSLException {
> ...
> try {
> shc.peerSupportedAuthorities = spec.getAuthorities();
> } catch (SSLException ssle) {
> shc.conContext.fatal(Alert.DECODE_ERROR, "Cannot parse peer supported
> authorities", ssle);
> }
> @XueleiFan Seems like an unnecessary wrapping of IAE when it is going to be
> rethrown anyway.
I'm fine if IAE get checked for every call to getAuthorities().
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13466#discussion_r1197881532
