From 50fa9f2b2ccd9f0a42a794e519943d2f167e0706 Mon Sep 17 00:00:00 2001
From: Fabio Pistolesi <fabio.pistolesi@forgerock.com>
Date: Tue, 10 May 2016 16:47:38 +0000
Subject: [PATCH] OPENDJ-2946 Fix some problems found by stress testing DN handling
---
opendj-core/src/test/java/org/forgerock/opendj/ldap/DNTestCase.java | 15 +++++
opendj-server-legacy/src/main/java/org/opends/server/protocols/ldap/LDAPFilter.java | 10 +++
opendj-core/src/main/java/org/forgerock/opendj/ldap/RDN.java | 4 -
opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java | 15 ++--
opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java | 62 +++++++++++++++-----
opendj-server-legacy/src/main/java/org/opends/server/loggers/TextErrorLogPublisher.java | 19 +++--
6 files changed, 88 insertions(+), 37 deletions(-)
diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java
index d868705..cfe47d8 100644
--- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java
+++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java
@@ -19,6 +19,7 @@
import java.util.Arrays;
import java.util.Iterator;
import java.util.LinkedHashMap;
+import java.util.LinkedList;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.UUID;
@@ -28,6 +29,7 @@
import org.forgerock.opendj.ldap.schema.CoreSchema;
import org.forgerock.opendj.ldap.schema.Schema;
import org.forgerock.opendj.ldap.schema.UnknownSchemaElementException;
+import org.forgerock.util.Pair;
import org.forgerock.util.Reject;
import com.forgerock.opendj.util.SubstringReader;
@@ -270,25 +272,41 @@
ERR_DN_TYPE_NOT_FOUND.get(reader.getString(), e.getMessageObject()));
}
- if (reader.remaining() > 0 && reader.read() == ',') {
+ LinkedList<Pair<Integer, RDN>> parentRDNs = null;
+ DN parent = null;
+ while (reader.remaining() > 0 && reader.read() == ',') {
reader.skipWhitespaces();
if (reader.remaining() == 0) {
throw new LocalizedIllegalArgumentException(ERR_ATTR_SYNTAX_DN_ATTR_NO_NAME.get(reader.getString()));
}
reader.mark();
final String parentString = reader.read(reader.remaining());
- DN parent = cache.get(parentString);
- if (parent == null) {
- reader.reset();
- parent = decode(reader, schema, cache);
-
- // Only cache parent DNs since leaf DNs are likely to make the cache to volatile.
- cache.put(parentString, parent);
+ parent = cache.get(parentString);
+ if (parent != null) {
+ break;
}
- return new DN(schema, parent, rdn);
- } else {
- return new DN(schema, ROOT_DN, rdn);
+ reader.reset();
+ if (parentRDNs == null) {
+ parentRDNs = new LinkedList<>();
+ }
+ parentRDNs.add(Pair.of(reader.pos(), RDN.decode(reader, schema)));
}
+ if (parent == null) {
+ parent = ROOT_DN;
+ }
+
+ if (parentRDNs != null) {
+ Iterator<Pair<Integer, RDN>> iter = parentRDNs.descendingIterator();
+ int parentsLeft = parentRDNs.size();
+ while (iter.hasNext()) {
+ Pair<Integer, RDN> parentRDN = iter.next();
+ parent = new DN(schema, parent, parentRDN.getSecond());
+ if (parentsLeft-- < DN_CACHE_SIZE) {
+ cache.put(reader.getString().substring(parentRDN.getFirst()), parent);
+ }
+ }
+ }
+ return new DN(schema, parent, rdn);
}
@SuppressWarnings("serial")
@@ -877,10 +895,15 @@
public String toString() {
// We don't care about potential race conditions here.
if (stringValue == null) {
- final StringBuilder builder = rdn.toString(new StringBuilder());
- if (!parent.isRootDN()) {
+ final StringBuilder builder = new StringBuilder();
+ builder.append(rdn);
+ for (DN dn = parent; dn.rdn != null; dn = dn.parent) {
builder.append(RDN_CHAR_SEPARATOR);
- builder.append(parent);
+ if (dn.stringValue != null) {
+ builder.append(dn.stringValue);
+ break;
+ }
+ builder.append(dn.rdn);
}
stringValue = builder.toString();
}
@@ -902,9 +925,14 @@
if (rdn == null) {
normalizedDN = ByteString.empty();
} else {
- final ByteString normalizedParent = parent.toNormalizedByteString();
- final ByteStringBuilder builder = new ByteStringBuilder(normalizedParent.length() + 16);
- builder.appendBytes(normalizedParent);
+ final ByteStringBuilder builder = new ByteStringBuilder(size * 8);
+ if (parent.normalizedDN == null) {
+ for (int i = size() - 1; i > 0; i--) {
+ parent(i).rdn().toNormalizedByteString(builder);
+ }
+ } else {
+ builder.appendBytes(parent.normalizedDN);
+ }
rdn.toNormalizedByteString(builder);
normalizedDN = builder.toByteString();
}
diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/RDN.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/RDN.java
index 5430c05..af69c2e 100644
--- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/RDN.java
+++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/RDN.java
@@ -493,10 +493,6 @@
return stringValue;
}
- StringBuilder toString(final StringBuilder builder) {
- return builder.append(this);
- }
-
/**
* Returns the normalized byte string representation of this RDN.
* <p>
diff --git a/opendj-core/src/test/java/org/forgerock/opendj/ldap/DNTestCase.java b/opendj-core/src/test/java/org/forgerock/opendj/ldap/DNTestCase.java
index 6c03e6f..d5a8489 100644
--- a/opendj-core/src/test/java/org/forgerock/opendj/ldap/DNTestCase.java
+++ b/opendj-core/src/test/java/org/forgerock/opendj/ldap/DNTestCase.java
@@ -1393,4 +1393,19 @@
public void rdnShouldThrowIAEForNegativeIndexes() throws Exception {
DN.valueOf("dc=example,dc=com").rdn(-1);
}
+
+ @Test
+ public void testToStringOnExtremelyLongDNs() {
+ int numRDNs = 16384;
+ String rdn = "cn=verylongdn,";
+ String base = "dc=example,dc=com";
+ StringBuilder builder = new StringBuilder(rdn.length() * numRDNs + base.length());
+ for (int i = 0; i < numRDNs; i++) {
+ builder.append(rdn);
+ }
+ builder.append(base);
+ DN longDN = DN.valueOf(builder.toString());
+ assertEquals(longDN.toString(), builder.toString(),
+ "String representation of a very long DN does not match the source DN");
+ }
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/loggers/TextErrorLogPublisher.java b/opendj-server-legacy/src/main/java/org/opends/server/loggers/TextErrorLogPublisher.java
index 2eea96a..b18fa8c 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/loggers/TextErrorLogPublisher.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/loggers/TextErrorLogPublisher.java
@@ -455,16 +455,19 @@
StringBuilder sb = new StringBuilder();
sb.append("[");
sb.append(TimeThread.getLocalTime());
- sb.append("] category=").append(category).
- append(" severity=").append(severity).
- append(" msgID=").append(message.resourceName())
- .append('.')
- .append(message.ordinal()).
- append(" msg=").append(message);
+ sb.append("] category=")
+ .append(category)
+ .append(" severity=")
+ .append(severity)
+ .append(" msgID=")
+ .append(message.resourceName())
+ .append('.')
+ .append(message.ordinal())
+ .append(" msg=")
+ .append(message.toString());
if (exception != null)
{
- sb.append(" exception=").append(
- StaticUtils.stackTraceToSingleLineString(exception));
+ sb.append(" exception=").append(StaticUtils.stackTraceToSingleLineString(exception));
}
writer.writeRecord(sb.toString());
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/protocols/ldap/LDAPFilter.java b/opendj-server-legacy/src/main/java/org/opends/server/protocols/ldap/LDAPFilter.java
index aef820c..2e74ce3 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/protocols/ldap/LDAPFilter.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/protocols/ldap/LDAPFilter.java
@@ -25,6 +25,7 @@
import java.util.Set;
import org.forgerock.i18n.LocalizableMessage;
+import org.forgerock.i18n.LocalizedIllegalArgumentException;
import org.forgerock.i18n.slf4j.LocalizedLogger;
import org.forgerock.opendj.ldap.AttributeDescription;
import org.forgerock.opendj.ldap.ByteString;
@@ -1891,7 +1892,14 @@
Set<String> options = Collections.emptySet();
if (attributeDescription != null)
{
- attrDesc = AttributeDescription.valueOf(attributeDescription);
+ try
+ {
+ attrDesc = AttributeDescription.valueOf(attributeDescription);
+ }
+ catch (LocalizedIllegalArgumentException e)
+ {
+ throw new DirectoryException(ResultCode.PROTOCOL_ERROR, e.getMessageObject(), e);
+ }
attributeType = attrDesc.getAttributeType();
options = toSet(attrDesc);
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java
index 0ca7cc7..83b9cd1 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java
@@ -20,6 +20,7 @@
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
+import java.util.Map;
import java.util.TreeMap;
import org.forgerock.i18n.LocalizableMessage;
@@ -898,14 +899,14 @@
private static LocalBackendWorkflowElement getLocalBackendWorkflowElement(DN entryDN)
{
- while (entryDN != null)
+ if (entryDN.isRootDN())
{
- final LocalBackendWorkflowElement workflow = registeredLocalBackends.get(entryDN);
- if (workflow != null)
- {
- return workflow;
- }
- entryDN = DirectoryServer.getParentDNInSuffix(entryDN);
+ return registeredLocalBackends.get(entryDN);
+ }
+ Map.Entry<DN, LocalBackendWorkflowElement> backendWorkflow = registeredLocalBackends.floorEntry(entryDN);
+ if (backendWorkflow.getKey().isSuperiorOrEqualTo(entryDN))
+ {
+ return backendWorkflow.getValue();
}
return null;
}
--
Gitblit v1.10.0