On Tue, 2 Sep 2025 15:28:00 GMT, Weibing Xiao <[email protected]> wrote:

>> [webrev.zip](https://github.com/user-attachments/files/22605072/webrev.zip)
>> NPE thrown from SASL GSSAPI impl when TLS is used with QOP auth-int against 
>> Active Directory.
>> 
>> When the exception is triggered, LDAP Connection will do "clean-up" 
>> operation and output stream get flushed and closed the context while 
>> GssKrb5Client is still wrapping the message, and tried to send the abandoned 
>> info to the client at line  
>> https://github.com/openjdk/jdk/blob/master/src/jdk.security.jgss/share/classes/com/sun/security/sasl/gsskerb/GssKrb5Base.java#L140.
>>  That's the reason to throw NPE.
>> 
>> The change is going to close socket and output stream in LdapClient.java. It 
>> would allow SASL client code to send the abandoned request to client; then 
>> dispose GSS context. This will avoid NPE to thrown at line 140 of 
>> GssKrb5Base.java.
>> 
>> No test file is attached for this MR since it needs Sasl LDAP server with 
>> security setup. Attached the updated webrev for the reference.
>
> Weibing Xiao has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - remove unused code
>  - removed the commented out code

test/jdk/com/sun/jndi/ldap/SocketCloseTest.java line 143:

> 141:     }
> 142: 
> 143:     private static class CustomSocket extends Socket {

minor: could you please cleanup unused imports after this change?

test/jdk/com/sun/jndi/ldap/SocketCloseTest.java line 144:

> 142:     private static class CustomSocket extends Socket {
> 143:         private int closeMethodCalled = 0;
> 144:         private LdapOutputStream output = new LdapOutputStream();

I believe these are local objects which are no longer used after removal of 
this. I'd personally remove the `private static class LdapInputStream extends 
InputStream {` and `private static class LdapOutputStream extends OutputStream 
{` further down the file

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26566#discussion_r2399024192
PR Review Comment: https://git.openjdk.org/jdk/pull/26566#discussion_r2399032966

Reply via email to