You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by se...@apache.org on 2021/04/03 22:18:53 UTC
[directory-ldap-api] branch DIRAPI-368-fix-stackoverflow created
(now c5acb33)
This is an automated email from the ASF dual-hosted git repository.
seelmann pushed a change to branch DIRAPI-368-fix-stackoverflow
in repository https://gitbox.apache.org/repos/asf/directory-ldap-api.git.
at c5acb33 DIRAPI-368, DIRSERVER-2340: Fix StackOverflowError
This branch includes the following new commits:
new c5acb33 DIRAPI-368, DIRSERVER-2340: Fix StackOverflowError
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[directory-ldap-api] 01/01: DIRAPI-368,
DIRSERVER-2340: Fix StackOverflowError
Posted by se...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
seelmann pushed a commit to branch DIRAPI-368-fix-stackoverflow
in repository https://gitbox.apache.org/repos/asf/directory-ldap-api.git
commit c5acb33ea53806d5df6ed90679f63cb46b026e82
Author: Stefan Seelmann <ma...@stefan-seelmann.de>
AuthorDate: Sun Apr 4 00:16:16 2021 +0200
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.
---
.../api/ldap/codec/factory/AddRequestFactory.java | 17 ++++-----
.../ldap/codec/factory/ModifyRequestFactory.java | 17 ++++-----
.../codec/factory/SearchResultEntryFactory.java | 20 +++++-----
.../api/ldap/codec/add/AddRequestTest.java | 39 +++++++++++++++++++
.../api/ldap/codec/modify/ModifyRequestTest.java | 39 +++++++++++++++++++
.../ldap/codec/search/SearchResultEntryTest.java | 40 +++++++++++++++++++-
.../apache/directory/api/util/CollectionUtils.java | 44 ++++++++++++++++++++++
7 files changed, 184 insertions(+), 32 deletions(-)
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.Entry;
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 @@ public final class AddRequestFactory implements Messagefactory
/**
- * 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 @@ public final class AddRequestFactory implements Messagefactory
*/
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 @@ public final class AddRequestFactory implements Messagefactory
/**
* 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 @@ public final class AddRequestFactory implements Messagefactory
*/
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.ModificationOperation;
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 @@ public final class ModifyRequestFactory implements Messagefactory
/**
- * Encode the values, recursively
+ * Encode the values, in reverse order
* <pre>
* 0x04 LL attributeValue
* ...
@@ -62,13 +63,11 @@ public final class ModifyRequestFactory implements Messagefactory
*/
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 @@ public final class ModifyRequestFactory implements Messagefactory
}
/**
- * 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 @@ public final class ModifyRequestFactory implements Messagefactory
*/
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.Entry;
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 @@ public final class SearchResultEntryFactory extends ResponseFactory
/**
- * Encode the values recursively
+ * Encode the values in reverse order.
*
* <pre>
* 0x04 LL attributeValue
@@ -61,12 +62,11 @@ public final class SearchResultEntryFactory extends ResponseFactory
*/
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 @@ public final class SearchResultEntryFactory extends ResponseFactory
/**
- * Encode the attributes recursively
+ * Encode the attributes in reverse order.
*
* <pre>
* 0x30 LL partialAttributeList
@@ -97,16 +97,14 @@ public final class SearchResultEntryFactory extends ResponseFactory
*/
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 final class SearchResultEntryFactory extends ResponseFactory
// 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.HashMap;
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.LdapMessageContainer;
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.Message;
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 class AddRequestTest extends AbstractCodecServiceTest
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 static org.junit.jupiter.api.Assertions.fail;
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.LdapMessageContainer;
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 class ModifyRequestTest extends AbstractCodecServiceTest
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 static org.junit.jupiter.api.Assertions.assertTrue;
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.LdapEncoder;
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 class SearchResultEntryTest extends AbstractCodecServiceTest
assertTrue( Arrays.equals( stream.array(), buffer.getBytes().array() ) );
}
-
/**
* Test the decoding of a SearchResultEntry
*/
@@ -971,4 +974,39 @@ public class SearchResultEntryTest extends AbstractCodecServiceTest
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();
+ }
+}