Merge pull request #16 from apache/DIRAPI-368-fix-stackoverflow

DIRAPI-368, DIRSERVER-2340: Fix StackOverflowError
apache · Apr 4, 2021 · 275b983 · 275b983
2 parents 607e52b + 286524c
commit 275b983
Show file tree

Hide file tree

Showing 8 changed files with 247 additions and 32 deletions.
diff --git a/...dec/core/src/main/java/org/apache/directory/api/ldap/codec/factory/AddRequestFactory.java b/...dec/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 @@ private AddRequestFactory()
 
 
     /**
-     * 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 AddRequestFactory()
      */
     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 @@ private void encodeValueReverse( Asn1Buffer buffer, Iterator<Value> iterator )
 
     /**
      * 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 encodeValueReverse( Asn1Buffer buffer, Iterator<Value> iterator )
      */
     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/.../core/src/main/java/org/apache/directory/api/ldap/codec/factory/ModifyRequestFactory.java b/.../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 @@ private ModifyRequestFactory()
 
 
     /**
-     * Encode the values, recursively
+     * Encode the values, in reverse order
      * <pre>
      * 0x04 LL attributeValue
      * ...
@@ -62,13 +63,11 @@ private ModifyRequestFactory()
      */
     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 @@ private void encodeValues( Asn1Buffer buffer, Iterator<Value> values )
     }
 
     /**
-     * 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 encodeValues( Asn1Buffer buffer, Iterator<Value> values )
      */
     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/...e/src/main/java/org/apache/directory/api/ldap/codec/factory/SearchResultEntryFactory.java b/...e/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 @@ private SearchResultEntryFactory()
 
 
     /**
-     * Encode the values recursively
+     * Encode the values in reverse order.
      *
      * <pre>
      * 0x04 LL attributeValue
@@ -61,12 +62,11 @@ private SearchResultEntryFactory()
      */
     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 @@ private void encodeValues( Asn1Buffer buffer, Iterator<Value> values )
 
 
     /**
-     * Encode the attributes recursively
+     * Encode the attributes in reverse order.
      *
      * <pre>
      *  0x30 LL partialAttributeList
@@ -97,16 +97,14 @@ private void encodeValues( Asn1Buffer buffer, Iterator<Value> values )
      */
     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 @@ public void encodeReverse( LdapApiService codec, Asn1Buffer buffer, Message mess
         // 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
@@ -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 @@ public void testDecodeAddRequestEmptyAttributeValueWithControl() throws  Decoder
 
         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 < 100000; attributeIndex++ )
+        {
+            entry.add( "objectclass" + attributeIndex, "top", "person" );
+        }
+        String[] values = IntStream.range( 0, 100000 ).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/...odec/core/src/test/java/org/apache/directory/api/ldap/codec/modify/ModifyRequestTest.java b/...odec/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 @@ public void testDecodeModifyRequestAddOperationModificationIncrementWithValue()
 
         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 < 100000; modIndex++ )
+        {
+            originalModifyRequest.addModification( new DefaultModification( ModificationOperation.REPLACE_ATTRIBUTE,
+                "objectclass" + modIndex, "top", "person" ) );
+        }
+        String[] values = IntStream.range( 0, 100000 ).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/.../core/src/test/java/org/apache/directory/api/ldap/codec/search/SearchResultEntryTest.java b/.../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 @@ public void testDecodeSearchResultEntrySuccess() throws DecoderException, Encode
         assertTrue( Arrays.equals( stream.array(), buffer.getBytes().array() ) );
     }
 
-
     /**
      * Test the decoding of a SearchResultEntry
      */
@@ -971,4 +974,39 @@ public void testDecodeSearchResultEntryEmptyAttributeValueWithControls() throws
 
         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 < 100000; attributeIndex++ )
+        {
+            entry.add( "objectclass" + attributeIndex, "top", "person" );
+        }
+        String[] values = IntStream.range( 0, 100000 ).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 );
+    }
+
 }