From ac272266185cd7aefee60209b4fcfd47733d4d15 Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Wed, 16 Mar 2016 00:46:49 +0000
Subject: [PATCH] OPENDJ-2776 Remove insignificant/illegal whitespace during parsing of DNs

---
 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFChangeRecordReaderTestCase.java |   32 +++++----------
 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFEntryReaderTestCase.java        |   22 ++++-------
 opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java                             |   35 +++++++----------
 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFChangeRecordWriterTestCase.java |    5 +-
 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/DNTestCase.java                     |   20 ++++++++++
 5 files changed, 56 insertions(+), 58 deletions(-)

diff --git a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java
index 602c971..ad247e0 100644
--- a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java
+++ b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java
@@ -60,7 +60,7 @@
     static final char RDN_CHAR_SEPARATOR = ',';
     static final char AVA_CHAR_SEPARATOR = '+';
 
-    private static final DN ROOT_DN = new DN(CoreSchema.getInstance(), null, null, "");
+    private static final DN ROOT_DN = new DN(CoreSchema.getInstance(), null, null);
 
     /**
      * This is the size of the per-thread per-schema DN cache. We should
@@ -238,8 +238,7 @@
         }
 
         // Not in cache so decode.
-        final SubstringReader reader = new SubstringReader(dn);
-        return decode(dn, reader, schema, cache);
+        return decode(new SubstringReader(dn), schema, cache);
     }
 
     /**
@@ -258,8 +257,7 @@
     }
 
     /** Decodes a DN using the provided reader and schema. */
-    private static DN decode(final String dnString, final SubstringReader reader,
-            final Schema schema, final Map<String, DN> cache) {
+    private static DN decode(final SubstringReader reader, final Schema schema, final Map<String, DN> cache) {
         reader.skipWhitespaces();
         if (reader.remaining() == 0) {
             return ROOT_DN;
@@ -282,7 +280,7 @@
             parent = cache.get(parentString);
             if (parent == null) {
                 reader.reset();
-                parent = decode(parentString, reader, schema, cache);
+                parent = decode(reader, schema, cache);
 
                 // Only cache parent DNs since leaf DNs are likely to make the
                 // cache to volatile.
@@ -292,7 +290,7 @@
             parent = ROOT_DN;
         }
 
-        return new DN(schema, parent, rdn, dnString);
+        return new DN(schema, parent, rdn);
     }
 
     @SuppressWarnings("serial")
@@ -322,27 +320,24 @@
      */
     private ByteString normalizedDN;
 
-    /**
-     * We need to store the original string value if provided in order to
-     * preserve the original whitespace.
-     */
+    /** The RFC 4514 string representation of this DN. */
     private String stringValue;
 
     /** The schema used to create this DN. */
     private final Schema schema;
 
     /** Private constructor. */
-    private DN(final Schema schema, final DN parent, final RDN rdn, final String stringValue) {
-        this(schema, parent, rdn, stringValue, parent != null ? parent.size + 1 : 0);
+    private DN(final Schema schema, final DN parent, final RDN rdn) {
+        this(schema, parent, rdn, parent != null ? parent.size + 1 : 0);
     }
 
     /** Private constructor. */
-    private DN(final Schema schema, final DN parent, final RDN rdn, final String stringValue, final int size) {
+    private DN(final Schema schema, final DN parent, final RDN rdn, final int size) {
         this.schema = schema;
         this.parent = parent;
         this.rdn = rdn;
-        this.stringValue = stringValue;
         this.size = size;
+        this.stringValue = rdn == null ? "" : null; // Compute lazily.
     }
 
     /**
@@ -370,7 +365,7 @@
             }
             DN newDN = this;
             for (i = 0; i < rdns.length; i++) {
-                newDN = new DN(this.schema, newDN, rdns[i], null);
+                newDN = new DN(this.schema, newDN, rdns[i]);
             }
             return newDN;
         }
@@ -394,7 +389,7 @@
      */
     public DN child(final RDN rdn) {
         Reject.ifNull(rdn);
-        return new DN(this.schema, this, rdn, null);
+        return new DN(this.schema, this, rdn);
     }
 
     /**
@@ -741,11 +736,11 @@
         } else if (index >= size) {
             return this;
         } else {
-            final DN localName = new DN(this.schema, null, rdn, null, index);
+            final DN localName = new DN(this.schema, null, rdn, index);
             DN nextLocalName = localName;
             DN lastDN = parent;
             for (int i = index - 1; i > 0; i--) {
-                nextLocalName.parent = new DN(this.schema, null, lastDN.rdn, null, i);
+                nextLocalName.parent = new DN(this.schema, null, lastDN.rdn, i);
                 nextLocalName = nextLocalName.parent;
                 lastDN = lastDN.parent;
             }
@@ -989,7 +984,7 @@
         private final Schema schema;
 
         private CompactDn(final DN dn) {
-            this.originalValue = dn.stringValue != null ? getBytes(dn.stringValue) : new byte[0];
+            this.originalValue = getBytes(dn.toString());
             this.schema = dn.schema;
         }
 
diff --git a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/DNTestCase.java b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/DNTestCase.java
index c497245..8166697 100644
--- a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/DNTestCase.java
+++ b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/DNTestCase.java
@@ -1246,4 +1246,24 @@
         UUID uuid2 = DN.valueOf("cn=test+dc=example,dc=com").toUUID();
         assertEquals(uuid1, uuid2);
     }
+
+    @DataProvider
+    public Object[][] toStringShouldStripOutIllegalWhitespaceDataProvider() {
+        // @formatter:off
+        return new Object[][] {
+            { " ", "" },
+            { " dc = hello  world ", "dc=hello  world" },
+            { " dc =\\  hello  world\\  ", "dc=\\  hello  world\\ " },
+            { " dc = example , dc = com ", "dc=example,dc=com" },
+            { " uid = crystal + dc = example , uid = palace + dc = com ", "uid=crystal+dc=example,uid=palace+dc=com" },
+        };
+        // @formatter:on
+    }
+
+    @Test(dataProvider = "toStringShouldStripOutIllegalWhitespaceDataProvider")
+    public void toStringShouldStripOutIllegalWhitespace(String withWhiteSpace, String withoutWhiteSpace) {
+        assertThat(DN.valueOf(withWhiteSpace).toString()).isEqualTo(withoutWhiteSpace);
+        assertThat(DN.valueOf(withWhiteSpace).toNormalizedByteString())
+                .isEqualTo(DN.valueOf(withoutWhiteSpace).toNormalizedByteString());
+    }
 }
diff --git a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFChangeRecordReaderTestCase.java b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFChangeRecordReaderTestCase.java
index fd50547..d102f6e 100644
--- a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFChangeRecordReaderTestCase.java
+++ b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFChangeRecordReaderTestCase.java
@@ -11,7 +11,7 @@
  * Header, with the fields enclosed by brackets [] replaced by your own identifying
  * information: "Portions Copyright [year] [name of copyright owner]".
  *
- * Copyright 2011 ForgeRock AS.
+ * Copyright 2011-2016 ForgeRock AS.
  * Portions copyright 2012 ForgeRock AS.
  * Portions Copyright 2014 Manuel Gaupp
  */
@@ -1122,8 +1122,7 @@
         // Read the record
         ChangeRecord record = reader.readChangeRecord();
         assertThat(record).isInstanceOf(DeleteRequest.class);
-        assertThat(record.getName().toString()).isEqualTo(
-                "ou=Product Development, dc=airius, dc=com");
+        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
         assertThat(record.getControls()).isNotEmpty();
         assertThat(record.getControls().get(0).getOID()).isEqualTo("1.2.840.113556.1.4.805");
         assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("cn");
@@ -1154,8 +1153,7 @@
         // Read the record
         ChangeRecord record = reader.readChangeRecord();
         assertThat(record).isInstanceOf(AddRequest.class);
-        assertThat(record.getName().toString()).isEqualTo(
-                "ou=Product Development, dc=airius, dc=com");
+        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
         assertThat(record.getControls()).isNotEmpty();
         assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.1");
         assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("cn");
@@ -1213,8 +1211,7 @@
         // Read the record
         ChangeRecord record = reader.readChangeRecord();
         assertThat(record).isInstanceOf(AddRequest.class);
-        assertThat(record.getName().toString()).isEqualTo(
-                "ou=Product Development, dc=airius, dc=com");
+        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
         assertThat(record.getControls()).isNotEmpty();
         assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.1");
         assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("cn");
@@ -1245,8 +1242,7 @@
         // Read the record
         ChangeRecord record = reader.readChangeRecord();
         assertThat(record).isInstanceOf(AddRequest.class);
-        assertThat(record.getName().toString()).isEqualTo(
-                "ou=Product Development, dc=airius, dc=com");
+        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
         assertThat(record.getControls()).isNotEmpty();
         assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.1");
         assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("sn");
@@ -1389,8 +1385,7 @@
         // Read the record
         ChangeRecord record = reader.readChangeRecord();
         assertThat(record).isInstanceOf(AddRequest.class);
-        assertThat(record.getName().toString()).isEqualTo(
-                "ou=Product Development, dc=airius, dc=com");
+        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
         assertThat(record.getControls()).isNotEmpty();
         assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.1");
         assertThat(record.getControls().get(0).getValue()).isNull();
@@ -1421,8 +1416,7 @@
         // Read the record
         ChangeRecord record = reader.readChangeRecord();
         assertThat(record).isInstanceOf(AddRequest.class);
-        assertThat(record.getName().toString()).isEqualTo(
-                "ou=Product Development, dc=airius, dc=com");
+        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
         assertThat(record.getControls()).isNotEmpty();
         assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.1");
         assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("description");
@@ -1500,24 +1494,21 @@
         reader.setSchemaValidationPolicy(SchemaValidationPolicy.defaultPolicy());
         // 1st
         ChangeRecord record = reader.readChangeRecord();
-        assertThat(record.getName().toString()).isEqualTo(
-                "ou=Product Development, dc=airius, dc=com");
+        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
         assertThat(record.getControls()).isNotEmpty();
         assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.1");
         assertThat(record.getControls().get(0).getValue()).isNull();
         //2nd
         record = reader.readChangeRecord();
         assertThat(record).isInstanceOf(AddRequest.class);
-        assertThat(record.getName().toString()).isEqualTo(
-                "cn=Paul Jensen, ou=Product Development, dc=airius, dc=com");
+        assertThat(record.getName().toString()).isEqualTo("cn=Paul Jensen,ou=Product Development,dc=airius,dc=com");
         assertThat(record.getControls()).isNotEmpty();
         assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.13");
         assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("cn");
         //3rd
         record = reader.readChangeRecord();
         assertThat(record).isInstanceOf(AddRequest.class);
-        assertThat(record.getName().toString()).isEqualTo(
-                "cn=Paula Jensen, ou=Product Development, dc=airius, dc=com");
+        assertThat(record.getName().toString()).isEqualTo("cn=Paula Jensen,ou=Product Development,dc=airius,dc=com");
         assertThat(record.getControls()).isNotEmpty();
         assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.13.16");
         assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("sn");
@@ -1525,8 +1516,7 @@
     }
 
     /**
-     * Test to read an record containing an invalid control. (pair.value is
-     * null) Must throw a DecodeException.
+     * Test to read an record containing an invalid control. (pair.value is null) Must throw a DecodeException.
      *
      * @throws Exception
      */
diff --git a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFChangeRecordWriterTestCase.java b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFChangeRecordWriterTestCase.java
index 1c45a2e..0bfc7a3 100644
--- a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFChangeRecordWriterTestCase.java
+++ b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFChangeRecordWriterTestCase.java
@@ -11,7 +11,7 @@
  * Header, with the fields enclosed by brackets [] replaced by your own identifying
  * information: "Portions Copyright [year] [name of copyright owner]".
  *
- * Copyright 2011-2015 ForgeRock AS.
+ * Copyright 2011-2016 ForgeRock AS.
  * Portions copyright 2012 ForgeRock AS.
  */
 
@@ -823,8 +823,7 @@
         writer.writeChangeRecord(changeRequest);
         writer.close();
 
-        assertThat(actual.get(0))
-                .isEqualTo("dn: cn=Robert Jensen, ou=Marketing, dc=airius, dc=com");
+        assertThat(actual.get(0)).isEqualTo("dn: cn=Robert Jensen,ou=Marketing,dc=airius,dc=com");
         assertThat(actual.get(1)).isEqualTo("changetype: delete");
     }
 
diff --git a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFEntryReaderTestCase.java b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFEntryReaderTestCase.java
index 40fc816..1fdbcf5 100644
--- a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFEntryReaderTestCase.java
+++ b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFEntryReaderTestCase.java
@@ -12,7 +12,7 @@
  * information: "Portions Copyright [year] [name of copyright owner]".
  *
  * Copyright 2009-2010 Sun Microsystems, Inc.
- * Portions copyright 2012 ForgeRock AS.
+ * Portions copyright 2012-2016 ForgeRock AS.
  */
 
 package org.forgerock.opendj.ldif;
@@ -1105,33 +1105,27 @@
         // @formatter:on
         final String path = TestCaseUtils.createTempFile(strEntry);
         final FileInputStream in = new FileInputStream(path);
-        final LDIFEntryReader reader = new LDIFEntryReader(in);
 
-        Entry entry = null;
-        try {
+        try (LDIFEntryReader reader = new LDIFEntryReader(in)) {
             assertThat(reader.hasNext());
             // 1st entry
-            entry = reader.readEntry();
-            assertThat(entry.getName().toString()).isEqualTo(
-                    "cn=Barbara Jensen, ou=Product Development, dc=airius, dc=com");
+            Entry entry = reader.readEntry();
+            assertThat(entry.getName()
+                            .toString()).isEqualTo("cn=Barbara Jensen,ou=Product Development,dc=airius,dc=com");
             assertThat(entry.getAttributeCount()).isEqualTo(6);
             // 2nd
             entry = reader.readEntry();
-            assertThat(entry.getName().toString()).isEqualTo(
-                    "cn=Bjorn Jensen, ou=Accounting, dc=airius, dc=com");
+            assertThat(entry.getName().toString()).isEqualTo("cn=Bjorn Jensen,ou=Accounting,dc=airius,dc=com");
             assertThat(entry.getAttributeCount()).isEqualTo(4);
 
             assertThat(reader.hasNext()).isFalse();
-        } finally {
-            reader.close();
         }
     }
 
 
     /**
-     * Tries to read an entry composed by multi-valued attributes. The
-     * multi-valued attributes contains an interesting case where two of them
-     * represents the same value, one in uppercase and the other in lower case.
+     * Tries to read an entry composed by multi-valued attributes. The multi-valued attributes contains an interesting
+     * case where two of them represents the same value, one in uppercase and the other in lower case.
      *
      * @throws Exception
      */

--
Gitblit v1.10.0