From ca2e693934e06bebc22bd420414b74121f08f26f Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Mon, 01 Oct 2012 11:58:35 +0000
Subject: [PATCH] Fix OPENDJ-604: Support configuration of max request size in LDAPListener
---
opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPListenerImpl.java | 3
opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPListenerOptions.java | 44 +++++++++-
opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPServerFilter.java | 9 ++
opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPConnection.java | 1
opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPClientFilter.java | 3
opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPListenerTestCase.java | 137 ++++++++++++++++++++++++----------
6 files changed, 146 insertions(+), 51 deletions(-)
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPClientFilter.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPClientFilter.java
index 71c66bd..ee66956 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPClientFilter.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPClientFilter.java
@@ -450,7 +450,7 @@
public void exceptionOccurred(final FilterChainContext ctx, final Throwable error) {
final Connection<?> connection = ctx.getConnection();
if (!connection.isOpen()) {
- // Grizzly doens't not deregister the read interest from the
+ // Grizzly doesn't not deregister the read interest from the
// selector so closing the connection results in an EOFException.
// Just ignore errors on closed connections.
return;
@@ -474,7 +474,6 @@
final Connection<?> connection = ctx.getConnection();
final LDAPConnection ldapConnection = LDAP_CONNECTION_ATTR.remove(connection);
if (ldapConnection != null) {
- TimeoutChecker.INSTANCE.removeConnection(ldapConnection);
final Result errorResult = Responses.newResult(ResultCode.CLIENT_SIDE_SERVER_DOWN);
ldapConnection.close(null, false, errorResult);
}
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPConnection.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPConnection.java
index 9d6fb88a..23102b2 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPConnection.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPConnection.java
@@ -671,6 +671,7 @@
// Underlying channel prob blown up. Just ignore.
}
}
+ TimeoutChecker.INSTANCE.removeConnection(this);
connection.closeSilently();
// Notify listeners.
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPListenerImpl.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPListenerImpl.java
index e5b1000..5aa88b5 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPListenerImpl.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPListenerImpl.java
@@ -81,7 +81,8 @@
final DecodeOptions decodeOptions = new DecodeOptions(options.getDecodeOptions());
this.defaultFilterChain =
FilterChainBuilder.stateless().add(new TransportFilter()).add(
- new LDAPServerFilter(this, new LDAPReader(decodeOptions), 0)).build();
+ new LDAPServerFilter(this, new LDAPReader(decodeOptions), options
+ .getMaxRequestSize())).build();
this.serverConnection = transport.bind(address, options.getBacklog());
this.serverConnection.setProcessor(defaultFilterChain);
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPServerFilter.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPServerFilter.java
index 4cc3bab..18e2f4a 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPServerFilter.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPServerFilter.java
@@ -606,6 +606,9 @@
// following RFCs: 5289, 4346, 3268,4132 and 4162.
private static final Map<String, Integer> CIPHER_KEY_SIZES;
+ // Default maximum request size for incoming requests.
+ private static final int DEFAULT_MAX_REQUEST_SIZE = 5 * 1024 * 1024;
+
private static final Attribute<ASN1BufferReader> LDAP_ASN1_READER_ATTR =
Grizzly.DEFAULT_ATTRIBUTE_BUILDER.createAttribute("LDAPASN1Reader");
@@ -773,7 +776,8 @@
final int maxASN1ElementSize) {
this.listener = listener;
this.ldapReader = ldapReader;
- this.maxASN1ElementSize = maxASN1ElementSize;
+ this.maxASN1ElementSize =
+ maxASN1ElementSize <= 0 ? DEFAULT_MAX_REQUEST_SIZE : maxASN1ElementSize;
}
@Override
@@ -826,6 +830,9 @@
while (asn1Reader.elementAvailable()) {
ldapReader.decode(asn1Reader, serverRequestHandler, ctx);
}
+ } catch (IOException e) {
+ exceptionOccurred(ctx, e);
+ throw e;
} finally {
asn1Reader.disposeBytesRead();
}
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPListenerOptions.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPListenerOptions.java
index 5dba9b5..3290349 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPListenerOptions.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/LDAPListenerOptions.java
@@ -36,8 +36,9 @@
*/
public final class LDAPListenerOptions {
- private DecodeOptions decodeOptions;
private int backlog;
+ private DecodeOptions decodeOptions;
+ private int maxRequestSize;
private TCPNIOTransport transport;
/**
@@ -46,6 +47,7 @@
*/
public LDAPListenerOptions() {
this.backlog = 0;
+ this.maxRequestSize = 0;
this.decodeOptions = new DecodeOptions();
this.transport = null;
}
@@ -59,6 +61,7 @@
*/
public LDAPListenerOptions(final LDAPListenerOptions options) {
this.backlog = options.backlog;
+ this.maxRequestSize = options.maxRequestSize;
this.decodeOptions = new DecodeOptions(options.decodeOptions);
this.transport = options.transport;
}
@@ -71,7 +74,7 @@
*
* @return The maximum queue length for incoming connections requests.
*/
- public final int getBacklog() {
+ public int getBacklog() {
return backlog;
}
@@ -82,11 +85,23 @@
* @return The decoding options which will be used to control how requests
* and responses are decoded (never {@code null}).
*/
- public final DecodeOptions getDecodeOptions() {
+ public DecodeOptions getDecodeOptions() {
return decodeOptions;
}
/**
+ * Returns the maximum request size in bytes for incoming LDAP requests. If
+ * an incoming request exceeds the limit then the connection will be aborted
+ * by the listener. If the limit is less than {@code 1} then a default value
+ * of {@code 5MB} will be used.
+ *
+ * @return The maximum request size in bytes for incoming LDAP requests.
+ */
+ public int getMaxRequestSize() {
+ return maxRequestSize;
+ }
+
+ /**
* Returns the Grizzly TCP transport which will be used when initiating
* connections with the Directory Server.
* <p>
@@ -98,7 +113,7 @@
* default transport factory should be used to obtain a TCP
* transport.
*/
- public final TCPNIOTransport getTCPNIOTransport() {
+ public TCPNIOTransport getTCPNIOTransport() {
return transport;
}
@@ -112,7 +127,7 @@
* The maximum queue length for incoming connections requests.
* @return A reference to this LDAP listener options.
*/
- public final LDAPListenerOptions setBacklog(final int backlog) {
+ public LDAPListenerOptions setBacklog(final int backlog) {
this.backlog = backlog;
return this;
}
@@ -128,13 +143,28 @@
* @throws NullPointerException
* If {@code decodeOptions} was {@code null}.
*/
- public final LDAPListenerOptions setDecodeOptions(final DecodeOptions decodeOptions) {
+ public LDAPListenerOptions setDecodeOptions(final DecodeOptions decodeOptions) {
Validator.ensureNotNull(decodeOptions);
this.decodeOptions = decodeOptions;
return this;
}
/**
+ * Sets the maximum request size in bytes for incoming LDAP requests. If an
+ * incoming request exceeds the limit then the connection will be aborted by
+ * the listener. If the limit is less than {@code 1} then a default value of
+ * {@code 5MB} will be used.
+ *
+ * @param maxRequestSize
+ * The maximum request size in bytes for incoming LDAP requests.
+ * @return A reference to this LDAP listener options.
+ */
+ public LDAPListenerOptions setMaxRequestSize(final int maxRequestSize) {
+ this.maxRequestSize = maxRequestSize;
+ return this;
+ }
+
+ /**
* Sets the Grizzly TCP transport which will be used when initiating
* connections with the Directory Server.
* <p>
@@ -148,7 +178,7 @@
* transport.
* @return A reference to this connection options.
*/
- public final LDAPListenerOptions setTCPNIOTransport(final TCPNIOTransport transport) {
+ public LDAPListenerOptions setTCPNIOTransport(final TCPNIOTransport transport) {
this.transport = transport;
return this;
}
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPListenerTestCase.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPListenerTestCase.java
index 1a9fc25..ca24716 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPListenerTestCase.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPListenerTestCase.java
@@ -27,6 +27,9 @@
package org.forgerock.opendj.ldap;
+import static org.fest.assertions.Assertions.assertThat;
+import static org.fest.assertions.Fail.fail;
+
import java.util.Arrays;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
@@ -47,11 +50,11 @@
import org.forgerock.opendj.ldap.responses.ExtendedResult;
import org.forgerock.opendj.ldap.responses.Responses;
import org.forgerock.opendj.ldap.responses.Result;
-import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+import com.forgerock.opendj.util.AsynchronousFutureResult;
import com.forgerock.opendj.util.StaticUtils;
/**
@@ -60,8 +63,10 @@
public class LDAPListenerTestCase extends SdkTestCase {
private static class MockServerConnection implements ServerConnection<Integer> {
- volatile LDAPClientContext context = null;
- final CountDownLatch isConnected = new CountDownLatch(1);
+ final AsynchronousFutureResult<Throwable> connectionError =
+ new AsynchronousFutureResult<Throwable>(null);
+ final AsynchronousFutureResult<LDAPClientContext> context =
+ new AsynchronousFutureResult<LDAPClientContext>(null);
final CountDownLatch isClosed = new CountDownLatch(1);
MockServerConnection() {
@@ -132,7 +137,7 @@
*/
@Override
public void handleConnectionError(final Throwable error) {
- // Do nothing.
+ connectionError.handleResult(error);
}
/**
@@ -208,8 +213,7 @@
@Override
public ServerConnection<Integer> handleAccept(final LDAPClientContext clientContext)
throws ErrorResultException {
- serverConnection.context = clientContext;
- serverConnection.isConnected.countDown();
+ serverConnection.context.handleResult(clientContext);
return serverConnection;
}
}
@@ -248,12 +252,10 @@
final Connection connection =
new LDAPConnectionFactory(listener.getSocketAddress()).getConnection();
- Assert.assertTrue(serverConnection.isConnected.await(10, TimeUnit.SECONDS));
- Assert.assertEquals(serverConnection.isClosed.getCount(), 1);
-
+ assertThat(serverConnection.context.get(10, TimeUnit.SECONDS)).isNotNull();
+ assertThat(serverConnection.isClosed.getCount()).isEqualTo(1);
connection.close();
-
- Assert.assertTrue(serverConnection.isClosed.await(10, TimeUnit.SECONDS));
+ assertThat(serverConnection.isClosed.await(10, TimeUnit.SECONDS)).isTrue();
} finally {
listener.close();
}
@@ -321,8 +323,8 @@
final Connection connection =
new LDAPConnectionFactory(proxyListener.getSocketAddress()).getConnection();
- Assert.assertTrue(proxyServerConnection.isConnected.await(10, TimeUnit.SECONDS));
- Assert.assertTrue(onlineServerConnection.isConnected.await(10, TimeUnit.SECONDS));
+ assertThat(proxyServerConnection.context.get(10, TimeUnit.SECONDS)).isNotNull();
+ assertThat(onlineServerConnection.context.get(10, TimeUnit.SECONDS)).isNotNull();
// Wait for connect/close to complete.
connection.close();
@@ -412,9 +414,9 @@
try {
connection.bind("cn=test", "password".toCharArray());
- Assert.assertTrue(proxyServerConnection.isConnected.await(10, TimeUnit.SECONDS));
- Assert.assertTrue(onlineServerConnection.isConnected
- .await(10, TimeUnit.SECONDS));
+ assertThat(proxyServerConnection.context.get(10, TimeUnit.SECONDS)).isNotNull();
+ assertThat(onlineServerConnection.context.get(10, TimeUnit.SECONDS))
+ .isNotNull();
} finally {
connection.close();
}
@@ -500,8 +502,8 @@
final Connection connection =
new LDAPConnectionFactory(proxyListener.getSocketAddress()).getConnection();
- Assert.assertTrue(proxyServerConnection.isConnected.await(10, TimeUnit.SECONDS));
- Assert.assertTrue(onlineServerConnection.isConnected.await(10, TimeUnit.SECONDS));
+ assertThat(proxyServerConnection.context.get(10, TimeUnit.SECONDS)).isNotNull();
+ assertThat(onlineServerConnection.context.get(10, TimeUnit.SECONDS)).isNotNull();
connection.close();
@@ -589,9 +591,9 @@
try {
connection.bind("cn=test", "password".toCharArray());
- Assert.assertTrue(proxyServerConnection.isConnected.await(10, TimeUnit.SECONDS));
- Assert.assertTrue(onlineServerConnection.isConnected
- .await(10, TimeUnit.SECONDS));
+ assertThat(proxyServerConnection.context.get(10, TimeUnit.SECONDS)).isNotNull();
+ assertThat(onlineServerConnection.context.get(10, TimeUnit.SECONDS))
+ .isNotNull();
} finally {
connection.close();
}
@@ -607,6 +609,65 @@
}
/**
+ * Tests that an incoming request which is too big triggers the connection
+ * to be closed and an error notification to occur.
+ *
+ * @throws Exception
+ * If an unexpected error occurred.
+ */
+ @Test
+ public void testMaxRequestSize() throws Exception {
+ final MockServerConnection serverConnection = new MockServerConnection();
+ final MockServerConnectionFactory factory =
+ new MockServerConnectionFactory(serverConnection);
+ final LDAPListenerOptions options = new LDAPListenerOptions().setMaxRequestSize(2048);
+ final LDAPListener listener =
+ new LDAPListener("localhost", TestCaseUtils.findFreePort(), factory, options);
+
+ Connection connection = null;
+ try {
+ connection = new LDAPConnectionFactory(listener.getSocketAddress()).getConnection();
+
+ // Small request
+ connection.bind("cn=test", "password".toCharArray());
+ assertThat(serverConnection.context.get().isClosed()).isFalse();
+ assertThat(serverConnection.connectionError.isDone()).isFalse();
+
+ // Big but valid request.
+ final char[] password1 = new char[2000];
+ Arrays.fill(password1, 'a');
+ connection.bind("cn=test", password1);
+ assertThat(serverConnection.context.get().isClosed()).isFalse();
+ assertThat(serverConnection.connectionError.isDone()).isFalse();
+
+ // Big invalid request.
+ final char[] password2 = new char[2048];
+ Arrays.fill(password2, 'a');
+ try {
+ connection.bind("cn=test", password2);
+ fail("Big bind unexpectedly succeeded");
+ } catch (final ErrorResultException e) {
+ // Expected exception.
+ assertThat(e.getResult().getResultCode()).isEqualTo(
+ ResultCode.CLIENT_SIDE_SERVER_DOWN);
+
+ assertThat(serverConnection.connectionError.get(10, TimeUnit.SECONDS)).isNotNull();
+ assertThat(serverConnection.connectionError.get()).isInstanceOf(
+ DecodeException.class);
+ assertThat(((DecodeException) serverConnection.connectionError.get()).isFatal())
+ .isTrue();
+ assertThat(serverConnection.isClosed.getCount()).isEqualTo(1);
+ assertThat(serverConnection.context.get().isClosed()).isTrue();
+ }
+ } finally {
+ if (connection != null) {
+ connection.close();
+ }
+ listener.close();
+ }
+ }
+
+ /**
* Tests server-side disconnection.
*
* @throws Exception
@@ -614,19 +675,16 @@
*/
@Test
public void testServerDisconnect() throws Exception {
- final MockServerConnection onlineServerConnection = new MockServerConnection();
- final MockServerConnectionFactory onlineServerConnectionFactory =
- new MockServerConnectionFactory(onlineServerConnection);
- final LDAPListener onlineServerListener =
- new LDAPListener("localhost", TestCaseUtils.findFreePort(),
- onlineServerConnectionFactory);
+ final MockServerConnection serverConnection = new MockServerConnection();
+ final MockServerConnectionFactory factory =
+ new MockServerConnectionFactory(serverConnection);
+ final LDAPListener listener =
+ new LDAPListener("localhost", TestCaseUtils.findFreePort(), factory);
final Connection connection;
try {
// Connect and bind.
- connection =
- new LDAPConnectionFactory(onlineServerListener.getSocketAddress())
- .getConnection();
+ connection = new LDAPConnectionFactory(listener.getSocketAddress()).getConnection();
try {
connection.bind("cn=test", "password".toCharArray());
} catch (final ErrorResultException e) {
@@ -634,33 +692,32 @@
throw e;
}
} finally {
- onlineServerConnection.context.disconnect();
- onlineServerListener.close();
+ serverConnection.context.get().disconnect();
+ listener.close();
}
try {
// Connect and bind.
final Connection failedConnection =
- new LDAPConnectionFactory(onlineServerListener.getSocketAddress())
- .getConnection();
+ new LDAPConnectionFactory(listener.getSocketAddress()).getConnection();
failedConnection.close();
connection.close();
- Assert.fail("Connection attempt to closed listener succeeded unexpectedly");
+ fail("Connection attempt to closed listener succeeded unexpectedly");
} catch (final ConnectionException e) {
// Expected.
}
try {
connection.bind("cn=test", "password".toCharArray());
- Assert.fail("Bind attempt on closed connection succeeded unexpectedly");
+ fail("Bind attempt on closed connection succeeded unexpectedly");
} catch (final ErrorResultException e) {
// Expected.
- Assert.assertFalse(connection.isValid());
- Assert.assertFalse(connection.isClosed());
+ assertThat(connection.isValid()).isFalse();
+ assertThat(connection.isClosed()).isFalse();
} finally {
connection.close();
- Assert.assertFalse(connection.isValid());
- Assert.assertTrue(connection.isClosed());
+ assertThat(connection.isValid()).isFalse();
+ assertThat(connection.isClosed()).isTrue();
}
}
}
--
Gitblit v1.10.0