DIRAPI-368, DIRSERVER-2340: Fix StackOverflowError
Fix StackOverflowError when working with entries with thousands
of attributes or values. Change the recursive algorithm to an
iterative one.
diff --git a/ldap/codec/core/src/main/java/org/apache/directory/api/ldap/codec/factory/AddRequestFactory.java b/ldap/codec/core/src/main/java/org/apache/directory/api/ldap/codec/factory/AddRequestFactory.java
index 7431d7a..66f6f75 100644
--- a/ldap/codec/core/src/main/java/org/apache/directory/api/ldap/codec/factory/AddRequestFactory.java
+++ b/ldap/codec/core/src/main/java/org/apache/directory/api/ldap/codec/factory/AddRequestFactory.java
@@ -30,6 +30,7 @@
import org.apache.directory.api.ldap.model.entry.Value;
import org.apache.directory.api.ldap.model.message.AddRequest;
import org.apache.directory.api.ldap.model.message.Message;
+import org.apache.directory.api.util.CollectionUtils;
import org.apache.directory.api.util.Strings;
/**
@@ -49,7 +50,7 @@
/**
- * Encode an entry's Attribute's values. It's done recursively, to have the
+ * Encode an entry's Attribute's values. It's done in reverse order, to have the
* last value encoded first in the reverse buffer.
* <br>
* The values are encoded this way :
@@ -65,13 +66,11 @@
*/
private void encodeValueReverse( Asn1Buffer buffer, Iterator<Value> iterator )
{
- if ( iterator.hasNext() )
+ iterator = CollectionUtils.reverse( iterator );
+ while ( iterator.hasNext() )
{
Value value = iterator.next();
- // Recursive call to have the latest values encoded first
- encodeValueReverse( buffer, iterator );
-
// Encode the value
BerValue.encodeOctetString( buffer, value.getBytes() );
}
@@ -80,7 +79,7 @@
/**
* Encode the attributes, starting from the end. We iterate through the list
- * of attributes, recursively. The last attribute will be encoded first, when
+ * of attributes in reverse order. The last attribute will be encoded first, when
* the end of the list will be reached, which is what we went, as we encode from
* the end.
* <br>
@@ -99,13 +98,11 @@
*/
private void encodeAttributeReverse( Asn1Buffer buffer, Iterator<Attribute> iterator )
{
- if ( iterator.hasNext() )
+ iterator = CollectionUtils.reverse( iterator );
+ while ( iterator.hasNext() )
{
Attribute attribute = iterator.next();
- // Recursive call to have the latest attributes encoded first
- encodeAttributeReverse( buffer, iterator );
-
// Remind the current position
int start = buffer.getPos();
diff --git a/ldap/codec/core/src/main/java/org/apache/directory/api/ldap/codec/factory/ModifyRequestFactory.java b/ldap/codec/core/src/main/java/org/apache/directory/api/ldap/codec/factory/ModifyRequestFactory.java
index 7d044d7..a8fefa2 100644
--- a/ldap/codec/core/src/main/java/org/apache/directory/api/ldap/codec/factory/ModifyRequestFactory.java
+++ b/ldap/codec/core/src/main/java/org/apache/directory/api/ldap/codec/factory/ModifyRequestFactory.java
@@ -32,6 +32,7 @@
import org.apache.directory.api.ldap.model.entry.Value;
import org.apache.directory.api.ldap.model.message.Message;
import org.apache.directory.api.ldap.model.message.ModifyRequest;
+import org.apache.directory.api.util.CollectionUtils;
/**
* The ModifyRequest factory.
@@ -50,7 +51,7 @@
/**
- * Encode the values, recursively
+ * Encode the values, in reverse order
* <pre>
* 0x04 LL attributeValue
* ...
@@ -62,13 +63,11 @@
*/
private void encodeValues( Asn1Buffer buffer, Iterator<Value> values )
{
- if ( values.hasNext() )
+ values = CollectionUtils.reverse( values );
+ while ( values.hasNext() )
{
Value value = values.next();
- // Recurse on the values
- encodeValues( buffer, values );
-
// The value
if ( value.isHumanReadable() )
{
@@ -82,7 +81,7 @@
}
/**
- * Recursively encode the modifications, starting from the last one.
+ * Encode the modifications, starting from the last one.
* <pre>
* 0x30 LL modification sequence
* 0x0A 0x01 operation
@@ -99,13 +98,11 @@
*/
private void encodeModifications( Asn1Buffer buffer, Iterator<Modification> modifications )
{
- if ( modifications.hasNext() )
+ modifications = CollectionUtils.reverse( modifications );
+ while ( modifications.hasNext() )
{
Modification modification = modifications.next();
- // Recurse
- encodeModifications( buffer, modifications );
-
int start = buffer.getPos();
// The Attribute
diff --git a/ldap/codec/core/src/main/java/org/apache/directory/api/ldap/codec/factory/SearchResultEntryFactory.java b/ldap/codec/core/src/main/java/org/apache/directory/api/ldap/codec/factory/SearchResultEntryFactory.java
index 6afc4ee..55cd32e 100644
--- a/ldap/codec/core/src/main/java/org/apache/directory/api/ldap/codec/factory/SearchResultEntryFactory.java
+++ b/ldap/codec/core/src/main/java/org/apache/directory/api/ldap/codec/factory/SearchResultEntryFactory.java
@@ -30,6 +30,7 @@
import org.apache.directory.api.ldap.model.entry.Value;
import org.apache.directory.api.ldap.model.message.Message;
import org.apache.directory.api.ldap.model.message.SearchResultEntry;
+import org.apache.directory.api.util.CollectionUtils;
/**
* The SearchResultEntry factory.
@@ -48,7 +49,7 @@
/**
- * Encode the values recursively
+ * Encode the values in reverse order.
*
* <pre>
* 0x04 LL attributeValue
@@ -61,12 +62,11 @@
*/
private void encodeValues( Asn1Buffer buffer, Iterator<Value> values )
{
- if ( values.hasNext() )
+ values = CollectionUtils.reverse( values );
+ while ( values.hasNext() )
{
Value value = values.next();
- encodeValues( buffer, values );
-
// The value
if ( value.isHumanReadable() )
{
@@ -81,7 +81,7 @@
/**
- * Encode the attributes recursively
+ * Encode the attributes in reverse order.
*
* <pre>
* 0x30 LL partialAttributeList
@@ -97,16 +97,14 @@
*/
private void encodeAttributes( Asn1Buffer buffer, Iterator<Attribute> attributes )
{
- if ( attributes.hasNext() )
+ attributes = CollectionUtils.reverse( attributes );
+ while ( attributes.hasNext() )
{
Attribute attribute = attributes.next();
- // Recursive call
- encodeAttributes( buffer, attributes );
-
int start = buffer.getPos();
- // The values, recursively, if any
+ // The values if any
if ( attribute.size() != 0 )
{
encodeValues( buffer, attribute.iterator() );
@@ -160,7 +158,7 @@
// The partial attribute list
Entry entry = searchResultEntry.getEntry();
- // The attributes, recursively, if we have any
+ // The attributes, if we have any
if ( ( entry != null ) && ( entry.size() != 0 ) )
{
encodeAttributes( buffer, entry.iterator() );
diff --git a/ldap/codec/core/src/test/java/org/apache/directory/api/ldap/codec/add/AddRequestTest.java b/ldap/codec/core/src/test/java/org/apache/directory/api/ldap/codec/add/AddRequestTest.java
index 30424f9..94f0911 100644
--- a/ldap/codec/core/src/test/java/org/apache/directory/api/ldap/codec/add/AddRequestTest.java
+++ b/ldap/codec/core/src/test/java/org/apache/directory/api/ldap/codec/add/AddRequestTest.java
@@ -30,6 +30,7 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
+import java.util.stream.IntStream;
import org.apache.directory.api.asn1.DecoderException;
import org.apache.directory.api.asn1.EncoderException;
@@ -40,8 +41,10 @@
import org.apache.directory.api.ldap.codec.api.ResponseCarryingException;
import org.apache.directory.api.ldap.codec.osgi.AbstractCodecServiceTest;
import org.apache.directory.api.ldap.model.entry.Attribute;
+import org.apache.directory.api.ldap.model.entry.DefaultEntry;
import org.apache.directory.api.ldap.model.entry.Entry;
import org.apache.directory.api.ldap.model.entry.Value;
+import org.apache.directory.api.ldap.model.exception.LdapException;
import org.apache.directory.api.ldap.model.message.AddRequest;
import org.apache.directory.api.ldap.model.message.AddRequestImpl;
import org.apache.directory.api.ldap.model.message.AddResponseImpl;
@@ -50,6 +53,7 @@
import org.apache.directory.api.ldap.model.message.ResultCodeEnum;
import org.apache.directory.api.ldap.model.message.controls.ManageDsaIT;
import org.apache.directory.api.ldap.model.message.controls.ManageDsaITImpl;
+import org.apache.directory.api.ldap.model.name.Dn;
import org.apache.directory.api.util.Strings;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.Execution;
@@ -693,4 +697,39 @@
assertArrayEquals( stream.array(), asn1Buffer.getBytes().array() );
}
+
+
+ /**
+ * Test that encoding and decoding of an add request with 10k attributes and 10k values
+ * succeeds without StackOverflowError (DIRAPI-368, DIRSERVER-2340).
+ */
+ @Test
+ public void testEncodeDecodeLarge() throws DecoderException, EncoderException, LdapException
+ {
+ Asn1Buffer buffer = new Asn1Buffer();
+
+ AddRequest originalAddRequest = new AddRequestImpl();
+ originalAddRequest.setMessageId( 3 );
+ Dn dn = new Dn( "cn=test,ou=users,ou=system" );
+ originalAddRequest.setEntryDn( dn );
+ Entry entry = new DefaultEntry( dn );
+ for ( int attributeIndex = 0; attributeIndex < 10000; attributeIndex++ )
+ {
+ entry.add( "objectclass" + attributeIndex, "top", "person" );
+ }
+ String[] values = IntStream.range( 0, 10000 ).boxed().map( i -> "value" + i ).toArray( String[]::new );
+ entry.add( "objectclass", values );
+ originalAddRequest.setEntry( entry );
+
+ LdapEncoder.encodeMessage( buffer, codec, originalAddRequest );
+
+ LdapMessageContainer<AddRequest> ldapMessageContainer = new LdapMessageContainer<>( codec );
+
+ Asn1Decoder.decode( buffer.getBytes(), ldapMessageContainer );
+
+ AddRequest decodedAddRequest = ldapMessageContainer.getMessage();
+
+ assertEquals( originalAddRequest, decodedAddRequest );
+ }
+
}
diff --git a/ldap/codec/core/src/test/java/org/apache/directory/api/ldap/codec/modify/ModifyRequestTest.java b/ldap/codec/core/src/test/java/org/apache/directory/api/ldap/codec/modify/ModifyRequestTest.java
index 4c95a97..7ee38b8 100644
--- a/ldap/codec/core/src/test/java/org/apache/directory/api/ldap/codec/modify/ModifyRequestTest.java
+++ b/ldap/codec/core/src/test/java/org/apache/directory/api/ldap/codec/modify/ModifyRequestTest.java
@@ -29,6 +29,7 @@
import java.nio.ByteBuffer;
import java.util.Collection;
import java.util.Map;
+import java.util.stream.IntStream;
import org.apache.directory.api.asn1.DecoderException;
import org.apache.directory.api.asn1.EncoderException;
@@ -39,15 +40,18 @@
import org.apache.directory.api.ldap.codec.api.ResponseCarryingException;
import org.apache.directory.api.ldap.codec.osgi.AbstractCodecServiceTest;
import org.apache.directory.api.ldap.model.entry.Attribute;
+import org.apache.directory.api.ldap.model.entry.DefaultModification;
import org.apache.directory.api.ldap.model.entry.Modification;
import org.apache.directory.api.ldap.model.entry.ModificationOperation;
import org.apache.directory.api.ldap.model.exception.LdapException;
import org.apache.directory.api.ldap.model.message.Control;
import org.apache.directory.api.ldap.model.message.Message;
import org.apache.directory.api.ldap.model.message.ModifyRequest;
+import org.apache.directory.api.ldap.model.message.ModifyRequestImpl;
import org.apache.directory.api.ldap.model.message.ModifyResponseImpl;
import org.apache.directory.api.ldap.model.message.ResultCodeEnum;
import org.apache.directory.api.ldap.model.message.controls.ManageDsaIT;
+import org.apache.directory.api.ldap.model.name.Dn;
import org.apache.directory.api.util.Strings;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.Execution;
@@ -1223,4 +1227,39 @@
assertArrayEquals( stream.array(), buffer.getBytes().array() );
}
+
+
+ /**
+ * Test that encoding and decoding of a modify request with 10k attributes and 10k values
+ * succeeds without StackOverflowError (DIRAPI-368, DIRSERVER-2340).
+ */
+ @Test
+ public void testEncodeDecodeLarge() throws DecoderException, EncoderException, LdapException
+ {
+ Asn1Buffer buffer = new Asn1Buffer();
+
+ ModifyRequest originalModifyRequest = new ModifyRequestImpl();
+ originalModifyRequest.setMessageId( 3 );
+ Dn dn = new Dn( "cn=test,ou=users,ou=system" );
+ originalModifyRequest.setName( dn );
+ for ( int modIndex = 0; modIndex < 10000; modIndex++ )
+ {
+ originalModifyRequest.addModification( new DefaultModification( ModificationOperation.REPLACE_ATTRIBUTE,
+ "objectclass" + modIndex, "top", "person" ) );
+ }
+ String[] values = IntStream.range( 0, 10000 ).boxed().map( i -> "value" + i ).toArray( String[]::new );
+ originalModifyRequest.addModification(
+ new DefaultModification( ModificationOperation.REPLACE_ATTRIBUTE, "objectclass", values ) );
+
+ LdapEncoder.encodeMessage( buffer, codec, originalModifyRequest );
+
+ LdapMessageContainer<ModifyRequest> ldapMessageContainer = new LdapMessageContainer<>( codec );
+
+ Asn1Decoder.decode( buffer.getBytes(), ldapMessageContainer );
+
+ ModifyRequest decodedModifyRequest = ldapMessageContainer.getMessage();
+
+ assertEquals( originalModifyRequest, decodedModifyRequest );
+ }
+
}
diff --git a/ldap/codec/core/src/test/java/org/apache/directory/api/ldap/codec/search/SearchResultEntryTest.java b/ldap/codec/core/src/test/java/org/apache/directory/api/ldap/codec/search/SearchResultEntryTest.java
index 9b168dd..33f8748 100644
--- a/ldap/codec/core/src/test/java/org/apache/directory/api/ldap/codec/search/SearchResultEntryTest.java
+++ b/ldap/codec/core/src/test/java/org/apache/directory/api/ldap/codec/search/SearchResultEntryTest.java
@@ -28,6 +28,7 @@
import java.nio.ByteBuffer;
import java.util.Arrays;
import java.util.Map;
+import java.util.stream.IntStream;
import org.apache.directory.api.asn1.DecoderException;
import org.apache.directory.api.asn1.EncoderException;
@@ -37,11 +38,14 @@
import org.apache.directory.api.ldap.codec.api.LdapMessageContainer;
import org.apache.directory.api.ldap.codec.osgi.AbstractCodecServiceTest;
import org.apache.directory.api.ldap.model.entry.Attribute;
+import org.apache.directory.api.ldap.model.entry.DefaultEntry;
import org.apache.directory.api.ldap.model.entry.Entry;
import org.apache.directory.api.ldap.model.exception.LdapException;
import org.apache.directory.api.ldap.model.message.Control;
import org.apache.directory.api.ldap.model.message.SearchResultEntry;
+import org.apache.directory.api.ldap.model.message.SearchResultEntryImpl;
import org.apache.directory.api.ldap.model.message.controls.EntryChange;
+import org.apache.directory.api.ldap.model.name.Dn;
import org.apache.directory.api.util.Strings;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.Execution;
@@ -122,7 +126,6 @@
assertTrue( Arrays.equals( stream.array(), buffer.getBytes().array() ) );
}
-
/**
* Test the decoding of a SearchResultEntry
*/
@@ -971,4 +974,39 @@
assertArrayEquals( stream.array(), result.array() );
}
+
+
+ /**
+ * Test that encoding and decoding of a search result entry with 10k attributes and 10k values
+ * succeeds without StackOverflowError (DIRAPI-368, DIRSERVER-2340).
+ */
+ @Test
+ public void testEncodeDecodeLarge() throws DecoderException, EncoderException, LdapException
+ {
+ Asn1Buffer buffer = new Asn1Buffer();
+
+ SearchResultEntry originalSearchResultEntry = new SearchResultEntryImpl();
+ originalSearchResultEntry.setMessageId( 3 );
+ Dn dn = new Dn( "cn=test,ou=users,ou=system" );
+ originalSearchResultEntry.setObjectName( dn );
+ Entry entry = new DefaultEntry( dn );
+ for ( int attributeIndex = 0; attributeIndex < 10000; attributeIndex++ )
+ {
+ entry.add( "objectclass" + attributeIndex, "top", "person" );
+ }
+ String[] values = IntStream.range( 0, 10000 ).boxed().map( i -> "value" + i ).toArray( String[]::new );
+ entry.add( "objectclass", values );
+ originalSearchResultEntry.setEntry( entry );
+
+ LdapEncoder.encodeMessage( buffer, codec, originalSearchResultEntry );
+
+ LdapMessageContainer<SearchResultEntry> ldapMessageContainer = new LdapMessageContainer<>( codec );
+
+ Asn1Decoder.decode( buffer.getBytes(), ldapMessageContainer );
+
+ SearchResultEntry decodedSearchResultEntry = ldapMessageContainer.getMessage();
+
+ assertEquals( originalSearchResultEntry, decodedSearchResultEntry );
+ }
+
}
diff --git a/util/src/main/java/org/apache/directory/api/util/CollectionUtils.java b/util/src/main/java/org/apache/directory/api/util/CollectionUtils.java
new file mode 100644
index 0000000..acc5b43
--- /dev/null
+++ b/util/src/main/java/org/apache/directory/api/util/CollectionUtils.java
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.directory.api.util;
+
+
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+
+/**
+ * Collection and Iterator utils.
+ *
+ * @author <a href="mailto:dev@directory.apache.org">Apache Directory Project</a>
+ */
+public final class CollectionUtils
+{
+ public static <T> Iterator<T> reverse( Iterator<T> iterator )
+ {
+ List<T> rev = new ArrayList<>();
+ while ( iterator.hasNext() )
+ {
+ rev.add( 0, iterator.next() );
+ }
+ return rev.iterator();
+ }
+}
