On Thu, 14 Apr 2022 21:05:06 GMT, Daniel Jeliński <[email protected]> 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
