RFR: 8284694: Avoid evaluating SSLAlgorithmConstraints twice [v3]

[Xue-Lei Andrew Fan]

On Thu, 14 Apr 2022 21:05:06 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

>> During TLS handshake, hundreds of constraints are evaluated to determine which cipher suites are usable. Most of the evaluations are performed using `HandshakeContext#algorithmConstraints` object. By default that object contains a `SSLAlgorithmConstraints` instance wrapping another `SSLAlgorithmConstraints` instance. As a result the constraints defined in `SSLAlgorithmConstraints` are evaluated twice.
>> 
>> This PR improves the default case; if the user-specified constraints are left at defaults, we use a single `SSLAlgorithmConstraints` instance, and avoid duplicate checks.
>
> Daniel Jeliński has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - add more factory methods, update copyright
>  - return DEFAULT also when user constraints are null

Thanks for the update.  It looks good to me, except comments for the coding style.

src/java.base/share/classes/sun/security/ssl/SSLAlgorithmConstraints.java line 94:

> 92:      * @return a SSLAlgorithmConstraints instance
> 93:      */
> 94:     static AlgorithmConstraints forSocket(SSLSocket socket,

I like the idea to use a static method for the construction.  What do you think if the same coding style is used for other constructors, by making them private and add forSocket() methods accordingly?  For example, changing the following constructor to a private method.

SSLAlgorithmConstraints(SSLEngine engine, String[] supportedAlgorithms,
            boolean withDefaultCertPathConstraints) {

src/java.base/share/classes/sun/security/ssl/SSLAlgorithmConstraints.java line 118:

> 116:         if (userSpecifiedConstraints == null) {
> 117:             return withDefaultCertPathConstraints ? DEFAULT : DEFAULT_SSL_ONLY;
> 118:         }

It looks like a partial duplicate of nullIfDefault().  What do you think if  merging the logic into nullIfDefault()?  Or even merging nullIfDefault() logic into the getUserSpecifiedConstraints() method?  New parameters may be required for the getUserSpecifiedConstraints() methods.

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

PR: https://git.openjdk.java.net/jdk/pull/8199