RFR: 8294985: SSLEngine throws IAE during parsing of X500Principal [v10]

[Xue-Lei Andrew Fan]

On Wed, 17 May 2023 21:54:20 GMT, Kevin Driver <kdriver at openjdk.org> wrote:

>> Fixes: [JDK-8294985](https://bugs.openjdk.org/browse/JDK-8294985)
>
> Kevin Driver has updated the pull request incrementally with one additional commit since the last revision:
> 
>   rework based upon code review comments

Similar comments for update in CertificateRequest.java

Similar comments for update in CertificateRequest.java

src/java.base/share/classes/sun/security/ssl/CertificateAuthoritiesExtension.java line 126:

> 124:         }
> 125: 
> 126:         X500Principal[] getAuthorities() throws IllegalArgumentException {

IAE is unchecked exception, and should not be throwing explicitly in method signature/statement.  I'm not sure if this throwing is really helpful for caller to check the exception.

src/java.base/share/classes/sun/security/ssl/CertificateAuthoritiesExtension.java line 147:

> 145:                     builder.append(principal.toString());
> 146:                 } catch (IllegalArgumentException iae) {
> 147:                     builder.append("unparseable X500Principal");

I may use the iae message as well for better debugging.

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);
}

-------------

Changes requested by xuelei (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13466#pullrequestreview-1431961001
PR Review: https://git.openjdk.org/jdk/pull/13466#pullrequestreview-1431970382
PR Review Comment: https://git.openjdk.org/jdk/pull/13466#discussion_r1197326576
PR Review Comment: https://git.openjdk.org/jdk/pull/13466#discussion_r1197327083
PR Review Comment: https://git.openjdk.org/jdk/pull/13466#discussion_r1197334523