On Thu, 9 Apr 2026 05:17:54 GMT, Jaikiran Pai <[email protected]> wrote:

>> Can I please get a review of this change which proposes to address the issue 
>> noted in https://bugs.openjdk.org/browse/JDK-8261289?
>> 
>> The JDK's implementation of the `LdapContext` allows for the LDAPv3 Extended 
>> Response for StartTLS. `LdapContext.extendedOperation(new 
>> StartTlsRequest())` can be invoked by an application to obtain a 
>> `StartTlsResponse` which can then be used to `StartTlsResponse.negotiate()` 
>> a TLS session. A successful TLS negotiation will result in the underlying 
>> LDAP connection's input/output streams being switched to TLS specific 
>> streams. Any subsequent communication over the LDAP context will happen over 
>> these TLS streams, until the `StartTlsResponse.close()` is called.
>> 
>> One part of TLS negotiation involves certificate verification. In the JDK's 
>> implementation of `StartTlsResponse`, if the certificate verification fails 
>> (for whatever reason) after the LDAP connection's streams have been switched 
>> to TLS specific streams, then these streams must be switched back to the 
>> original streams that were present before the TLS negotiation was attempted. 
>> However, due to a bug, this currently doesn't happen and after a failed TLS 
>> negotiation, subsequent communication over the LDAP connection (which is 
>> allowed) continues to use these TLS streams.
>> 
>> The commit in this PR addresses that issue in the implementation of 
>> `StartTlsResponse`. Minor related clean up is done to that implementation to 
>> properly handle exceptions. A new jtreg test has been introduced to 
>> reproduce the issue and verify the fix.
>> 
>> tier1, tier2, tier3 tests continue to pass with this change.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Jaikiran Pai has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - merge latest from master branch
>  - merge latest from master branch
>  - 8261289: incorrect cleanup in LDAP TLS handling

Thank you Aleksei for the review.

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

PR Comment: https://git.openjdk.org/jdk/pull/30547#issuecomment-4288086818

Reply via email to