RFR: 8326381: com.sun.net.httpserver.HttpsParameters and SSLStreams incorrectly handle needClientAuth and wantClientAuth [v5]

[John Jiang]

On Fri, 23 Feb 2024 02:05:09 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Can I please get a review of this change which proposes to fix https://bugs.openjdk.org/browse/JDK-8326381?
>> 
>> As noted in the JBS issue, the implementation in `setNeedClientAuth()` and `setWantClientAuth()` of `com.sun.net.httpserver.HttpsParameters` wasn't matching the API specification. The commit in this PR fixes that issue and it now matches the API specification as well as what is done in `javax.net.ssl.SSLParameters` class.
>> 
>> Additionally, as noted in the JBS issue, the (internal class) `sun.net.httpserver.SSLStreams` had a bug where it could end up resetting the `needClientAuth` flag on the `SSLEngine` because of the way the `setNeedClientAuth()` and `setWantClientAuth()` methods were being called on the `SSLEngine`. This too has been fixed in this PR.
>> 
>> A new jtreg test has been introduced to reproduce the issue in the `HttpsParameters` class and verify this fix.
>
> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   undo changes to SimpleSSLContext in test library

test/jdk/com/sun/net/httpserver/HttpsParametersClientAuthTest.java line 114:

> 112:         needClientAuthParams.setWantClientAuth(initialWantClientAuth);
> 113:         // needClientAuth = true and thus wantClientAuth = false
> 114:         needClientAuthParams.setNeedClientAuth(true);

I just want to clarify that these new cases look not suggested by my previous comment.

In my suggested cases, as shown as the below, 

setNeedClientAuth(true);
setWantClientAuth(false);


setWantClientAuth(true);
setNeedClientAuth(false);

this first setting takes a state (`needClientAuth` or `wantClientAuth`) to `true`, and the second setting takes both states to `false`.
Before this fix, the second setting doesn't reset the `true` state specified by the first setting to `false`.

Of course, if you think it's unnecessary to cover these corner cases, please feel free to ignore them.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17940#discussion_r1500171456