Fix for OPENDJ-856: Make Rest2LDAP close the cached authenticated connection.

Matthew Swift

09.03.2013 857c966db460df76c426946b54074d004b04ef4c

Fix for OPENDJ-856: Make Rest2LDAP close the cached authenticated connection.

 opendj3/opendj-rest2ldap-servlet/src/main/java/org/forgerock/opendj/rest2ldap/servlet/Rest2LDAPAuthnFilter.java

@@ -56,6 +56,7 @@
import org.forgerock.opendj.ldap.ByteString;
import org.forgerock.opendj.ldap.Connection;
import org.forgerock.opendj.ldap.ConnectionFactory;
import org.forgerock.opendj.ldap.Connections;
import org.forgerock.opendj.ldap.DN;
import org.forgerock.opendj.ldap.EntryNotFoundException;
import org.forgerock.opendj.ldap.ErrorResultException;
@@ -430,9 +431,14 @@

                    @Override
                    public void handleResult(final BindResult result) {
                        // Cache the pre-authenticated connection.
                        /*
                         * Cache the pre-authenticated connection and prevent
                         * downstream components from closing it since this
                         * filter will close it.
                         */
                        if (reuseAuthenticatedConnection) {
                            request.setAttribute(ATTRIBUTE_AUTHN_CONNECTION, connection);
                            request.setAttribute(ATTRIBUTE_AUTHN_CONNECTION, Connections
                                    .uncloseable(connection));
                        }

                        // Pass through the authentication ID and authorization principals.

 opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Context.java

@@ -17,13 +17,13 @@

import static org.forgerock.opendj.ldap.ErrorResultException.newErrorResult;
import static org.forgerock.opendj.rest2ldap.Rest2LDAP.asResourceException;
import static org.forgerock.opendj.rest2ldap.Utils.i18n;

import java.io.Closeable;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicReference;

import org.forgerock.json.resource.InternalServerErrorException;
import org.forgerock.json.resource.ResourceException;
@@ -121,7 +121,10 @@
        }

        FutureResult<Result> getFutureResult() {
            // Perform uninterrupted wait since this method is unlikely to block for a long time.
            /*
             * Perform uninterrupted wait since this method is unlikely to block
             * for a long time.
             */
            boolean wasInterrupted = false;
            while (true) {
                try {
@@ -197,23 +200,25 @@
    };

    private final Config config;
    private final AtomicReference<Connection> connection = new AtomicReference<Connection>();
    private final ServerContext context;
    private final Connection preAuthenticatedConnection;
    private Connection connection;
    private Control proxiedAuthzControl = null;

    Context(final Config config, final ServerContext context) {
        this.config = config;
        this.context = context;

        // Re-use the pre-authenticated connection if available and the authorization policy allows.
        /*
         * Re-use the pre-authenticated connection if available and the
         * authorization policy allows.
         */
        if (config.getAuthorizationPolicy() != AuthorizationPolicy.NONE
                && context.containsContext(AuthenticatedConnectionContext.class)) {
            final Connection connection =
                    context.asContext(AuthenticatedConnectionContext.class).getConnection();
            this.preAuthenticatedConnection = connection != null ? wrap(connection) : null;
            this.connection = wrap(connection);
        } else {
            this.preAuthenticatedConnection = null;
            this.connection = null; // We'll allocate the connection.
        }
    }

@@ -222,15 +227,7 @@
     */
    @Override
    public void close() {
        /*
         * Only release the connection that we acquired. Don't release the
         * cached connection since that is the responsibility of the component
         * which acquired it.
         */
        final Connection c = connection.getAndSet(null);
        if (c != null) {
            c.close();
        }
        connection.close();
    }

    Config getConfig() {
@@ -238,7 +235,7 @@
    }

    Connection getConnection() {
        return preAuthenticatedConnection != null ? preAuthenticatedConnection : connection.get();
        return connection;
    }

    ServerContext getServerContext() {
@@ -268,8 +265,7 @@
         * cached connection since cached connections are supposed to have been
         * pre-authenticated and therefore do not require proxied authorization.
         */
        if (preAuthenticatedConnection == null
                && config.getAuthorizationPolicy() == AuthorizationPolicy.PROXY) {
        if (connection == null && config.getAuthorizationPolicy() == AuthorizationPolicy.PROXY) {
            if (context.containsContext(SecurityContext.class)) {
                try {
                    final SecurityContext securityContext =
@@ -283,9 +279,9 @@
                    return;
                }
            } else {
                // FIXME: i18n.
                handler.handleError(new InternalServerErrorException(
                        "The request could not be authorized because it did not contain a security context"));
                        i18n("The request could not be authorized because it did "
                                + "not contain a security context")));
                return;
            }
        }
@@ -295,7 +291,7 @@
         * requests. A null factory indicates that Rest2LDAP has been configured
         * to re-use the LDAP connection which was used for authentication.
         */
        if (preAuthenticatedConnection != null) {
        if (connection != null) {
            // Invoke the handler immediately since a connection is available.
            runnable.run();
        } else if (config.connectionFactory() != null) {
@@ -307,17 +303,14 @@

                @Override
                public final void handleResult(final Connection result) {
                    if (!connection.compareAndSet(null, wrap(result))) {
                        // This should never happen.
                        throw new IllegalStateException("LDAP connection obtained multiple times");
                    }
                    connection = wrap(result);
                    runnable.run();
                }
            });
        } else {
            // FIXME: i18n
            handler.handleError(new InternalServerErrorException(
                    "The request could not be processed because there was no LDAP connection available for use"));
                    i18n("The request could not be processed because there was no LDAP "
                            + "connection available for use")));
        }
    }


 opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Utils.java

@@ -57,10 +57,10 @@
     *            The type of result.
     */
    private static final class AccumulatingResultHandler<V> implements ResultHandler<V> {
        private ResourceException exception; // Guarded by latch.
        private final ResultHandler<List<V>> handler;
        private final AtomicInteger latch;
        private final List<V> results;
        private ResourceException exception; // Guarded by latch.

        private AccumulatingResultHandler(final int size, final ResultHandler<List<V>> handler) {
            if (size <= 0) {
@@ -88,9 +88,12 @@
        }

        private void latch() {
            // Invoke the handler once all results have been received. Avoid failing-fast
            // when an error occurs because some in-flight tasks may depend on resources
            // (e.g. connections) which are automatically closed on completion.
            /*
             * Invoke the handler once all results have been received. Avoid
             * failing-fast when an error occurs because some in-flight tasks
             * may depend on resources (e.g. connections) which are
             * automatically closed on completion.
             */
            if (latch.decrementAndGet() == 0) {
                if (exception != null) {
                    handler.handleError(exception);
@@ -217,6 +220,19 @@
        return a.getAttributeDescription().withoutOption("binary").toString();
    }

    /**
     * Stub formatter for i18n strings.
     *
     * @param format
     *            The format string.
     * @param args
     *            The string arguments.
     * @return The formatted string.
     */
    static String i18n(final String format, final Object... args) {
        return String.format(format, args);
    }

    static boolean isJSONPrimitive(final Object value) {
        return value instanceof String || value instanceof Boolean || value instanceof Number;
    }
@@ -322,7 +338,7 @@
            public void handleResult(final M result) {
                try {
                    handler.handleResult(f.apply(result, null));
                } catch (Throwable t) {
                } catch (final Throwable t) {
                    handler.handleError(asResourceException(t));
                }
            }