[#1216] Keep the decoding error of the client filter when no LDAP connectio...

Valery Kharseko

3 hours ago fc4e7423e8007b9b6f5368fa101493cfd5711760

[#1216] Keep the decoding error of the client filter when no LDAP connection is attached (#1217)

Fixes #1216

## Problem

`LDAPClientFilter.handleReadException()` and `exceptionOccurred()` close
the LDAP connection attached to the Grizzly connection without checking
that one is attached. After `handleClose()` has removed it, a message
the client cannot decode throws a `NullPointerException` in place of the
`DecodeException`. Grizzly logs the `NullPointerException` at `WARNING`
(`GRIZZLY0013`), and the decoding error with its message is lost.

The failed job of #1213 ([run
38947864951](https://github.com/OpenIdentityPlatform/OpenDJ/actions/runs/38947864951/job/117131465938))
logged exactly that: the client read its own Unbind on a connection it
had already closed.

## Change

Both methods return when no LDAP connection is attached, as
`handleClose()` and the result handlers of `ClientResponseHandler`
already do.

- `handleRead()` then throws the `DecodeException`, as it does with a
connection attached. `DefaultFilterChain.execute()` logs it at `FINE`
and closes the connection.
- In `exceptionOccurred()` the Grizzly connection is still open (the
method returns earlier otherwise), so only a connection not registered
yet has no LDAP connection. No failure was seen there. The check keeps
the method from failing the same way: an exception thrown from it inside
`DefaultFilterChain.execute()` would escape `throwChain()` before the
`closeWithReason()` that follows.

One correction to the issue: reads cannot reach `handleRead()` before
`registerConnection()`. Grizzly 4.0.2 enables reading only after the
connect completion handler has run:
`TCPNIOConnectorHandler.EnableReadHandler.onComplete()` calls
`completed()` before `enableInitialOpRead()`. The window seen in
practice is the one after `handleClose()`.

## Tests

`LDAPClientFilterTestCase` (new):

| Test | What it pins |
|---|---|
| `handleReadWithoutLDAPConnectionThrowsTheDecodeError` | No LDAP
connection attached: `handleRead()` throws `Unexpected LDAP request:
id=1, …`. Red on master with the NPE. |
| `exceptionOccurredWithoutLDAPConnectionReturns` | No LDAP connection
attached: `exceptionOccurred()` returns. Red on master with the NPE. |
| `handleReadClosesTheLDAPConnectionOnDecodeError` | With one attached,
it is still closed with `CLIENT_SIDE_DECODING_ERROR`. |
| `exceptionOccurredClosesTheLDAPConnection` | With one attached, it is
still closed with `CLIENT_SIDE_LOCAL_ERROR`. |

`GrizzlyLDAPConnection` is final, so the last two use a real one and
check the error through a `ConnectionEventListener` mock.

Mutants of `LDAPClientFilter`, each run against the class:

| Mutant | Caught by |
|---|---|
| no null check in `handleReadException()` |
`handleReadWithoutLDAPConnectionThrowsTheDecodeError` |
| no null check in `exceptionOccurred()` |
`exceptionOccurredWithoutLDAPConnectionReturns` |
| `handleReadException()` never closes |
`handleReadClosesTheLDAPConnectionOnDecodeError` |
| `exceptionOccurred()` never closes |
`exceptionOccurredClosesTheLDAPConnection` |

## Verification

- `org.forgerock.opendj.grizzly.**` in the reactor (`-pl opendj-grizzly
-am`), JDK 27, macOS: 1044 tests, 0 failures, 0 errors, 0 skipped.

 opendj-grizzly/src/main/java/org/forgerock/opendj/grizzly/LDAPClientFilter.java

@@ -13,6 +13,7 @@
 *
 * Copyright 2010 Sun Microsystems, Inc.
 * Portions Copyright 2012-2016 ForgeRock AS.
 * Portions Copyright 2026 3A Systems, LLC.
 */

package org.forgerock.opendj.grizzly;
@@ -403,6 +404,10 @@
            return;
        }
        final GrizzlyLDAPConnection ldapConnection = LDAP_CONNECTION_ATTR.get(connection);
        if (ldapConnection == null) {
            // Not registered yet: nothing to close.
            return;
        }

        Result errorResult;
        if (error instanceof EOFException) {
@@ -446,6 +451,10 @@

    private final void handleReadException(FilterChainContext ctx, IOException e) {
        final GrizzlyLDAPConnection ldapConnection = LDAP_CONNECTION_ATTR.get(ctx.getConnection());
        if (ldapConnection == null) {
            // Already removed by handleClose: nothing to close, the caller passes the error on to Grizzly.
            return;
        }
        final Result errorResult =
                Responses.newResult(ResultCode.CLIENT_SIDE_DECODING_ERROR).setCause(e)
                        .setDiagnosticMessage(e.getMessage());

 opendj-grizzly/src/test/java/org/forgerock/opendj/grizzly/LDAPClientFilterTestCase.java

New file
@@ -0,0 +1,127 @@
/*
 * The contents of this file are subject to the terms of the Common Development and
 * Distribution License (the License). You may not use this file except in compliance with the
 * License.
 *
 * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the
 * specific language governing permission and limitations under the License.
 *
 * When distributing Covered Software, include this CDDL Header Notice in each file and include
 * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL
 * Header, with the fields enclosed by brackets [] replaced by your own identifying
 * information: "Portions copyright [year] [name of copyright owner]".
 *
 * Copyright 2026 3A Systems, LLC.
 */
package org.forgerock.opendj.grizzly;

import static org.fest.assertions.Assertions.assertThat;
import static org.fest.assertions.Fail.fail;
import static org.mockito.Mockito.*;

import java.io.IOException;

import org.forgerock.opendj.ldap.ConnectionEventListener;
import org.forgerock.opendj.ldap.DecodeException;
import org.forgerock.opendj.ldap.DecodeOptions;
import org.forgerock.opendj.ldap.LdapException;
import org.forgerock.opendj.ldap.ResultCode;
import org.forgerock.opendj.ldap.SdkTestCase;
import org.forgerock.util.Options;
import org.glassfish.grizzly.Connection;
import org.glassfish.grizzly.Grizzly;
import org.glassfish.grizzly.filterchain.FilterChainContext;
import org.glassfish.grizzly.memory.Buffers;
import org.glassfish.grizzly.memory.MemoryManager;
import org.mockito.ArgumentCaptor;
import org.testng.annotations.Test;

/**
 * Tests how the client filter handles errors on a connection with and without an LDAP connection
 * attached. None is attached after {@code handleClose} has removed it, or before
 * {@code registerConnection} has set it.
 */
@SuppressWarnings("javadoc")
public class LDAPClientFilterTestCase extends SdkTestCase {
    /** An UnbindRequest with message ID 1: a request is a decoding error on the client side. */
    private static final byte[] UNBIND_REQUEST = { 0x30, 0x05, 0x02, 0x01, 0x01, 0x42, 0x00 };

    private final LDAPClientFilter filter = new LDAPClientFilter(new DecodeOptions(), 0);

    @Test(expectedExceptions = DecodeException.class,
        expectedExceptionsMessageRegExp = "^Unexpected LDAP request: id=1, .*")
    public void handleReadWithoutLDAPConnectionThrowsTheDecodeError() throws Exception {
        filter.handleRead(newContext(newConnection(), UNBIND_REQUEST));
    }

    @Test
    public void exceptionOccurredWithoutLDAPConnectionReturns() throws Exception {
        filter.exceptionOccurred(newContext(newConnection(), null), new IOException("read failed"));
    }

    @Test
    public void handleReadClosesTheLDAPConnectionOnDecodeError() throws Exception {
        final Connection<?> connection = newConnection();
        final GrizzlyLDAPConnectionFactory factory = newFactory();
        try {
            final ConnectionEventListener listener = attachLDAPConnection(connection, factory);
            try {
                filter.handleRead(newContext(connection, UNBIND_REQUEST));
                fail("The client accepted an LDAP request");
            } catch (final DecodeException expected) {
                // The filter closes the LDAP connection and passes the error on to Grizzly.
            }
            assertConnectionError(listener, ResultCode.CLIENT_SIDE_DECODING_ERROR);
        } finally {
            factory.close();
        }
    }

    @Test
    public void exceptionOccurredClosesTheLDAPConnection() throws Exception {
        final Connection<?> connection = newConnection();
        final GrizzlyLDAPConnectionFactory factory = newFactory();
        try {
            final ConnectionEventListener listener = attachLDAPConnection(connection, factory);
            filter.exceptionOccurred(newContext(connection, null), new IOException("read failed"));
            assertConnectionError(listener, ResultCode.CLIENT_SIDE_LOCAL_ERROR);
        } finally {
            factory.close();
        }
    }

    /** Returns an open Grizzly connection with no LDAP connection attached. */
    private static Connection<?> newConnection() {
        final Connection<?> connection = mock(Connection.class);
        when(connection.getAttributes()).thenReturn(Grizzly.DEFAULT_ATTRIBUTE_BUILDER.createSafeAttributeHolder());
        when(connection.isOpen()).thenReturn(true);
        return connection;
    }

    private static FilterChainContext newContext(final Connection<?> connection, final byte[] message) {
        final FilterChainContext ctx = FilterChainContext.create(connection);
        if (message != null) {
            ctx.setMessage(Buffers.wrap(MemoryManager.DEFAULT_MEMORY_MANAGER, message));
        }
        return ctx;
    }

    private static GrizzlyLDAPConnectionFactory newFactory() {
        return new GrizzlyLDAPConnectionFactory("localhost", 0, Options.defaultOptions());
    }

    private ConnectionEventListener attachLDAPConnection(final Connection<?> connection,
            final GrizzlyLDAPConnectionFactory factory) {
        final GrizzlyLDAPConnection ldapConnection = new GrizzlyLDAPConnection(connection, factory);
        final ConnectionEventListener listener = mock(ConnectionEventListener.class);
        ldapConnection.addConnectionEventListener(listener);
        filter.registerConnection(connection, ldapConnection);
        return listener;
    }

    private static void assertConnectionError(final ConnectionEventListener listener, final ResultCode resultCode) {
        final ArgumentCaptor<LdapException> error = ArgumentCaptor.forClass(LdapException.class);
        verify(listener).handleConnectionError(eq(false), error.capture());
        assertThat(error.getValue().getResult().getResultCode()).isEqualTo(resultCode);
    }
}