From 93ee5308b9a94a9f27738de0762be95376f3f716 Mon Sep 17 00:00:00 2001
From: Jean-Noël Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Tue, 09 Feb 2016 14:00:40 +0000
Subject: [PATCH] OPENDJSDK-85 AVA does not provide a way to get back the user provided attribute type

---
 opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/AVA.java                                       |  122 +++++++++++++++++-------------
 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/CollationSubstringMatchingRuleTest.java |    6 
 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/AVATestCase.java                               |   64 ++++++++++++++++
 opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/ByteString.java                                |    5 
 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/ByteStringTestCase.java                        |    9 ++
 5 files changed, 145 insertions(+), 61 deletions(-)

diff --git a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/AVA.java b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/AVA.java
index 876714b..3c1cbea 100644
--- a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/AVA.java
+++ b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/AVA.java
@@ -22,13 +22,14 @@
  *
  *
  *      Copyright 2010 Sun Microsystems, Inc.
- *      Portions copyright 2011-2015 ForgeRock AS.
+ *      Portions copyright 2011-2016 ForgeRock AS.
  */
-
 package org.forgerock.opendj.ldap;
 
-import static com.forgerock.opendj.util.StaticUtils.*;
 import static com.forgerock.opendj.ldap.CoreMessages.*;
+import static com.forgerock.opendj.util.StaticUtils.*;
+
+import static org.forgerock.util.Reject.*;
 
 import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
@@ -42,9 +43,7 @@
 import org.forgerock.opendj.ldap.schema.AttributeType;
 import org.forgerock.opendj.ldap.schema.MatchingRule;
 import org.forgerock.opendj.ldap.schema.Schema;
-import org.forgerock.opendj.ldap.schema.Syntax;
 import org.forgerock.opendj.ldap.schema.UnknownSchemaElementException;
-import org.forgerock.util.Reject;
 
 import com.forgerock.opendj.util.StaticUtils;
 import com.forgerock.opendj.util.SubstringReader;
@@ -62,6 +61,10 @@
  * cn=Kurt Zeilenga
  * </pre>
  *
+ * Note: The name <em>AVA</em> is historical, coming from X500/LDAPv2.
+ * However, in LDAP context, this class actually represents an
+ * <code>AttributeTypeAndValue</code>.
+ *
  * @see <a href="http://tools.ietf.org/html/rfc4512#section-2.3">RFC 4512 -
  *      Lightweight Directory Access Protocol (LDAP): Directory Information
  *      Models </a>
@@ -121,7 +124,8 @@
             throw new LocalizedIllegalArgumentException(message);
         }
 
-        final AttributeType attribute = readAttributeName(reader, schema);
+        final String nameOrOid = readAttributeName(reader);
+        final AttributeType attribute = schema.getAttributeType(nameOrOid);
 
         // Skip over any spaces if we have.
         reader.skipWhitespaces();
@@ -151,7 +155,7 @@
         // Parse the value for this RDN component.
         final ByteString value = readAttributeValue(reader);
 
-        return new AVA(attribute, value);
+        return new AVA(attribute, nameOrOid, value);
     }
 
     static void escapeAttributeValue(final String str, final StringBuilder builder) {
@@ -396,7 +400,7 @@
         return ByteString.valueOfUtf8(valueBuffer);
     }
 
-    private static AttributeType readAttributeName(final SubstringReader reader, final Schema schema) {
+    private static String readAttributeName(final SubstringReader reader) {
         int length = 1;
         reader.mark();
 
@@ -413,19 +417,11 @@
                     break;
                 } else if (c == '.') {
                     if (lastWasPeriod) {
-                        final LocalizableMessage message =
-                                ERR_ATTR_SYNTAX_DN_ATTR_ILLEGAL_CHAR.get(reader.getString(), c,
-                                        reader.pos() - 1);
-                        throw new LocalizedIllegalArgumentException(message);
-                    } else {
-                        lastWasPeriod = true;
+                        throw illegalCharacter(reader, c);
                     }
+                    lastWasPeriod = true;
                 } else if (!isDigit(c)) {
-                    // This must have been an illegal character.
-                    final LocalizableMessage message =
-                            ERR_ATTR_SYNTAX_DN_ATTR_ILLEGAL_CHAR.get(reader.getString(), c, reader
-                                    .pos() - 1);
-                    throw new LocalizedIllegalArgumentException(message);
+                    throw illegalCharacter(reader, c);
                 } else {
                     lastWasPeriod = false;
                 }
@@ -442,28 +438,25 @@
                     // This signals the end of the OID.
                     break;
                 } else if (!isAlpha(c) && !isDigit(c) && c != '-') {
-                    // This is an illegal character.
-                    final LocalizableMessage message =
-                            ERR_ATTR_SYNTAX_DN_ATTR_ILLEGAL_CHAR.get(reader.getString(), c, reader
-                                    .pos() - 1);
-                    throw new LocalizedIllegalArgumentException(message);
+                    throw illegalCharacter(reader, c);
                 }
 
                 length++;
             }
         } else {
-            final LocalizableMessage message =
-                    ERR_ATTR_SYNTAX_DN_ATTR_ILLEGAL_CHAR.get(reader.getString(), c,
-                            reader.pos() - 1);
-            throw new LocalizedIllegalArgumentException(message);
+            throw illegalCharacter(reader, c);
         }
 
         reader.reset();
 
-        // Return the position of the first non-space character after the
-        // token.
+        // Return the position of the first non-space character after the token
+        return reader.read(length);
+    }
 
-        return schema.getAttributeType(reader.read(length));
+    private static LocalizedIllegalArgumentException illegalCharacter(
+            final SubstringReader reader, final char c) {
+        return new LocalizedIllegalArgumentException(
+                ERR_ATTR_SYNTAX_DN_ATTR_ILLEGAL_CHAR.get(reader.getString(), c, reader.pos() - 1));
     }
 
     private static ByteString readAttributeValue(final SubstringReader reader) {
@@ -591,12 +584,11 @@
     }
 
     private final AttributeType attributeType;
-
+    private final String attributeName;
     private final ByteString attributeValue;
 
     /** Cached normalized value using equality matching rule. */
     private ByteString equalityNormalizedAttributeValue;
-
     /** Cached normalized value using ordering matching rule. */
     private ByteString orderingNormalizedAttributeValue;
 
@@ -616,10 +608,29 @@
      *             {@code null}.
      */
     public AVA(final AttributeType attributeType, final Object attributeValue) {
-        Reject.ifNull(attributeType, attributeValue);
+        this(attributeType, null, attributeValue);
+    }
 
-        this.attributeType = attributeType;
-        this.attributeValue = ByteString.valueOfObject(attributeValue);
+    /**
+     * Creates a new attribute value assertion (AVA) using the provided
+     * attribute type, name and value.
+     * <p>
+     * If {@code attributeValue} is not an instance of {@code ByteString} then
+     * it will be converted using the {@link ByteString#valueOfObject(Object)} method.
+     *
+     * @param attributeType
+     *            The attribute type.
+     * @param attributeName
+     *            The user provided attribute name.
+     * @param attributeValue
+     *            The attribute value.
+     * @throws NullPointerException
+     *             If {@code attributeType}, {@code attributeName} or {@code attributeValue} was {@code null}.
+     */
+    public AVA(final AttributeType attributeType, final String attributeName, final Object attributeValue) {
+        this.attributeType = checkNotNull(attributeType);
+        this.attributeName = computeAttributeName(attributeName, attributeType);
+        this.attributeValue = ByteString.valueOfObject(checkNotNull(attributeValue));
     }
 
     /**
@@ -640,13 +651,15 @@
      *             {@code null}.
      */
     public AVA(final String attributeType, final Object attributeValue) {
-        Reject.ifNull(attributeType, attributeValue);
-
+        this.attributeName = checkNotNull(attributeType);
         this.attributeType = Schema.getDefaultSchema().getAttributeType(attributeType);
-        this.attributeValue = ByteString.valueOfObject(attributeValue);
+        this.attributeValue = ByteString.valueOfObject(checkNotNull(attributeValue));
     }
 
-    /** {@inheritDoc} */
+    private String computeAttributeName(final String attributeName, final AttributeType attributeType) {
+        return attributeName != null ? attributeName : attributeType.getNameOrOID();
+    }
+
     @Override
     public int compareTo(final AVA ava) {
         final int result = attributeType.compareTo(ava.attributeType);
@@ -659,7 +672,6 @@
         return normalizedValue.compareTo(otherNormalizedValue);
     }
 
-    /** {@inheritDoc} */
     @Override
     public boolean equals(final Object obj) {
         if (this == obj) {
@@ -689,6 +701,15 @@
     }
 
     /**
+     * Returns the attribute name associated with this AVA.
+     *
+     * @return The attribute name associated with this AVA.
+     */
+    public String getAttributeName() {
+        return attributeName;
+    }
+
+    /**
      * Returns the attribute value associated with this AVA.
      *
      * @return The attribute value associated with this AVA.
@@ -697,7 +718,6 @@
         return attributeValue;
     }
 
-    /** {@inheritDoc} */
     @Override
     public int hashCode() {
         return attributeType.hashCode() * 31 + getEqualityNormalizedValue().hashCode();
@@ -715,7 +735,6 @@
         return new LinkedAttribute(ad, attributeValue);
     }
 
-    /** {@inheritDoc} */
     @Override
     public String toString() {
         final StringBuilder builder = new StringBuilder();
@@ -723,17 +742,15 @@
     }
 
     StringBuilder toString(final StringBuilder builder) {
-        if (!attributeType.getNames().iterator().hasNext()) {
+        if (attributeName.equals(attributeType.getOID())) {
             builder.append(attributeType.getOID());
             builder.append("=#");
             builder.append(attributeValue.toHexString());
         } else {
-            final String name = attributeType.getNameOrOID();
-            builder.append(name);
+            builder.append(attributeName);
             builder.append("=");
 
-            final Syntax syntax = attributeType.getSyntax();
-            if (!syntax.isHumanReadable()) {
+            if (!attributeType.getSyntax().isHumanReadable()) {
                 builder.append("#");
                 builder.append(attributeValue.toHexString());
             } else {
@@ -879,15 +896,12 @@
     }
 
     private boolean needEscaping(final ByteString value) {
-        boolean needEscaping = false;
         for (int i = 0; i < value.length(); i++) {
-            final byte b = value.byteAt(i);
-            if (isByteToEscape(b)) {
-                needEscaping = true;
-                break;
+            if (isByteToEscape(value.byteAt(i))) {
+                return true;
             }
         }
-        return needEscaping;
+        return false;
     }
 
     private boolean isByteToEscape(final byte b) {
diff --git a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/ByteString.java b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/ByteString.java
old mode 100755
new mode 100644
index 4f6a696..511bf65
--- a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/ByteString.java
+++ b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/ByteString.java
@@ -22,7 +22,7 @@
  *
  *
  *      Copyright 2009-2010 Sun Microsystems, Inc.
- *      Portions copyright 2011-2015 ForgeRock AS
+ *      Portions copyright 2011-2016 ForgeRock AS
  */
 package org.forgerock.opendj.ldap;
 
@@ -802,10 +802,9 @@
         if (isEmpty()) {
             return "";
         }
-        StringBuilder builder = new StringBuilder((length - 1) * 3 + 2);
+        StringBuilder builder = new StringBuilder(length * 2);
         builder.append(StaticUtils.byteToHex(buffer[offset]));
         for (int i = 1; i < length; i++) {
-            builder.append(' ');
             builder.append(StaticUtils.byteToHex(buffer[offset + i]));
         }
         return builder.toString();
diff --git a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/AVATestCase.java b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/AVATestCase.java
new file mode 100644
index 0000000..f958a97
--- /dev/null
+++ b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/AVATestCase.java
@@ -0,0 +1,64 @@
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License, Version 1.0 only
+ * (the "License").  You may not use this file except in compliance
+ * with the License.
+ *
+ * You can obtain a copy of the license at legal-notices/CDDLv1_0.txt
+ * or http://forgerock.org/license/CDDLv1.0.html.
+ * See the License for the specific language governing permissions
+ * and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at legal-notices/CDDLv1_0.txt.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information:
+ *      Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ *
+ *      Portions copyright 2016 ForgeRock AS.
+ */
+package org.forgerock.opendj.ldap;
+
+import static org.assertj.core.api.Assertions.*;
+
+import org.forgerock.opendj.ldap.schema.AttributeType;
+import org.forgerock.opendj.ldap.schema.Schema;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+/** This class defines a set of tests for the {@link AVA} class. */
+@SuppressWarnings("javadoc")
+public class AVATestCase extends SdkTestCase {
+
+    @DataProvider
+    private Object[][] valueOfDataProvider() {
+        AttributeType cnAttrType = Schema.getCoreSchema().getAttributeType("commonName");
+        return new Object[][] {
+            { "CN=value", cnAttrType, "CN", "value" },
+            { "commonname=value", cnAttrType, "commonname", "value" },
+            { "2.5.4.3=#76616C7565", cnAttrType, "2.5.4.3", "value" },
+        };
+    }
+
+    @Test(dataProvider = "valueOfDataProvider")
+    public void valueOf(String avaString, AttributeType expectedAttrType, String expectedAttrName,
+            String expectedValue) throws Exception {
+        AVA ava = AVA.valueOf(avaString);
+        assertThat(ava.getAttributeType()).isEqualTo(expectedAttrType);
+        assertThat(ava.getAttributeName()).isEqualTo(expectedAttrName);
+        assertThat(ava.getAttributeValue()).isEqualTo(ByteString.valueOfUtf8(expectedValue));
+        assertThat(ava.toString()).isEqualTo(avaString);
+    }
+
+    @Test
+    public void hexEncodingDoesNotLoseInformation() throws Exception {
+        final String avaString = "2.5.4.3=#76616C7565";
+        final String roundtrippedValue = AVA.valueOf(avaString).toString();
+        assertThat(AVA.valueOf(roundtrippedValue).toString()).isEqualTo(avaString);
+    }
+}
diff --git a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/ByteStringTestCase.java b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/ByteStringTestCase.java
index 21f08ae..06c39c9 100644
--- a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/ByteStringTestCase.java
+++ b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/ByteStringTestCase.java
@@ -266,7 +266,7 @@
     @Test
     public void testToHex() throws Exception {
         ByteString byteString = new ByteStringBuilder().appendUtf8("org=example").toByteString();
-        assertThat(byteString.toHexString()).isEqualTo("6F 72 67 3D 65 78 61 6D 70 6C 65");
+        assertThat(byteString.toHexString()).isEqualTo("6F72673D6578616D706C65");
 
         assertThat(ByteString.empty().toHexString()).isEqualTo("");
     }
@@ -343,6 +343,13 @@
     }
 
     @Test
+    public void testToHexValueOfHex() {
+        ByteString bs = ByteString.valueOfUtf8("cn=testvalue");
+        ByteString roundtrippedBS = ByteString.valueOfHex(bs.toHexString());
+        assertThat(roundtrippedBS).isEqualTo(bs);
+    }
+
+    @Test
     public void testValueOfEmptyHex() {
         ByteString byteString = ByteString.valueOfHex("");
         assertThat(byteString.toString()).isEqualTo("");
diff --git a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/CollationSubstringMatchingRuleTest.java b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/CollationSubstringMatchingRuleTest.java
index 2851ae6..2386818 100644
--- a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/CollationSubstringMatchingRuleTest.java
+++ b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/CollationSubstringMatchingRuleTest.java
@@ -161,9 +161,9 @@
         ByteString bfinal = matchingRule.normalizeAttributeValue(ByteString.valueOfUtf8("c"));
         assertEquals(indexQuery,
             "intersect["
-            + "rangeMatch(fr.shared, '" + binit.toHexString() + "' <= value < '00 54'), "
-            + "rangeMatch(fr.substring, '" + bfinal.toHexString() + "' <= value < '00 56'), "
-            + "rangeMatch(fr.substring, '" + binit.toHexString() + "' <= value < '00 54')]"
+            + "rangeMatch(fr.shared, '" + binit.toHexString() + "' <= value < '0054'), "
+            + "rangeMatch(fr.substring, '" + bfinal.toHexString() + "' <= value < '0056'), "
+            + "rangeMatch(fr.substring, '" + binit.toHexString() + "' <= value < '0054')]"
         );
     }
 }

--
Gitblit v1.10.0