On Fri, 23 Feb 2024 02:05:09 GMT, Jaikiran Pai <[email protected]> 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

The fix for [8326233](https://bugs.openjdk.org/browse/JDK-8326233) uses two 
consecutive `if`s instead of `if`/`else if`: 
https://github.com/openjdk/jdk/pull/17923/files#diff-6fdccf370b8c2ed3fc9690a3f5ee9a7d65af933a84252d99709c8926c52de43dR628

This shouldn't make a difference except for invalid parameter objects (which 
could be created using reflection or by extending the class). I'm just asking 
if both places should use the same logic when copying the parameters?

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

PR Review: https://git.openjdk.org/jdk/pull/17940#pullrequestreview-1900246630

Reply via email to